USB: fix crash when URBs are unlinked after the device is gone

This patch (as1151) protects usbcore against drivers that try to
unlink an URB after the URB's device or bus have been removed.  The
core does not currently check for this, and certain drivers can cause
a crash if they are running while an HCD is unloaded.

Certainly it would be best to fix the guilty drivers.  But a little
defensive programming doesn't hurt, especially since it appears that
quite a few drivers need to be fixed.

The patch prevents the problem by grabbing a reference to the device
while an unlink is in progress and using a new spinlock to synchronize
unlinks with device removal.  (There's no need to acquire a reference
to the bus as well, since the device structure itself keeps a
reference to the bus.)  In addition, the kerneldoc is updated to
indicate that URBs should not be unlinked after the disconnect method
returns.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

1 files changed, 32 insertions, 3 deletions

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index fc9018e72a0..e1b42626d04 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -106,6 +106,9 @@ static DEFINE_SPINLOCK(hcd_root_hub_lock);
 /* used when updating an endpoint's URB list */
 static DEFINE_SPINLOCK(hcd_urb_list_lock);
 
+/* used to protect against unlinking URBs after the device is gone */
+static DEFINE_SPINLOCK(hcd_urb_unlink_lock);
+
 /* wait queue for synchronous unlinks */
 DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue);
 
@@ -1376,10 +1379,25 @@ static int unlink1(struct usb_hcd *hcd, struct urb *urb, int status)
 int usb_hcd_unlink_urb (struct urb *urb, int status)
 {
 	struct usb_hcd		*hcd;
-	int			retval;
+	int			retval = -EIDRM;
+	unsigned long		flags;
 
-	hcd = bus_to_hcd(urb->dev->bus);
-	retval = unlink1(hcd, urb, status);
+	/* Prevent the device and bus from going away while
+	 * the unlink is carried out.  If they are already gone
+	 * then urb->use_count must be 0, since disconnected
+	 * devices can't have any active URBs.
+	 */
+	spin_lock_irqsave(&hcd_urb_unlink_lock, flags);
+	if (atomic_read(&urb->use_count) > 0) {
+		retval = 0;
+		usb_get_dev(urb->dev);
+	}
+	spin_unlock_irqrestore(&hcd_urb_unlink_lock, flags);
+	if (retval == 0) {
+		hcd = bus_to_hcd(urb->dev->bus);
+		retval = unlink1(hcd, urb, status);
+		usb_put_dev(urb->dev);
+	}
 
 	if (retval == 0)
 		retval = -EINPROGRESS;
@@ -1528,6 +1546,17 @@ void usb_hcd_disable_endpoint(struct usb_device *udev,
 		hcd->driver->endpoint_disable(hcd, ep);
 }
 
+/* Protect against drivers that try to unlink URBs after the device
+ * is gone, by waiting until all unlinks for @udev are finished.
+ * Since we don't currently track URBs by device, simply wait until
+ * nothing is running in the locked region of usb_hcd_unlink_urb().
+ */
+void usb_hcd_synchronize_unlinks(struct usb_device *udev)
+{
+	spin_lock_irq(&hcd_urb_unlink_lock);
+	spin_unlock_irq(&hcd_urb_unlink_lock);
+}
+
 /*-------------------------------------------------------------------------*/
 
 /* called in any context */