linux-kernel - [PATCH v2] usb: core: Make sure usb_set

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <1386300593-1270-1-git-send-email-jwerner@chromium.org>
Date:	Thu,  5 Dec 2013 19:29:53 -0800
From:	Julius Werner <jwerner@...omium.org>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	Alan Stern <stern@...land.harvard.edu>,
	Sarah Sharp <sarah.a.sharp@...ux.intel.com>,
	Vincent Palatin <vpalatin@...omium.org>,
	Doug Anderson <dianders@...omium.org>,
	Vivek Gautam <gautam.vivek@...sung.com>,
	Julius Werner <jwerner@...omium.org>
Subject: [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 <jwerner@...omium.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-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/