All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org,
	alan@lxorguk.ukuu.org.uk, Alan Stern <stern@rowland.harvard.edu>,
	Sarah Sharp <sarah.a.sharp@linux.intel.com>
Subject: [ 22/42] USB: fix deadlock in bConfigurationValue attribute method
Date: Tue, 24 Apr 2012 15:33:15 -0700	[thread overview]
Message-ID: <20120424223255.365387564@linuxfoundation.org> (raw)
In-Reply-To: <20120424223311.GA8456@kroah.com>

3.0-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Alan Stern <stern@rowland.harvard.edu>

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>

---
 drivers/usb/core/hub.c     |    3 ---
 drivers/usb/core/message.c |    6 +++---
 2 files changed, 3 insertions(+), 6 deletions(-)

--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1644,7 +1644,6 @@ void usb_disconnect(struct usb_device **
 {
 	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 **
 	 * 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);
 	usb_hcd_synchronize_unlinks(udev);
 
 	usb_remove_ep_devs(&udev->ep0);
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1136,8 +1136,6 @@ void usb_disable_interface(struct usb_de
  * 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_devic
 			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) {
 		mutex_unlock(hcd->bandwidth_mutex);



  parent reply	other threads:[~2012-04-24 22:38 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-24 22:33 [ 00/42] 3.0.30-stable review Greg KH
2012-04-24 22:32 ` [ 01/42] Perf: fix build breakage Greg KH
2012-04-24 22:32 ` [ 02/42] crypto: sha512 - Fix byte counter overflow in SHA-512 Greg KH
2012-04-24 22:32   ` Greg KH
2012-04-24 22:32 ` [ 03/42] hwmon: fam15h_power: fix bogus values with current BIOSes Greg KH
2012-04-25 19:46   ` Ben Hutchings
2012-04-26 21:16     ` Greg KH
2012-04-24 22:32 ` [ 04/42] ALSA: hda/conexant - Dont set HP pin-control bit unconditionally Greg KH
2012-04-24 22:32 ` [ 05/42] ARM: clps711x: serial driver hungs are a result of call disable_irq within ISR Greg KH
2012-04-24 22:32 ` [ 06/42] xen/gntdev: do not set VM_PFNMAP Greg KH
2012-04-24 22:33 ` [ 07/42] xen/xenbus: Add quirk to deal with misconfigured backends Greg KH
2012-04-24 22:33 ` [ 08/42] USB: yurex: Remove allocation of coherent buffer for setup-packet buffer Greg KH
2012-04-24 22:33 ` [ 09/42] USB: yurex: Fix missing URB_NO_TRANSFER_DMA_MAP flag in urb Greg KH
2012-04-24 22:33 ` [ 10/42] uwb: fix use of del_timer_sync() in interrupt Greg KH
2012-04-24 22:33 ` [ 11/42] uwb: fix error handling Greg KH
2012-04-24 22:33 ` [ 12/42] davinci_mdio: Fix MDIO timeout check Greg KH
2012-04-24 22:33 ` [ 13/42] media: rc-core: set mode for winbond-cir Greg KH
2012-04-24 22:33 ` [ 14/42] cfg80211: fix interface combinations check Greg KH
2012-04-24 22:33 ` [ 15/42] mm: fix s390 BUG by __set_page_dirty_no_writeback on swap Greg KH
2012-04-24 22:33 ` [ 16/42] jbd2: use GFP_NOFS for blkdev_issue_flush Greg KH
2012-04-24 22:33 ` [ 17/42] USB: serial: cp210x: Fixed usb_control_msg timeout values Greg KH
2012-04-24 22:33 ` [ 18/42] pch_uart: Fix dma channel unallocated issue Greg KH
2012-04-24 22:33 ` [ 19/42] drivers/tty/amiserial.c: add missing tty_unlock Greg KH
2012-04-24 22:33 ` [ 20/42] USB: sierra: avoid QMI/wwan interface on MC77xx Greg KH
2012-04-24 22:33 ` [ 21/42] EHCI: always clear the STS_FLR status bit Greg KH
2012-04-24 22:33 ` Greg KH [this message]
2012-04-24 22:33 ` [ 23/42] usb: gadget: eliminate NULL pointer dereference (bugfix) Greg KH
2012-04-24 22:33 ` [ 24/42] usb: musb: omap: fix crash when musb glue (omap) gets initialized Greg KH
2012-04-24 22:33 ` [ 25/42] usb: musb: omap: fix the error check for pm_runtime_get_sync Greg KH
2012-04-24 22:33 ` [ 26/42] PCI: Add quirk for still enabled interrupts on Intel Sandy Bridge GPUs Greg KH
2012-04-24 22:33 ` [ 27/42] ext4: fix endianness breakage in ext4_split_extent_at() Greg KH
2012-04-24 22:33 ` [ 28/42] Bluetooth: Add support for Atheros [04ca:3005] Greg KH
2012-04-24 22:33 ` [ 29/42] Dont limit non-nested epoll paths Greg KH
2012-04-24 22:33 ` [ 30/42] spi: Fix device unregistration when unregistering the bus master Greg KH
2012-04-24 22:33 ` [ 31/42] rt2x00: Properly identify rt2800usb devices Greg KH
2012-04-24 22:33 ` [ 32/42] rt2800usb: Add new device ID for Belkin Greg KH
2012-04-24 22:33 ` [ 33/42] rt2x00: Add USB device ID of Buffalo WLI-UC-GNHP Greg KH
2012-04-24 22:33 ` [ 34/42] rt2800: Add support for the Fujitsu Stylistic Q550 Greg KH
2012-04-24 22:33 ` [ 35/42] rt2x00: Identify rt2800usb chipsets Greg KH
2012-04-24 22:33 ` [ 36/42] nfsd: fix compose_entry_fh() failure exits Greg KH
2012-04-24 22:33 ` [ 37/42] btrfs: btrfs_root_readonly() broken on big-endian Greg KH
2012-04-24 22:33 ` [ 38/42] ocfs2: ->l_next_free_req breakage " Greg KH
2012-04-24 22:33 ` [ 39/42] ocfs: ->rl_used " Greg KH
2012-04-24 22:33 ` [ 40/42] ocfs2: ->rl_count endianness breakage Greg KH
2012-04-24 22:33 ` [ 41/42] ocfs2: ->e_leaf_clusters " Greg KH
2012-04-24 22:33 ` [ 42/42] lockd: fix the endianness bug Greg KH

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=20120424223255.365387564@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sarah.a.sharp@linux.intel.com \
    --cc=stable@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=torvalds@linux-foundation.org \
    /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.