aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/usb
diff options
context:
space:
mode:
authorAlan Stern <stern@rowland.harvard.edu>2008-10-21 15:28:46 -0400
committerGreg Kroah-Hartman <gregkh@suse.de>2008-10-29 17:54:40 -0400
commitcde217a556ec552d28ac9e136c5a94684a69ae94 (patch)
tree6b8368511301e513c6de8a40dffa357c1064b20e /drivers/usb
parente946217e4fdaa67681bbabfa8e6b18641921f750 (diff)
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>
Diffstat (limited to 'drivers/usb')
-rw-r--r--drivers/usb/core/hcd.c35
-rw-r--r--drivers/usb/core/hcd.h1
-rw-r--r--drivers/usb/core/hub.c1
-rw-r--r--drivers/usb/core/urb.c22
4 files changed, 56 insertions, 3 deletions
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index fc9018e72a09..e1b42626d04d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -106,6 +106,9 @@ static DEFINE_SPINLOCK(hcd_root_hub_lock);
106/* used when updating an endpoint's URB list */ 106/* used when updating an endpoint's URB list */
107static DEFINE_SPINLOCK(hcd_urb_list_lock); 107static DEFINE_SPINLOCK(hcd_urb_list_lock);
108 108
109/* used to protect against unlinking URBs after the device is gone */
110static DEFINE_SPINLOCK(hcd_urb_unlink_lock);
111
109/* wait queue for synchronous unlinks */ 112/* wait queue for synchronous unlinks */
110DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue); 113DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue);
111 114
@@ -1376,10 +1379,25 @@ static int unlink1(struct usb_hcd *hcd, struct urb *urb, int status)
1376int usb_hcd_unlink_urb (struct urb *urb, int status) 1379int usb_hcd_unlink_urb (struct urb *urb, int status)
1377{ 1380{
1378 struct usb_hcd *hcd; 1381 struct usb_hcd *hcd;
1379 int retval; 1382 int retval = -EIDRM;
1383 unsigned long flags;
1380 1384
1381 hcd = bus_to_hcd(urb->dev->bus); 1385 /* Prevent the device and bus from going away while
1382 retval = unlink1(hcd, urb, status); 1386 * the unlink is carried out. If they are already gone
1387 * then urb->use_count must be 0, since disconnected
1388 * devices can't have any active URBs.
1389 */
1390 spin_lock_irqsave(&hcd_urb_unlink_lock, flags);
1391 if (atomic_read(&urb->use_count) > 0) {
1392 retval = 0;
1393 usb_get_dev(urb->dev);
1394 }
1395 spin_unlock_irqrestore(&hcd_urb_unlink_lock, flags);
1396 if (retval == 0) {
1397 hcd = bus_to_hcd(urb->dev->bus);
1398 retval = unlink1(hcd, urb, status);
1399 usb_put_dev(urb->dev);
1400 }
1383 1401
1384 if (retval == 0) 1402 if (retval == 0)
1385 retval = -EINPROGRESS; 1403 retval = -EINPROGRESS;
@@ -1528,6 +1546,17 @@ void usb_hcd_disable_endpoint(struct usb_device *udev,
1528 hcd->driver->endpoint_disable(hcd, ep); 1546 hcd->driver->endpoint_disable(hcd, ep);
1529} 1547}
1530 1548
1549/* Protect against drivers that try to unlink URBs after the device
1550 * is gone, by waiting until all unlinks for @udev are finished.
1551 * Since we don't currently track URBs by device, simply wait until
1552 * nothing is running in the locked region of usb_hcd_unlink_urb().
1553 */
1554void usb_hcd_synchronize_unlinks(struct usb_device *udev)
1555{
1556 spin_lock_irq(&hcd_urb_unlink_lock);
1557 spin_unlock_irq(&hcd_urb_unlink_lock);
1558}
1559
1531/*-------------------------------------------------------------------------*/ 1560/*-------------------------------------------------------------------------*/
1532 1561
1533/* called in any context */ 1562/* called in any context */
diff --git a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h
index 2dcde61c465e..9465e70f4dd0 100644
--- a/drivers/usb/core/hcd.h
+++ b/drivers/usb/core/hcd.h
@@ -232,6 +232,7 @@ extern void usb_hcd_flush_endpoint(struct usb_device *udev,
232 struct usb_host_endpoint *ep); 232 struct usb_host_endpoint *ep);
233extern void usb_hcd_disable_endpoint(struct usb_device *udev, 233extern void usb_hcd_disable_endpoint(struct usb_device *udev,
234 struct usb_host_endpoint *ep); 234 struct usb_host_endpoint *ep);
235extern void usb_hcd_synchronize_unlinks(struct usb_device *udev);
235extern int usb_hcd_get_frame_number(struct usb_device *udev); 236extern int usb_hcd_get_frame_number(struct usb_device *udev);
236 237
237extern struct usb_hcd *usb_create_hcd(const struct hc_driver *driver, 238extern struct usb_hcd *usb_create_hcd(const struct hc_driver *driver,
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 9b3f16bd12cb..37ff8aed256d 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1429,6 +1429,7 @@ void usb_disconnect(struct usb_device **pdev)
1429 */ 1429 */
1430 dev_dbg (&udev->dev, "unregistering device\n"); 1430 dev_dbg (&udev->dev, "unregistering device\n");
1431 usb_disable_device(udev, 0); 1431 usb_disable_device(udev, 0);
1432 usb_hcd_synchronize_unlinks(udev);
1432 1433
1433 usb_unlock_device(udev); 1434 usb_unlock_device(udev);
1434 1435
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index f2638009a464..4342bd9c3bb6 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -474,6 +474,12 @@ EXPORT_SYMBOL_GPL(usb_submit_urb);
474 * indicating that the request has been canceled (rather than any other 474 * indicating that the request has been canceled (rather than any other
475 * code). 475 * code).
476 * 476 *
477 * Drivers should not call this routine or related routines, such as
478 * usb_kill_urb() or usb_unlink_anchored_urbs(), after their disconnect
479 * method has returned. The disconnect function should synchronize with
480 * a driver's I/O routines to insure that all URB-related activity has
481 * completed before it returns.
482 *
477 * This request is always asynchronous. Success is indicated by 483 * This request is always asynchronous. Success is indicated by
478 * returning -EINPROGRESS, at which time the URB will probably not yet 484 * returning -EINPROGRESS, at which time the URB will probably not yet
479 * have been given back to the device driver. When it is eventually 485 * have been given back to the device driver. When it is eventually
@@ -550,6 +556,9 @@ EXPORT_SYMBOL_GPL(usb_unlink_urb);
550 * This routine may not be used in an interrupt context (such as a bottom 556 * This routine may not be used in an interrupt context (such as a bottom
551 * half or a completion handler), or when holding a spinlock, or in other 557 * half or a completion handler), or when holding a spinlock, or in other
552 * situations where the caller can't schedule(). 558 * situations where the caller can't schedule().
559 *
560 * This routine should not be called by a driver after its disconnect
561 * method has returned.
553 */ 562 */
554void usb_kill_urb(struct urb *urb) 563void usb_kill_urb(struct urb *urb)
555{ 564{
@@ -588,6 +597,9 @@ EXPORT_SYMBOL_GPL(usb_kill_urb);
588 * This routine may not be used in an interrupt context (such as a bottom 597 * This routine may not be used in an interrupt context (such as a bottom
589 * half or a completion handler), or when holding a spinlock, or in other 598 * half or a completion handler), or when holding a spinlock, or in other
590 * situations where the caller can't schedule(). 599 * situations where the caller can't schedule().
600 *
601 * This routine should not be called by a driver after its disconnect
602 * method has returned.
591 */ 603 */
592void usb_poison_urb(struct urb *urb) 604void usb_poison_urb(struct urb *urb)
593{ 605{
@@ -622,6 +634,9 @@ EXPORT_SYMBOL_GPL(usb_unpoison_urb);
622 * 634 *
623 * this allows all outstanding URBs to be killed starting 635 * this allows all outstanding URBs to be killed starting
624 * from the back of the queue 636 * from the back of the queue
637 *
638 * This routine should not be called by a driver after its disconnect
639 * method has returned.
625 */ 640 */
626void usb_kill_anchored_urbs(struct usb_anchor *anchor) 641void usb_kill_anchored_urbs(struct usb_anchor *anchor)
627{ 642{
@@ -651,6 +666,9 @@ EXPORT_SYMBOL_GPL(usb_kill_anchored_urbs);
651 * this allows all outstanding URBs to be poisoned starting 666 * this allows all outstanding URBs to be poisoned starting
652 * from the back of the queue. Newly added URBs will also be 667 * from the back of the queue. Newly added URBs will also be
653 * poisoned 668 * poisoned
669 *
670 * This routine should not be called by a driver after its disconnect
671 * method has returned.
654 */ 672 */
655void usb_poison_anchored_urbs(struct usb_anchor *anchor) 673void usb_poison_anchored_urbs(struct usb_anchor *anchor)
656{ 674{
@@ -672,6 +690,7 @@ void usb_poison_anchored_urbs(struct usb_anchor *anchor)
672 spin_unlock_irq(&anchor->lock); 690 spin_unlock_irq(&anchor->lock);
673} 691}
674EXPORT_SYMBOL_GPL(usb_poison_anchored_urbs); 692EXPORT_SYMBOL_GPL(usb_poison_anchored_urbs);
693
675/** 694/**
676 * usb_unlink_anchored_urbs - asynchronously cancel transfer requests en masse 695 * usb_unlink_anchored_urbs - asynchronously cancel transfer requests en masse
677 * @anchor: anchor the requests are bound to 696 * @anchor: anchor the requests are bound to
@@ -680,6 +699,9 @@ EXPORT_SYMBOL_GPL(usb_poison_anchored_urbs);
680 * from the back of the queue. This function is asynchronous. 699 * from the back of the queue. This function is asynchronous.
681 * The unlinking is just tiggered. It may happen after this 700 * The unlinking is just tiggered. It may happen after this
682 * function has returned. 701 * function has returned.
702 *
703 * This routine should not be called by a driver after its disconnect
704 * method has returned.
683 */ 705 */
684void usb_unlink_anchored_urbs(struct usb_anchor *anchor) 706void usb_unlink_anchored_urbs(struct usb_anchor *anchor)
685{ 707{