Re: [PATCH v2] usb: core: Make sure usb_set_configuration(-1) cannot fail
On Thu, 5 Dec 2013, Julius Werner wrote: 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 jwer...@chromium.org Sorry for not getting back to this sooner. Ironically, it looks like this change isn't needed any more, thanks to Thomas Pugliese's patch: http://marc.info/?l=linux-usbm=13866180520w=2 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -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; That isn't quite right. If the autoresume fails then we have to skip the autosuspend call later on. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: core: Make sure usb_set_configuration(-1) cannot fail
Sorry for not getting back to this sooner. Ironically, it looks like this change isn't needed any more, thanks to Thomas Pugliese's patch: http://marc.info/?l=linux-usbm=13866180520w=2 Yes... thanks for pointing it out, that would make this patch obsolete. I'll wait a few days with this to see if that patch gets accepted as it is. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: core: Make sure usb_set_configuration(-1) cannot fail
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 jwer...@chromium.org
---
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-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
