sd: Fix device removal NULL pointer dereference
PATCH v4 1/1 sd: Fix device removal NULL pointer dereference
Since scsi_prep_fn() may be invoked concurrently with scsi_remove_device(), keep the queuedata 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.
This patch survived the following test in which removal of a SCSI disk (sd) device was triggered 100.000 times, where removal was triggered frequently while LUN scanning was still ongoing:
$ time for ((i=0;i<100000;i++)); do \ echo 'id_ext=0002c9030005f34e,ioc_guid=0002c9030005f34e,dgid=fe800000000000000002c9030005f34f,pkey=ffff,service_id=0002c9030005f34e' \
Quoted 1 line
/sys/class/infiniband_srp/srp-mlx4_0-1/add_target; \
done
real 28m49.672s user 0m4.000s sys 3m25.950s
Reported-by Jun'ichi Nomura <[email protected]>
Signed-off-by Bart Van Assche <[email protected]>
Cc James Bottomley <[email protected]>
Cc Stefan Richter <[email protected]>
--- drivers/scsi/hosts.c | 6 ++++++ drivers/scsi/scsi_lib.c | 29 +++++++---------------------- drivers/scsi/scsi_sysfs.c | 3 --- 3 files changed, 13 insertions(+), 25 deletions(-)
Changes compared to v3:
- Added blk_queue_dead() test in scsi_lld_busy().
- Removed q->queuedata test in scsi_run_queue().
- Left out block layer changes.
drivers/scsi/hosts.c +6 -0
@@ -296,6 +296,12 @@ static void scsi_host_dev_release(struct device *dev)destroy_workqueue (shost->work_q); q = shost->uspace_req_q;if (q) { +/* +* Note: freeing queuedata before invoking scsi_free_queue() +* is safe here because no request function is associated with +* uspace_req_q. See also the __scsi_alloc_queue() call in +* drivers/scsi/scsi_tgt_lib.c. +*/ kfree (q->queuedata); q->queuedata =NULL ;scsi_free_queue (q);
drivers/scsi/scsi_lib.c +7 -22
@@ -406,10 +406,7 @@ 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 ; - +BUG_ON (!sdev); shost = sdev->host;if (scsi_target (sdev)->single_lun)scsi_single_lun_run (sdev);
@@ -1370,9 +1367,9 @@ 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) {
@@ -1380,9 +1377,10 @@ static int scsi_lld_busy(struct request_queue *q)struct Scsi_Host *shost;struct scsi_target *starget; -if (!sdev) +if (blk_queue_dead (q))return 0 ; +BUG_ON (!sdev); shost = sdev->host; starget =scsi_target (sdev);
@@ -1487,11 +1485,7 @@ 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 ; - } +BUG_ON (!sdev);if (!get_device (&sdev->sdev_gendev))/* We must be tearing the block queue down already */
@@ -1696,15 +1690,6 @@ struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)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); }
drivers/scsi/scsi_sysfs.c +0 -3
@@ -971,9 +971,6 @@ 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);put_device (dev);
Signature · 1 line
1.7.7