USB: core: Change usb_get_device_descriptor() API

The usb_get_device_descriptor() routine reads the device descriptor
from the udev device and stores it directly in udev->descriptor.  This
interface is error prone, because the USB subsystem expects in-memory
copies of a device's descriptors to be immutable once the device has
been initialized.

The interface is changed so that the device descriptor is left in a
kmalloc-ed buffer, not copied into the usb_device structure.  A
pointer to the buffer is returned to the caller, who is then
responsible for kfree-ing it.  The corresponding changes needed in the
various callers are fairly small.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Link: https://lore.kernel.org/r/d0111bb6-56c1-4f90-adf2-6cfe152f6561@rowland.harvard.edu
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 8300baedafd20e2d188ae0f7e537c14118b677d7..6af0a31ff1475879da59ec1a667d5b6e894a53d9 100644 (file)
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -983,6 +983,7 @@ static int register_root_hub(struct usb_hcd *hcd)
 {
        struct device *parent_dev = hcd->self.controller;
        struct usb_device *usb_dev = hcd->self.root_hub;
+       struct usb_device_descriptor *descr;
        const int devnum = 1;
        int retval;
 
@@ -994,13 +995,16 @@ static int register_root_hub(struct usb_hcd *hcd)
        mutex_lock(&usb_bus_idr_lock);
 
        usb_dev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
-       retval = usb_get_device_descriptor(usb_dev, USB_DT_DEVICE_SIZE);
-       if (retval != sizeof usb_dev->descriptor) {
+       descr = usb_get_device_descriptor(usb_dev);
+       if (IS_ERR(descr)) {
+               retval = PTR_ERR(descr);
                mutex_unlock(&usb_bus_idr_lock);
                dev_dbg (parent_dev, "can't read %s device descriptor %d\n",
                                dev_name(&usb_dev->dev), retval);
-               return (retval < 0) ? retval : -EMSGSIZE;
+               return retval;
        }
+       usb_dev->descriptor = *descr;
+       kfree(descr);
 
        if (le16_to_cpu(usb_dev->descriptor.bcdUSB) >= 0x0201) {
                retval = usb_get_bos_descriptor(usb_dev);

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 91abcd904d04484e42eff9fd564b8effca23c745..9279c8ccbcf250108f0b1f2514463dee3fc905d7 100644 (file)
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2694,12 +2694,17 @@ int usb_authorize_device(struct usb_device *usb_dev)
        }
 
        if (usb_dev->wusb) {
-               result = usb_get_device_descriptor(usb_dev, sizeof(usb_dev->descriptor));
-               if (result < 0) {
+               struct usb_device_descriptor *descr;
+
+               descr = usb_get_device_descriptor(usb_dev);
+               if (IS_ERR(descr)) {
+                       result = PTR_ERR(descr);
                        dev_err(&usb_dev->dev, "can't re-read device descriptor for "
                                "authorization: %d\n", result);
                        goto error_device_descriptor;
                }
+               usb_dev->descriptor = *descr;
+               kfree(descr);
        }
 
        usb_dev->authorized = 1;
@@ -4827,7 +4832,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
        const char              *driver_name;
        bool                    do_new_scheme;
        int                     maxp0;
-       struct usb_device_descriptor    *buf;
+       struct usb_device_descriptor    *buf, *descr;
 
        buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
        if (!buf)
@@ -5069,15 +5074,16 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
                usb_ep0_reinit(udev);
        }
 
-       retval = usb_get_device_descriptor(udev, USB_DT_DEVICE_SIZE);
-       if (retval < (signed)sizeof(udev->descriptor)) {
+       descr = usb_get_device_descriptor(udev);
+       if (IS_ERR(descr)) {
+               retval = PTR_ERR(descr);
                if (retval != -ENODEV)
                        dev_err(&udev->dev, "device descriptor read/all, error %d\n",
                                        retval);
-               if (retval >= 0)
-                       retval = -ENOMSG;
                goto fail;
        }
+       udev->descriptor = *descr;
+       kfree(descr);
 
        /*
         * Some superspeed devices have finished the link training process
@@ -5196,7 +5202,7 @@ hub_power_remaining(struct usb_hub *hub)
 
 
 static int descriptors_changed(struct usb_device *udev,
-               struct usb_device_descriptor *old_device_descriptor,
+               struct usb_device_descriptor *new_device_descriptor,
                struct usb_host_bos *old_bos)
 {
        int             changed = 0;
@@ -5207,8 +5213,8 @@ static int descriptors_changed(struct usb_device *udev,
        int             length;
        char            *buf;
 
-       if (memcmp(&udev->descriptor, old_device_descriptor,
-                       sizeof(*old_device_descriptor)) != 0)
+       if (memcmp(&udev->descriptor, new_device_descriptor,
+                       sizeof(*new_device_descriptor)) != 0)
                return 1;
 
        if ((old_bos && !udev->bos) || (!old_bos && udev->bos))
@@ -5533,9 +5539,8 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
 {
        struct usb_port *port_dev = hub->ports[port1 - 1];
        struct usb_device *udev = port_dev->child;
-       struct usb_device_descriptor descriptor;
+       struct usb_device_descriptor *descr;
        int status = -ENODEV;
-       int retval;
 
        dev_dbg(&port_dev->dev, "status %04x, change %04x, %s\n", portstatus,
                        portchange, portspeed(hub, portstatus));
@@ -5562,23 +5567,20 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
                         * changed device descriptors before resuscitating the
                         * device.
                         */
-                       descriptor = udev->descriptor;
-                       retval = usb_get_device_descriptor(udev,
-                                       sizeof(udev->descriptor));
-                       if (retval < 0) {
+                       descr = usb_get_device_descriptor(udev);
+                       if (IS_ERR(descr)) {
                                dev_dbg(&udev->dev,
-                                               "can't read device descriptor %d\n",
-                                               retval);
+                                               "can't read device descriptor %ld\n",
+                                               PTR_ERR(descr));
                        } else {
-                               if (descriptors_changed(udev, &descriptor,
+                               if (descriptors_changed(udev, descr,
                                                udev->bos)) {
                                        dev_dbg(&udev->dev,
                                                        "device descriptor has changed\n");
-                                       /* for disconnect() calls */
-                                       udev->descriptor = descriptor;
                                } else {
                                        status = 0; /* Nothing to do */
                                }
+                               kfree(descr);
                        }
 #ifdef CONFIG_PM
                } else if (udev->state == USB_STATE_SUSPENDED &&

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 0d2bfc909019b8a724a2835238e1432a3680b267..077dfe48d01c1cb2ab807197450daae79d56c59a 100644 (file)
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1041,40 +1041,35 @@ char *usb_cache_string(struct usb_device *udev, int index)
 EXPORT_SYMBOL_GPL(usb_cache_string);
 
 /*
- * usb_get_device_descriptor - (re)reads the device descriptor (usbcore)
- * @dev: the device whose device descriptor is being updated
- * @size: how much of the descriptor to read
+ * usb_get_device_descriptor - read the device descriptor
+ * @udev: the device whose device descriptor should be read
  *
  * Context: task context, might sleep.
  *
- * Updates the copy of the device descriptor stored in the device structure,
- * which dedicates space for this purpose.
- *
  * Not exported, only for use by the core.  If drivers really want to read
  * the device descriptor directly, they can call usb_get_descriptor() with
  * type = USB_DT_DEVICE and index = 0.
  *
- * This call is synchronous, and may not be used in an interrupt context.
- *
- * Return: The number of bytes received on success, or else the status code
- * returned by the underlying usb_control_msg() call.
+ * Returns: a pointer to a dynamically allocated usb_device_descriptor
+ * structure (which the caller must deallocate), or an ERR_PTR value.
  */
-int usb_get_device_descriptor(struct usb_device *dev, unsigned int size)
+struct usb_device_descriptor *usb_get_device_descriptor(struct usb_device *udev)
 {
        struct usb_device_descriptor *desc;
        int ret;
 
-       if (size > sizeof(*desc))
-               return -EINVAL;
        desc = kmalloc(sizeof(*desc), GFP_NOIO);
        if (!desc)
-               return -ENOMEM;
+               return ERR_PTR(-ENOMEM);
+
+       ret = usb_get_descriptor(udev, USB_DT_DEVICE, 0, desc, sizeof(*desc));
+       if (ret == sizeof(*desc))
+               return desc;
 
-       ret = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, size);
        if (ret >= 0)
-               memcpy(&dev->descriptor, desc, size);
+               ret = -EMSGSIZE;
        kfree(desc);
-       return ret;
+       return ERR_PTR(ret);
 }
 
 /*

diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 69ca59841083b360a12243d15e2680b4d510b491..60363153fc3f38aed322182bc46357ca5ca6ec74 100644 (file)
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -43,8 +43,8 @@ extern bool usb_endpoint_is_ignored(struct usb_device *udev,
                struct usb_endpoint_descriptor *epd);
 extern int usb_remove_device(struct usb_device *udev);
 
-extern int usb_get_device_descriptor(struct usb_device *dev,
-               unsigned int size);
+extern struct usb_device_descriptor *usb_get_device_descriptor(
+               struct usb_device *udev);
 extern int usb_set_isoch_delay(struct usb_device *dev);
 extern int usb_get_bos_descriptor(struct usb_device *dev);
 extern void usb_release_bos_descriptor(struct usb_device *dev);