[PATCH 01/19] usb: core: sysfs: take the port lock to read the BOS

[Greg Kroah-Hartman]

From: Josef Bacik <josef@xxxxxxxxxxxxxx>

usb_reset_and_verify_device() sets udev->bos aside, lets hub_port_init()
read a new BOS into udev->bos, compares the two, frees the new one and
puts the old one back. bos_descriptors_read() takes the device lock
against that since commit 4e0197fbb0ee ("usb: core: sysfs: add lock to
bos_descriptors_read()").

This does not cover one caller. finish_port_resume() does a reset-resume
if the device has the quirk, if its port lost power, or if the device
does not answer GET_STATUS after a normal resume. In an autoresume this
runs without the device lock, for instance from
usb_autopm_get_interface() of an interface driver. The comment above
usb_reset_and_verify_device() says that tasks which hold the device lock
are expected to resume the device first.

bos_descriptors_read() does not. The file is readable for everybody, so
a reader can meet the temporary BOS of a device which is being resumed:
while usb_get_bos_descriptor() has not set ->desc yet (NULL
dereference), before it has corrected a wTotalLength which is larger
than the buffer (read beyond the buffer), or when it is freed (use after
free).

The file was made to give the descriptors to everybody without USB I/O,
so the reader should not wake up the device. There is a lock which both
callers of usb_reset_and_verify_device() hold, the one of the port:
usb_reset_device() takes it around the call and usb_port_resume() for
the whole resume. Take it in the reader too, inside the device lock,
which is the order of usb_reset_device(). A root hub has no port and is
never reset this way.

The port belongs to the parent hub, which removes its ports after it has
disconnected the children. usb_disconnect() cannot finish while the file
is being read, device_del() waits for the reader.

Fixes: 12fc84e8c428 ("usb: Export BOS descriptor to sysfs")
Cc: stable <stable@xxxxxxxxxx>
Cc: Griffin Kroah-Hartman <griffin@xxxxxxxxx>
Cc: Oliver Neukum <oneukum@xxxxxxxx>
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
 drivers/usb/core/hub.c   | 32 ++++++++++++++++++++++++++++++++
 drivers/usb/core/sysfs.c |  4 ++++
 drivers/usb/core/usb.h   |  3 +++
 3 files changed, 39 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 199d3f535a89..9bc12079a80f 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3278,6 +3278,38 @@ static void usb_unlock_port(struct usb_port *port_dev)
 	__release(&port_dev->status_lock);
 }
 
+/**
+ * usb_lock_device_port - lock the port which a device is connected to
+ * @udev: a registered device, locked by the caller
+ *
+ * usb_reset_and_verify_device() runs with the port lock, also where it is
+ * called from an autoresume handler, without the device lock. This keeps it
+ * away without resuming the device.
+ *
+ * The hub removes its ports after usb_disconnect() has unregistered the
+ * child, so the caller has to make sure that this has not finished.
+ *
+ * Return: the port for usb_unlock_device_port(), NULL for a root hub.
+ */
+struct usb_port *usb_lock_device_port(struct usb_device *udev)
+{
+	struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
+	struct usb_port *port_dev;
+
+	if (!hub)
+		return NULL;
+
+	port_dev = hub->ports[udev->portnum - 1];
+	mutex_lock(&port_dev->status_lock);
+	return port_dev;
+}
+
+void usb_unlock_device_port(struct usb_port *port_dev)
+{
+	if (port_dev)
+		mutex_unlock(&port_dev->status_lock);
+}
+
 #ifdef	CONFIG_PM
 
 /* Check if a port is suspended(USB2.0 port) or in U3 state(USB3.0 port) */
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index d22dc78457d7..95a51c7c7a08 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -901,12 +901,15 @@ bos_descriptors_read(struct file *filp, struct kobject *kobj,
 	struct usb_device *udev = to_usb_device(dev);
 	struct usb_host_bos *bos;
 	struct usb_bos_descriptor *desc;
+	struct usb_port *port_dev;
 	size_t desclen, n = 0;
 	int rc;
 
 	rc = usb_lock_device_interruptible(udev);
 	if (rc < 0)
 		return -EINTR;
+	/* A reset-resume replaces udev->bos without the device lock */
+	port_dev = usb_lock_device_port(udev);
 	bos = udev->bos;
 	if (bos) {
 		desc = bos->desc;
@@ -916,6 +919,7 @@ bos_descriptors_read(struct file *filp, struct kobject *kobj,
 			memcpy(buf, (void *) desc + off, n);
 		}
 	}
+	usb_unlock_device_port(port_dev);
 	usb_unlock_device(udev);
 	return n;
 }
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index a9b37aeb515b..824e465bbb12 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -8,6 +8,7 @@
 
 struct usb_hub_descriptor;
 struct usb_dev_state;
+struct usb_port;
 
 /* Functions local to drivers/usb/core/ */
 
@@ -85,6 +86,8 @@ extern bool usb_device_is_owned(struct usb_device *udev);
 
 extern int  usb_hub_init(void);
 extern void usb_hub_cleanup(void);
+extern struct usb_port *usb_lock_device_port(struct usb_device *udev);
+extern void usb_unlock_device_port(struct usb_port *port_dev);
 extern int usb_major_init(void);
 extern void usb_major_cleanup(void);
 extern int usb_device_supports_lpm(struct usb_device *udev);

-- 
2.56.0