All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: Bart Van Assche <bvanassche@acm.org>
Cc: linux-scsi <linux-scsi@vger.kernel.org>,
	James Bottomley <jbottomley@parallels.com>,
	Mike Christie <michaelc@cs.wisc.edu>,
	Jun'ichi Nomura <j-nomura@ce.jp.nec.com>,
	Stefan Richter <stefanr@s5r6.in-berlin.de>
Subject: [PATCH 2/4] scsi: Fix device removal NULL pointer dereference
Date: Tue, 05 Jun 2012 17:11:54 +0000	[thread overview]
Message-ID: <4FCE3DDA.80802@acm.org> (raw)
In-Reply-To: <4FCE3D20.4000205@acm.org>

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.

Reported-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: James Bottomley <JBottomley@parallels.com>
Cc: Mike Christie <michaelc@cs.wisc.edu>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: <stable@kernel.org>
---
 drivers/scsi/hosts.c      |    8 +++++++-
 drivers/scsi/scsi_lib.c   |   35 ++++++++---------------------------
 drivers/scsi/scsi_priv.h  |    1 -
 drivers/scsi/scsi_sysfs.c |    5 +----
 4 files changed, 16 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index a3a056a..6b9d89a 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -299,9 +299,15 @@ 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 blk_cleanup_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);
+		blk_cleanup_queue(q);
 	}
 
 	scsi_destroy_command_freelist(shost);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6dfb978..c26ef49 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -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,16 +1367,18 @@ 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)
+	BUG_ON(!sdev);
+
+	if (blk_queue_dead(q))
 		return 0;
 
 	shost = sdev->host;
@@ -1490,11 +1489,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 */
@@ -1697,20 +1692,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 07ce3f5..2b8d8b5 100644
--- 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 04c2a27..42c35ff 100644
--- 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);
 }
 
-- 
1.7.7


  parent reply	other threads:[~2012-06-05 17:11 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-05 17:08 [PATCH 0/4 v7] Fixes for SCSI device removal Bart Van Assche
2012-06-05 17:10 ` [PATCH 1/4] block: Fix race on request_queue.end_io invocations Bart Van Assche
2012-06-05 21:32   ` Tejun Heo
2012-06-06 12:44     ` Bart Van Assche
2012-06-06 12:45   ` Jens Axboe
2012-06-06 13:10     ` Bart Van Assche
2012-06-05 17:11 ` Bart Van Assche [this message]
2012-06-05 17:12 ` [PATCH 3/4] scsi: Change return type of scsi_queue_insert() into void Bart Van Assche
2012-06-05 17:14 ` [PATCH 4/4] scsi: Stop accepting SCSI requests before removing a device Bart Van Assche
2012-06-05 21:36   ` Mike Christie
2012-06-06 12:17     ` Bart Van Assche
2012-06-06 13:29       ` Mike Christie
2012-06-06 14:53         ` Bart Van Assche
2012-06-06 15:21           ` Mike Christie
2012-06-05 22:08   ` Mike Christie
2012-06-06 12:25     ` Bart Van Assche
2012-06-06 13:43       ` Mike Christie
2012-06-06 14:01         ` Mike Christie
2012-06-06 14:12         ` Mike Christie
2012-06-06 15:04           ` Bart Van Assche
2012-06-06 15:28             ` Mike Christie
2012-06-06 16:18               ` Bart Van Assche
2012-06-06 15:07         ` Bart Van Assche
  -- strict thread matches above, loose matches on Subject: below --
2012-06-07 18:39 [PATCH 0/4 v8] Fixes for SCSI device removal Bart Van Assche
2012-06-07 18:43 ` [PATCH 2/4] scsi: Fix device removal NULL pointer dereference Bart Van Assche
2012-06-25 18:12 [PATCH 0/4 v9] SCSI device removal fixes Bart Van Assche
2012-06-25 18:15 ` [PATCH 2/4] scsi: Fix device removal NULL pointer dereference Bart Van Assche
2012-06-25 20:36   ` Tejun Heo
2012-06-25 21:14   ` James Bottomley
2012-06-25 21:21     ` Tejun Heo
2012-06-25 21:35       ` James Bottomley
2012-06-26  7:02         ` Bart Van Assche
2012-06-25 22:05     ` Mike Christie
2012-06-26  7:19       ` James Bottomley
2012-06-26  7:26         ` Bart Van Assche
2012-06-26  6:46     ` Bart Van Assche
2012-06-26  7:25       ` James Bottomley
2012-06-26 10:00         ` Bart Van Assche
2012-06-26  9:13       ` Mike Christie
2012-06-26 10:03         ` Bart Van Assche
2012-06-26 15:03         ` Hannes Reinecke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FCE3DDA.80802@acm.org \
    --to=bvanassche@acm.org \
    --cc=j-nomura@ce.jp.nec.com \
    --cc=jbottomley@parallels.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    --cc=stefanr@s5r6.in-berlin.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.