Home Home > GIT Browse
diff options
authorAlan Stern <stern@rowland.harvard.edu>2012-04-17 15:22:39 -0400
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2012-04-27 09:51:08 -0700
commit5eb68e665cadf165741a04c695a71cff11e1d4f1 (patch)
parent3599fbb407dfa09d1d8d1507961fa03088c3b695 (diff)
USB: fix deadlock in bConfigurationValue attribute method
commit 8963c487a80b4688c9e68dcc504a90074aacc145 upstream. This patch (as154) fixes a self-deadlock that occurs when userspace writes to the bConfigurationValue sysfs attribute for a hub with children. The task tries to lock the bandwidth_mutex at a time when it already owns the lock: The attribute's method calls usb_set_configuration(), which calls usb_disable_device() with the bandwidth_mutex held. usb_disable_device() unregisters the existing interfaces, which causes the hub driver to be unbound. The hub_disconnect() routine calls hub_quiesce(), which calls usb_disconnect() for each of the hub's children. usb_disconnect() attempts to acquire the bandwidth_mutex around a call to usb_disable_device(). The solution is to make usb_disable_device() acquire the mutex for itself instead of requiring the caller to hold it. Then the mutex can cover only the bandwidth deallocation operation and not the region where the interfaces are unregistered. This has the potential to change system behavior slightly when a config change races with another config or altsetting change. Some of the bandwidth released from the old config might get claimed by the other config or altsetting, make it impossible to restore the old config in case of a failure. But since we don't try to recover from config-change failures anyway, this doesn't matter. [This should be marked for stable kernels that contain the commit fccf4e86200b8f5edd9a65da26f150e32ba79808 "USB: Free bandwidth when usb_disable_device is called." That commit was marked for stable kernels as old as 2.6.32.] Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2 files changed, 3 insertions, 6 deletions
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3776ddf0c75d..34bb059607f5 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1644,7 +1644,6 @@ void usb_disconnect(struct usb_device **pdev)
struct usb_device *udev = *pdev;
int i;
- struct usb_hcd *hcd = bus_to_hcd(udev->bus);
if (!udev) {
pr_debug ("%s nodev\n", __func__);
@@ -1672,9 +1671,7 @@ void usb_disconnect(struct usb_device **pdev)
* so that the hardware is now fully quiesced.
dev_dbg (&udev->dev, "unregistering device\n");
- mutex_lock(hcd->bandwidth_mutex);
usb_disable_device(udev, 0);
- mutex_unlock(hcd->bandwidth_mutex);
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 922360066843..1eebd45b8137 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1136,8 +1136,6 @@ void usb_disable_interface(struct usb_device *dev, struct usb_interface *intf,
* Deallocates hcd/hardware state for the endpoints (nuking all or most
* pending urbs) and usbcore state for the interfaces, so that usbcore
* must usb_set_configuration() before any interfaces could be used.
- *
- * Must be called with hcd->bandwidth_mutex held.
void usb_disable_device(struct usb_device *dev, int skip_ep0)
@@ -1190,7 +1188,9 @@ void usb_disable_device(struct usb_device *dev, int skip_ep0)
usb_disable_endpoint(dev, i + USB_DIR_IN, false);
/* Remove endpoints from the host controller internal state */
+ mutex_lock(hcd->bandwidth_mutex);
usb_hcd_alloc_bandwidth(dev, NULL, NULL, NULL);
+ mutex_unlock(hcd->bandwidth_mutex);
/* Second pass: remove endpoint pointers */
for (i = skip_ep0; i < 16; ++i) {
@@ -1750,7 +1750,6 @@ free_interfaces:
/* if it's already configured, clear out old state first.
* getting rid of old interfaces means unbinding their drivers.
- mutex_lock(hcd->bandwidth_mutex);
if (dev->state != USB_STATE_ADDRESS)
usb_disable_device(dev, 1); /* Skip ep0 */
@@ -1763,6 +1762,7 @@ free_interfaces:
* host controller will not allow submissions to dropped endpoints. If
* this call fails, the device state is unchanged.
+ mutex_lock(hcd->bandwidth_mutex);
ret = usb_hcd_alloc_bandwidth(dev, cp, NULL, NULL);
if (ret < 0) {