linux-kernel - [PATCH] usb: fix deadlock with bandwidth

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1282049395-5718-1-git-send-email-cascardo@holoscopio.com>
Date:	Tue, 17 Aug 2010 09:49:55 -0300
From:	Thadeu Lima de Souza Cascardo <cascardo@...oscopio.com>
To:	linux-usb@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, Greg Kroah-Hartman <gregkh@...e.de>,
	Alan Stern <stern@...land.harvard.edu>,
	Sarah Sharp <sarah.a.sharp@...ux.intel.com>,
	Thadeu Lima de Souza Cascardo <cascardo@...oscopio.com>
Subject: [PATCH] usb: fix deadlock with bandwidth_mutex

When using the remove sysfs file, the device configuration is set to -1
(unconfigured). This eventually unbind drivers with the bandwidth_mutex
held. Some drivers may call functions that hold said mutex, like
usb_reset_device. This is the case for rtl8187, for example. This will
lead to the same process holding the mutex twice, which deadlocks.

Unbinding the driver before holding the bandwidth_mutex solves the
problem. If any operation after that fails, drivers are not bound again.
But that would be a problem anyway that the user may solve resetting the
device configuration to one that works, just like he would need to do in
most other failure cases.
---

NOTE: Not signed-off yet, because I'm waiting for some review. Thanks!

---
 drivers/usb/core/message.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index fd4c36e..fe6b9e8 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1724,6 +1724,12 @@ free_interfaces:
 	if (ret)
 		goto free_interfaces;
 
+	/* if it's already configured, clear out old state first.
+	 * getting rid of old interfaces means unbinding their drivers.
+	 */
+	if (dev->state != USB_STATE_ADDRESS)
+		usb_disable_device(dev, 1);	/* Skip ep0 */
+
 	/* Make sure we have bandwidth (and available HCD resources) for this
 	 * configuration.  Remove endpoints from the schedule if we're dropping
 	 * this configuration to set configuration 0.  After this point, the
@@ -1738,12 +1744,6 @@ free_interfaces:
 		goto free_interfaces;
 	}
 
-	/* if it's already configured, clear out old state first.
-	 * getting rid of old interfaces means unbinding their drivers.
-	 */
-	if (dev->state != USB_STATE_ADDRESS)
-		usb_disable_device(dev, 1);	/* Skip ep0 */
-
 	/* Get rid of pending async Set-Config requests for this device */
 	cancel_async_set_config(dev);
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/