From 2f2af6028f201365e1fe9e2ec8ea3e5e62ddb82e Mon Sep 17 00:00:00 2001 From: Max Krummenacher Date: Tue, 20 Feb 2024 17:19:45 +0100 Subject: Revert "USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()" This reverts commit 7fe9d87996062f5eb0ca476ad0257f79bf43aaf5. Downstream NXP and stable have deviated to far, do not pull this in. Signed-off-by: Max Krummenacher --- drivers/usb/core/hub.c | 114 +++++++++++++++++++------------------------------ 1 file changed, 44 insertions(+), 70 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 5fd6ea435b1e..36f625aaf348 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4741,17 +4741,10 @@ static int get_bMaxPacketSize0(struct usb_device *udev, * the port lock. For a newly detected device that is not accessible * through any global pointers, it's not necessary to lock the device, * but it is still necessary to lock the port. - * - * For a newly detected device, @dev_descr must be NULL. The device - * descriptor retrieved from the device will then be stored in - * @udev->descriptor. For an already existing device, @dev_descr - * must be non-NULL. The device descriptor will be stored there, - * not in @udev->descriptor, because descriptors for registered - * devices are meant to be immutable. */ static int hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, - int retry_counter, struct usb_device_descriptor *dev_descr) + int retry_counter) { struct usb_device *hdev = hub->hdev; struct usb_hcd *hcd = bus_to_hcd(hdev->bus); @@ -4763,7 +4756,6 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, int devnum = udev->devnum; const char *driver_name; bool do_new_scheme; - const bool initial = !dev_descr; int maxp0; struct usb_device_descriptor *buf, *descr; @@ -4802,34 +4794,32 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, } oldspeed = udev->speed; - if (initial) { - /* USB 2.0 section 5.5.3 talks about ep0 maxpacket ... - * it's fixed size except for full speed devices. - * For Wireless USB devices, ep0 max packet is always 512 (tho - * reported as 0xff in the device descriptor). WUSB1.0[4.8.1]. + /* USB 2.0 section 5.5.3 talks about ep0 maxpacket ... + * it's fixed size except for full speed devices. + * For Wireless USB devices, ep0 max packet is always 512 (tho + * reported as 0xff in the device descriptor). WUSB1.0[4.8.1]. + */ + switch (udev->speed) { + case USB_SPEED_SUPER_PLUS: + case USB_SPEED_SUPER: + case USB_SPEED_WIRELESS: /* fixed at 512 */ + udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512); + break; + case USB_SPEED_HIGH: /* fixed at 64 */ + udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64); + break; + case USB_SPEED_FULL: /* 8, 16, 32, or 64 */ + /* to determine the ep0 maxpacket size, try to read + * the device descriptor to get bMaxPacketSize0 and + * then correct our initial guess. */ - switch (udev->speed) { - case USB_SPEED_SUPER_PLUS: - case USB_SPEED_SUPER: - case USB_SPEED_WIRELESS: /* fixed at 512 */ - udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512); - break; - case USB_SPEED_HIGH: /* fixed at 64 */ - udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64); - break; - case USB_SPEED_FULL: /* 8, 16, 32, or 64 */ - /* to determine the ep0 maxpacket size, try to read - * the device descriptor to get bMaxPacketSize0 and - * then correct our initial guess. - */ - udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64); - break; - case USB_SPEED_LOW: /* fixed at 8 */ - udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8); - break; - default: - goto fail; - } + udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64); + break; + case USB_SPEED_LOW: /* fixed at 8 */ + udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8); + break; + default: + goto fail; } if (udev->speed == USB_SPEED_WIRELESS) @@ -4852,24 +4842,22 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, if (udev->speed < USB_SPEED_SUPER) dev_info(&udev->dev, "%s %s USB device number %d using %s\n", - (initial ? "new" : "reset"), speed, + (udev->config) ? "reset" : "new", speed, devnum, driver_name); - if (initial) { - /* Set up TT records, if needed */ - if (hdev->tt) { - udev->tt = hdev->tt; - udev->ttport = hdev->ttport; - } else if (udev->speed != USB_SPEED_HIGH - && hdev->speed == USB_SPEED_HIGH) { - if (!hub->tt.hub) { - dev_err(&udev->dev, "parent hub has no TT\n"); - retval = -EINVAL; - goto fail; - } - udev->tt = &hub->tt; - udev->ttport = port1; + /* Set up TT records, if needed */ + if (hdev->tt) { + udev->tt = hdev->tt; + udev->ttport = hdev->ttport; + } else if (udev->speed != USB_SPEED_HIGH + && hdev->speed == USB_SPEED_HIGH) { + if (!hub->tt.hub) { + dev_err(&udev->dev, "parent hub has no TT\n"); + retval = -EINVAL; + goto fail; } + udev->tt = &hub->tt; + udev->ttport = port1; } /* Why interleave GET_DESCRIPTOR and SET_ADDRESS this way? @@ -4898,12 +4886,6 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, maxp0 = get_bMaxPacketSize0(udev, buf, GET_DESCRIPTOR_BUFSIZE, retries == 0); - if (maxp0 > 0 && !initial && - maxp0 != udev->descriptor.bMaxPacketSize0) { - dev_err(&udev->dev, "device reset changed ep0 maxpacket size!\n"); - retval = -ENODEV; - goto fail; - } retval = hub_port_reset(hub, port1, udev, delay, false); if (retval < 0) /* error or disconnect */ @@ -4977,12 +4959,6 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, } else { u32 delay; - if (!initial && maxp0 != udev->descriptor.bMaxPacketSize0) { - dev_err(&udev->dev, "device reset changed ep0 maxpacket size!\n"); - retval = -ENODEV; - goto fail; - } - delay = udev->parent->hub_delay; udev->hub_delay = min_t(u32, delay, USB_TP_TRANSMISSION_DELAY_MAX); @@ -5026,10 +5002,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, retval); goto fail; } - if (initial) - udev->descriptor = *descr; - else - *dev_descr = *descr; + udev->descriptor = *descr; kfree(descr); /* @@ -5332,7 +5305,7 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, } /* reset (non-USB 3.0 devices) and get descriptor */ - status = hub_port_init(hub, udev, port1, i, NULL); + status = hub_port_init(hub, udev, port1, i); if (status < 0) goto loop; @@ -5948,7 +5921,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev) struct usb_device *parent_hdev = udev->parent; struct usb_hub *parent_hub; struct usb_hcd *hcd = bus_to_hcd(udev->bus); - struct usb_device_descriptor descriptor; + struct usb_device_descriptor descriptor = udev->descriptor; struct usb_host_bos *bos; int i, j, ret = 0; int port1 = udev->portnum; @@ -5990,7 +5963,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev) /* ep0 maxpacket size may change; let the HCD know about it. * Other endpoints will be handled by re-enumeration. */ usb_ep0_reinit(udev); - ret = hub_port_init(parent_hub, udev, port1, i, &descriptor); + ret = hub_port_init(parent_hub, udev, port1, i); if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV) break; } @@ -6002,6 +5975,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev) /* Device might have changed firmware (DFU or similar) */ if (descriptors_changed(udev, &descriptor, bos)) { dev_info(&udev->dev, "device firmware changed\n"); + udev->descriptor = descriptor; /* for disconnect() calls */ goto re_enumerate; } -- cgit v1.2.3