From 6ae6dc22d2d1ce6aa77a6da8a761e61aca216f8b Mon Sep 17 00:00:00 2001 From: Mathias Nyman Date: Tue, 16 Nov 2021 00:16:30 +0200 Subject: usb: hub: Fix usb enumeration issue due to address0 race xHC hardware can only have one slot in default state with address 0 waiting for a unique address at a time, otherwise "undefined behavior may occur" according to xhci spec 5.4.3.4 The address0_mutex exists to prevent this across both xhci roothubs. If hub_port_init() fails, it may unlock the mutex and exit with a xhci slot in default state. If the other xhci roothub calls hub_port_init() at this point we end up with two slots in default state. Make sure the address0_mutex protects the slot default state across hub_port_init() retries, until slot is addressed or disabled. Note, one known minor case is not fixed by this patch. If device needs to be reset during resume, but fails all hub_port_init() retries in usb_reset_and_verify_device(), then it's possible the slot is still left in default state when address0_mutex is unlocked. Cc: Fixes: 638139eb95d2 ("usb: hub: allow to process more usb hub events in parallel") Signed-off-by: Mathias Nyman Link: https://lore.kernel.org/r/20211115221630.871204-1-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 86658a81d284..00c3506324e4 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4700,8 +4700,6 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, if (oldspeed == USB_SPEED_LOW) delay = HUB_LONG_RESET_TIME; - mutex_lock(hcd->address0_mutex); - /* Reset the device; full speed may morph to high speed */ /* FIXME a USB 2.0 device may morph into SuperSpeed on reset. */ retval = hub_port_reset(hub, port1, udev, delay, false); @@ -5016,7 +5014,6 @@ fail: hub_port_disable(hub, port1, 0); update_devnum(udev, devnum); /* for disconnect processing */ } - mutex_unlock(hcd->address0_mutex); return retval; } @@ -5246,6 +5243,9 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, unit_load = 100; status = 0; + + mutex_lock(hcd->address0_mutex); + for (i = 0; i < PORT_INIT_TRIES; i++) { /* reallocate for each attempt, since references @@ -5282,6 +5282,8 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, if (status < 0) goto loop; + mutex_unlock(hcd->address0_mutex); + if (udev->quirks & USB_QUIRK_DELAY_INIT) msleep(2000); @@ -5370,6 +5372,7 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, loop_disable: hub_port_disable(hub, port1, 1); + mutex_lock(hcd->address0_mutex); loop: usb_ep0_reinit(udev); release_devnum(udev); @@ -5396,6 +5399,8 @@ loop: } done: + mutex_unlock(hcd->address0_mutex); + hub_port_disable(hub, port1, 1); if (hcd->driver->relinquish_port && !hub->hdev->parent) { if (status != -ENOTCONN && status != -ENODEV) @@ -5915,6 +5920,8 @@ static int usb_reset_and_verify_device(struct usb_device *udev) bos = udev->bos; udev->bos = NULL; + mutex_lock(hcd->address0_mutex); + for (i = 0; i < PORT_INIT_TRIES; ++i) { /* ep0 maxpacket size may change; let the HCD know about it. @@ -5924,6 +5931,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev) if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV) break; } + mutex_unlock(hcd->address0_mutex); if (ret < 0) goto re_enumerate; -- cgit v1.2.3 From 6cca13de26eea6d32a98d96d916a048d16a12822 Mon Sep 17 00:00:00 2001 From: Mathias Nyman Date: Tue, 23 Nov 2021 12:16:56 +0200 Subject: usb: hub: Fix locking issues with address0_mutex Fix the circular lock dependency and unbalanced unlock of addess0_mutex introduced when fixing an address0_mutex enumeration retry race in commit ae6dc22d2d1 ("usb: hub: Fix usb enumeration issue due to address0 race") Make sure locking order between port_dev->status_lock and address0_mutex is correct, and that address0_mutex is not unlocked in hub_port_connect "done:" codepath which may be reached without locking address0_mutex Fixes: 6ae6dc22d2d1 ("usb: hub: Fix usb enumeration issue due to address0 race") Cc: Reported-by: Marek Szyprowski Tested-by: Hans de Goede Tested-by: Marek Szyprowski Acked-by: Hans de Goede Signed-off-by: Mathias Nyman Link: https://lore.kernel.org/r/20211123101656.1113518-1-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 00c3506324e4..00070a8a6507 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -5188,6 +5188,7 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, struct usb_port *port_dev = hub->ports[port1 - 1]; struct usb_device *udev = port_dev->child; static int unreliable_port = -1; + bool retry_locked; /* Disconnect any existing devices under this port */ if (udev) { @@ -5244,10 +5245,10 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, status = 0; - mutex_lock(hcd->address0_mutex); - for (i = 0; i < PORT_INIT_TRIES; i++) { - + usb_lock_port(port_dev); + mutex_lock(hcd->address0_mutex); + retry_locked = true; /* reallocate for each attempt, since references * to the previous one can escape in various ways */ @@ -5255,6 +5256,8 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, if (!udev) { dev_err(&port_dev->dev, "couldn't allocate usb_device\n"); + mutex_unlock(hcd->address0_mutex); + usb_unlock_port(port_dev); goto done; } @@ -5276,13 +5279,13 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, } /* reset (non-USB 3.0 devices) and get descriptor */ - usb_lock_port(port_dev); status = hub_port_init(hub, udev, port1, i); - usb_unlock_port(port_dev); if (status < 0) goto loop; mutex_unlock(hcd->address0_mutex); + usb_unlock_port(port_dev); + retry_locked = false; if (udev->quirks & USB_QUIRK_DELAY_INIT) msleep(2000); @@ -5372,11 +5375,14 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, loop_disable: hub_port_disable(hub, port1, 1); - mutex_lock(hcd->address0_mutex); loop: usb_ep0_reinit(udev); release_devnum(udev); hub_free_dev(udev); + if (retry_locked) { + mutex_unlock(hcd->address0_mutex); + usb_unlock_port(port_dev); + } usb_put_dev(udev); if ((status == -ENOTCONN) || (status == -ENOTSUPP)) break; @@ -5399,8 +5405,6 @@ loop: } done: - mutex_unlock(hcd->address0_mutex); - hub_port_disable(hub, port1, 1); if (hcd->driver->relinquish_port && !hub->hdev->parent) { if (status != -ENOTCONN && status != -ENODEV) -- cgit v1.2.3 From 49989adc38f8693fb6e9f019904dd00c1d1db5ac Mon Sep 17 00:00:00 2001 From: Ole Ernst Date: Sat, 27 Nov 2021 10:05:45 +0100 Subject: USB: NO_LPM quirk Lenovo Powered USB-C Travel Hub This is another branded 8153 device that doesn't work well with LPM: r8152 2-2.1:1.0 enp0s13f0u2u1: Stop submitting intr, status -71 Disable LPM to resolve the issue. Signed-off-by: Ole Ernst Signed-off-by: David S. Miller --- drivers/usb/core/quirks.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index 8239fe7129dd..019351c0b52c 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -434,6 +434,9 @@ static const struct usb_device_id usb_quirk_list[] = { { USB_DEVICE(0x1532, 0x0116), .driver_info = USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL }, + /* Lenovo Powered USB-C Travel Hub (4X90S92381, RTL8153 GigE) */ + { USB_DEVICE(0x17ef, 0x721e), .driver_info = USB_QUIRK_NO_LPM }, + /* Lenovo ThinkCenter A630Z TI024Gen3 usb-audio */ { USB_DEVICE(0x17ef, 0xa012), .driver_info = USB_QUIRK_DISCONNECT_SUSPEND }, -- cgit v1.2.3 From 1a3910c80966e4a76b25ce812f6bea0ef1b1d530 Mon Sep 17 00:00:00 2001 From: Pavel Hofman Date: Fri, 10 Dec 2021 09:52:18 +0100 Subject: usb: core: config: fix validation of wMaxPacketValue entries The checks performed by commit aed9d65ac327 ("USB: validate wMaxPacketValue entries in endpoint descriptors") require that initial value of the maxp variable contains both maximum packet size bits (10..0) and multiple-transactions bits (12..11). However, the existing code assings only the maximum packet size bits. This patch assigns all bits of wMaxPacketSize to the variable. Fixes: aed9d65ac327 ("USB: validate wMaxPacketValue entries in endpoint descriptors") Cc: stable Acked-by: Alan Stern Signed-off-by: Pavel Hofman Link: https://lore.kernel.org/r/20211210085219.16796-1-pavel.hofman@ivitera.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c index 16b1fd9dc60c..e3c3a73e1ed8 100644 --- a/drivers/usb/core/config.c +++ b/drivers/usb/core/config.c @@ -406,7 +406,7 @@ static int usb_parse_endpoint(struct device *ddev, int cfgno, * the USB-2 spec requires such endpoints to have wMaxPacketSize = 0 * (see the end of section 5.6.3), so don't warn about them. */ - maxp = usb_endpoint_maxp(&endpoint->desc); + maxp = le16_to_cpu(endpoint->desc.wMaxPacketSize); if (maxp == 0 && !(usb_endpoint_xfer_isoc(d) && asnum == 0)) { dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has invalid wMaxPacketSize 0\n", cfgno, inum, asnum, d->bEndpointAddress); -- cgit v1.2.3 From ca5737396927afd4d57b133fd2874bbcf3421cdb Mon Sep 17 00:00:00 2001 From: Pavel Hofman Date: Fri, 10 Dec 2021 09:52:19 +0100 Subject: usb: core: config: using bit mask instead of individual bits Using standard USB_EP_MAXP_MULT_MASK instead of individual bits for extracting multiple-transactions bits from wMaxPacketSize value. Acked-by: Alan Stern Signed-off-by: Pavel Hofman Link: https://lore.kernel.org/r/20211210085219.16796-2-pavel.hofman@ivitera.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c index e3c3a73e1ed8..48bc8a4814ac 100644 --- a/drivers/usb/core/config.c +++ b/drivers/usb/core/config.c @@ -422,9 +422,9 @@ static int usb_parse_endpoint(struct device *ddev, int cfgno, maxpacket_maxes = full_speed_maxpacket_maxes; break; case USB_SPEED_HIGH: - /* Bits 12..11 are allowed only for HS periodic endpoints */ + /* Multiple-transactions bits are allowed only for HS periodic endpoints */ if (usb_endpoint_xfer_int(d) || usb_endpoint_xfer_isoc(d)) { - i = maxp & (BIT(12) | BIT(11)); + i = maxp & USB_EP_MAXP_MULT_MASK; maxp &= ~i; } fallthrough; -- cgit v1.2.3