SCSI: Fix device removal NULL pointer dereference

commit 67bd94130015c507011af37858989b199c52e1de upstream.

Use blk_queue_dead() to test whether the queue is dead instead
of !sdev. Since scsi_prep_fn() may be invoked concurrently with
__scsi_remove_device(), keep the queuedata (sdev) pointer in
__scsi_remove_device(). This patch fixes a kernel oops that
can be triggered by USB device removal. See also
http://www.spinics.net/lists/linux-scsi/msg56254.html.

Other changes included in this patch:
- Swap the blk_cleanup_queue() and kfree() calls in
  scsi_host_dev_release() to make that code easier to grasp.
- Remove the queue dead check from scsi_run_queue() since the
  queue state can change anyway at any point in that function
  where the queue lock is not held.
- Remove the queue dead check from the start of scsi_request_fn()
  since it is redundant with the scsi_device_online() check.

Reported-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
Reviewed-by: Tejun Heo <tj@kernel.org>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index a3a056a9db67bffb694208d30c44f4807c843a7a..b48c24f7d21aacbae95775e0f47e841de5015f66 100644 (file)
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -290,6 +290,7 @@ static void scsi_host_dev_release(struct device *dev)
        struct Scsi_Host *shost = dev_to_shost(dev);
        struct device *parent = dev->parent;
        struct request_queue *q;
+       void *queuedata;
 
        scsi_proc_hostdir_rm(shost->hostt);
 
@@ -299,9 +300,9 @@ static void scsi_host_dev_release(struct device *dev)
                destroy_workqueue(shost->work_q);
        q = shost->uspace_req_q;
        if (q) {
-               kfree(q->queuedata);
-               q->queuedata = NULL;
-               scsi_free_queue(q);
+               queuedata = q->queuedata;
+               blk_cleanup_queue(q);
+               kfree(queuedata);
        }
 
        scsi_destroy_command_freelist(shost);

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6dfb9785d34581eb06395ef1d4b372b243de5d87..7ebe1676e80a8331f78e26c8a4e1a369eb2387d2 100644 (file)
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -406,10 +406,6 @@ static void scsi_run_queue(struct request_queue *q)
        LIST_HEAD(starved_list);
        unsigned long flags;
 
-       /* if the device is dead, sdev will be NULL, so no queue to run */
-       if (!sdev)
-               return;
-
        shost = sdev->host;
        if (scsi_target(sdev)->single_lun)
                scsi_single_lun_run(sdev);
@@ -1370,16 +1366,16 @@ static inline int scsi_host_queue_ready(struct request_queue *q,
  * may be changed after request stacking drivers call the function,
  * regardless of taking lock or not.
  *
- * When scsi can't dispatch I/Os anymore and needs to kill I/Os
- * (e.g. !sdev), scsi needs to return 'not busy'.
- * Otherwise, request stacking drivers may hold requests forever.
+ * When scsi can't dispatch I/Os anymore and needs to kill I/Os scsi
+ * needs to return 'not busy'. Otherwise, request stacking drivers
+ * may hold requests forever.
  */
 static int scsi_lld_busy(struct request_queue *q)
 {
        struct scsi_device *sdev = q->queuedata;
        struct Scsi_Host *shost;
 
-       if (!sdev)
+       if (blk_queue_dead(q))
                return 0;
 
        shost = sdev->host;
@@ -1490,12 +1486,6 @@ static void scsi_request_fn(struct request_queue *q)
        struct scsi_cmnd *cmd;
        struct request *req;
 
-       if (!sdev) {
-               while ((req = blk_peek_request(q)) != NULL)
-                       scsi_kill_request(req, q);
-               return;
-       }
-
        if(!get_device(&sdev->sdev_gendev))
                /* We must be tearing the block queue down already */
                return;
@@ -1697,20 +1687,6 @@ struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
        return q;
 }
 
-void scsi_free_queue(struct request_queue *q)
-{
-       unsigned long flags;
-
-       WARN_ON(q->queuedata);
-
-       /* cause scsi_request_fn() to kill all non-finished requests */
-       spin_lock_irqsave(q->queue_lock, flags);
-       q->request_fn(q);
-       spin_unlock_irqrestore(q->queue_lock, flags);
-
-       blk_cleanup_queue(q);
-}
-
 /*
  * Function:    scsi_block_requests()
  *

diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 07ce3f51701dba2163074feb89e623054b451e5d..2b8d8b54c7e503b43b4705536d6040383aeb792e 100644 (file)
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -84,7 +84,6 @@ extern void scsi_next_command(struct scsi_cmnd *cmd);
 extern void scsi_io_completion(struct scsi_cmnd *, unsigned int);
 extern void scsi_run_host_queues(struct Scsi_Host *shost);
 extern struct request_queue *scsi_alloc_queue(struct scsi_device *sdev);
-extern void scsi_free_queue(struct request_queue *q);
 extern int scsi_init_queue(void);
 extern void scsi_exit_queue(void);
 struct request_queue;

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index f888aadc8c2551dcec5b90e6805102382b17ca0a..bb7c4828acc0416774f3f9958886df261d9d617a 100644 (file)
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -971,11 +971,8 @@ void __scsi_remove_device(struct scsi_device *sdev)
                sdev->host->hostt->slave_destroy(sdev);
        transport_destroy_device(dev);
 
-       /* cause the request function to reject all I/O requests */
-       sdev->request_queue->queuedata = NULL;
-
        /* Freeing the queue signals to block that we're done */
-       scsi_free_queue(sdev->request_queue);
+       blk_cleanup_queue(sdev->request_queue);
        put_device(dev);
 }