patch "usb-storage: fix deadlock involving host lock and scsi_done" added to usb-linus

[<gregkh@linuxfoundation.org>]

This is a note to let you know that I've just added the patch titled

    usb-storage: fix deadlock involving host lock and scsi_done

to my usb git tree which can be found at
    git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.

The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)

The patch will hopefully also be merged in Linus's tree for the
next -rc kernel release.

If you have any questions about this process, please let me know.


>From 8b52291a0743fc4db4a7495c846a6f31ee84d282 Mon Sep 17 00:00:00 2001
From: Alan Stern <stern@rowland.harvard.edu>
Date: Wed, 26 Jul 2017 11:49:19 -0400
Subject: usb-storage: fix deadlock involving host lock and scsi_done

Christoph Hellwig says that since version 4.12, the kernel switched to
using blk-mq by default.  The old code used a softirq for handling
request completions, but blk-mq can handle completions in the caller's
context.  This may cause a problem for usb-storage, because it invokes
the ->scsi_done callback while holding the host lock, and the
completion routine sometimes tries to acquire the same lock (when
running the error handler, for example).

The consequence is that the existing code will sometimes deadlock upon
error completion of a SCSI command (with a lockdep warning).

This is easy enough to fix, since usb-storage doesn't really need to
hold the host lock while the callback runs.  It was simpler to write
it that way, but moving the call outside the locked region is pretty
easy and there's no downside.  That's what this patch does.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: Arthur Marsh <arthur.marsh@internode.on.net>
CC: Christoph Hellwig <hch@lst.de>
CC: <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/storage/usb.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 06615934fed1..0dceb9fa3a06 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -315,6 +315,7 @@ static int usb_stor_control_thread(void * __us)
 {
 	struct us_data *us = (struct us_data *)__us;
 	struct Scsi_Host *host = us_to_host(us);
+	struct scsi_cmnd *srb;
 
 	for (;;) {
 		usb_stor_dbg(us, "*** thread sleeping\n");
@@ -330,6 +331,7 @@ static int usb_stor_control_thread(void * __us)
 		scsi_lock(host);
 
 		/* When we are called with no command pending, we're done */
+		srb = us->srb;
 		if (us->srb == NULL) {
 			scsi_unlock(host);
 			mutex_unlock(&us->dev_mutex);
@@ -398,14 +400,11 @@ static int usb_stor_control_thread(void * __us)
 		/* lock access to the state */
 		scsi_lock(host);
 
-		/* indicate that the command is done */
-		if (us->srb->result != DID_ABORT << 16) {
-			usb_stor_dbg(us, "scsi cmd done, result=0x%x\n",
-				     us->srb->result);
-			us->srb->scsi_done(us->srb);
-		} else {
+		/* was the command aborted? */
+		if (us->srb->result == DID_ABORT << 16) {
 SkipForAbort:
 			usb_stor_dbg(us, "scsi command aborted\n");
+			srb = NULL;	/* Don't call srb->scsi_done() */
 		}
 
 		/*
@@ -429,6 +428,13 @@ static int usb_stor_control_thread(void * __us)
 
 		/* unlock the device pointers */
 		mutex_unlock(&us->dev_mutex);
+
+		/* now that the locks are released, notify the SCSI core */
+		if (srb) {
+			usb_stor_dbg(us, "scsi cmd done, result=0x%x\n",
+					srb->result);
+			srb->scsi_done(srb);
+		}
 	} /* for (;;) */
 
 	/* Wait until we are told to stop */
-- 
2.13.3