[PATCH v2] usb: core: Make sure usb_set_configuration(-1) cannot fail

From: Julius Werner
Date: Thu Dec 05 2013 - 22:30:32 EST


usb_deauthorize_device() tries to unset the configuration of a USB
device and then unconditionally blows away the configuration descriptors
with usb_destroy_configuration(). This is bad if the
usb_set_configuration() call failed before the configuration could be
correctly unset, since pointers like udev->actconfig can keep pointing
to the now invalid memory. We have encountered hard to reproduce crashes
from devices disconnected during suspend due to this, where khubd's
disconnect handling races with userspace tools that change authorization
on resume.

It seems desirable that a USB device can always be marked as
unconfigured (reclaiming its bandwidth) in the kernel, regardless of
communication problems. This patch changes usb_set_configuration() to
ignore all failures in the case where no new configuration is being set.

Signed-off-by: Julius Werner <jwerner@xxxxxxxxxxxx>
---
drivers/usb/core/message.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index bb31597..f980b6d 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1706,7 +1706,8 @@ static void __usb_queue_reset_device(struct work_struct *ws)
* underlying call that failed. On successful completion, each interface
* in the original device configuration has been destroyed, and each one
* in the new configuration has been probed by all relevant usb device
- * drivers currently known to the kernel.
+ * drivers currently known to the kernel. Guaranteed not to fail if the
+ * device is to be unconfigured (@configuration = -1).
*/
int usb_set_configuration(struct usb_device *dev, int configuration)
{
@@ -1774,7 +1775,7 @@ free_interfaces:

/* Wake up the device so we can send it the Set-Config request */
ret = usb_autoresume_device(dev);
- if (ret)
+ if (ret && cp)
goto free_interfaces;

/* if it's already configured, clear out old state first.
@@ -1797,7 +1798,7 @@ free_interfaces:
* installed, so that the xHCI driver can recalculate the U1/U2
* timeouts.
*/
- if (dev->actconfig && usb_disable_lpm(dev)) {
+ if (dev->actconfig && usb_disable_lpm(dev) && cp) {
dev_err(&dev->dev, "%s Failed to disable LPM\n.", __func__);
mutex_unlock(hcd->bandwidth_mutex);
ret = -ENOMEM;
--
1.7.12.4

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