[PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

An outstanding syzbot bug report has been traced to a race between the
routine that reads in the device descriptor for a device being
reinitialized and the routine that writes the descriptors to a sysfs
attribute file.  The problem is that reinitializing a device, like
initializing it for the first time, stores the device descriptor
directly in the usb_device structure, where it may be accessed
concurrently as part of sending the descriptors to the sysfs reader.

This three-part series fixes the problem:

	The first patch unites the code paths responsible for first
	reading the device descriptor in hub.c's old scheme and new
	scheme, so that neither of them will call
	usb_get_device_descriptor().

	The second patch changes usb_get_device_descriptor(), making
	it return the descriptor in a dynamically allocated buffer
	rather than storing it directly in the device structure.

	The third patch changes hub_port_init(), adding a new argument
	that specifies a buffer in which to store the device
	descriptor for devices being reinitialized.

As a result of these changes, the copy of the device descriptor stored
in the usb_device structure will never be overwritten once it has been
initialized.  This eliminates the data race causing the bug identified
by syzbot.

It would be nice at some point to make a similar change to the code
that reads the device's BOS descriptor; reinitialization should not
overwrite its existing data either.  This series doesn't attempt to do
that, but it would be a good thing to do.

Alan Stern

[PATCH 1/3] USB: core: Unite old scheme and new scheme descriptor reads

[Alan Stern]

In preparation for reworking the usb_get_device_descriptor() routine,
it is desirable to unite the two different code paths responsible for
initially determining endpoint 0's maximum packet size in a newly
discovered USB device.  Making this determination presents a
chicken-and-egg sort of problem, in that the only way to learn the
maxpacket value is to get it from the device descriptor retrieved from
the device, but communicating with the device to retrieve a descriptor
requires us to know beforehand the ep0 maxpacket size.

In practice this problem is solved in two different ways, referred to
in hub.c as the "old scheme" and the "new scheme".  The old scheme
(which is the approach recommended by the USB-2 spec) involves asking
the device to send just the first eight bytes of its device
descriptor.  Such a transfer uses packets containing no more than
eight bytes each, and every USB device must have an ep0 maxpacket size
>= 8, so this should succeed.  Since the bMaxPacketSize0 field of the
device descriptor lies within the first eight bytes, this is all we
need.

The new scheme is an imitation of the technique used in an early
Windows USB implementation, giving it the happy advantage of working
with a wide variety of devices (some of them at the time would not
work with the old scheme, although that's probably less true now).  It
involves making an initial guess of the ep0 maxpacket size, asking the
device to send up to 64 bytes worth of its device descriptor (which is
only 18 bytes long), and then resetting the device to clear any error
condition that might have resulted from the guess being wrong.  The
initial guess is determined by the connection speed; it should be
correct in all cases other than full speed, for which the allowed
values are 8, 16, 32, and 64 (in this case the initial guess is 64).

The reason for this patch is that the old- and new-scheme parts of
hub_port_init() use different code paths, one involving
usb_get_device_descriptor() and one not, for their initial reads of
the device descriptor.  Since these reads have essentially the same
purpose and are made under essentially the same circumstances, this is
illogical.  It makes more sense to have both of them use a common
subroutine.

This subroutine does basically what the new scheme's code did, because
that approach is more general than the one used by the old scheme.  It
only needs to know how many bytes to transfer and whether or not it is
being called for the first iteration of a retry loop (in case of
certain time-out errors).  There are two main differences from the
former code:

	We initialize the bDescriptorType field of the transfer buffer
	to 0 before performing the transfer, to avoid possibly
	accessing an uninitialized value afterward.

	We read the device descriptor into a temporary buffer rather
	than storing it directly into udev->descriptor, which the old
	scheme implementation used to do.

Since the whole point of this first read of the device descriptor is
to determine the bMaxPacketSize0 value, that is what the new routine
returns (or an error code).  The value is stored in a local variable
rather than in udev->descriptor.  As a side effect, this necessitates
moving a section of code that checks the bcdUSB field for SuperSpeed
devices until after the full device descriptor has been retrieved.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: Oliver Neukum <oneukum@suse.com>

---

 drivers/usb/core/hub.c |  173 ++++++++++++++++++++++++++-----------------------
 1 file changed, 94 insertions(+), 79 deletions(-)

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4718,6 +4718,67 @@ static int hub_enable_device(struct usb_
 	return hcd->driver->enable_device(hcd, udev);
 }
 
+/*
+ * Get the bMaxPacketSize0 value during initialization by reading the
+ * device's device descriptor.  Since we don't already know this value,
+ * the transfer is unsafe and it ignores I/O errors, only testing for
+ * reasonable received values.
+ *
+ * For "old scheme" initialization, size will be 8 so we read just the
+ * start of the device descriptor, which should work okay regardless of
+ * the actual bMaxPacketSize0 value.  For "new scheme" initialization,
+ * size will be 64 (and buf will point to a sufficiently large buffer),
+ * which might not be kosher according to the USB spec but it's what
+ * Windows does and what many devices expect.
+ *
+ * Returns: bMaxPacketSize0 or a negative error code.
+ */
+static int get_bMaxPacketSize0(struct usb_device *udev,
+		struct usb_device_descriptor *buf, int size, bool first_time)
+{
+	int i, rc;
+
+	/*
+	 * Retry on all errors; some devices are flakey.
+	 * 255 is for WUSB devices, we actually need to use
+	 * 512 (WUSB1.0[4.8.1]).
+	 */
+	for (i = 0; i < GET_MAXPACKET0_TRIES; ++i) {
+		/* Start with invalid values in case the transfer fails */
+		buf->bDescriptorType = buf->bMaxPacketSize0 = 0;
+		rc = usb_control_msg(udev, usb_rcvaddr0pipe(),
+				USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
+				USB_DT_DEVICE << 8, 0,
+				buf, size,
+				initial_descriptor_timeout);
+		switch (buf->bMaxPacketSize0) {
+		case 8: case 16: case 32: case 64: case 255:
+			if (buf->bDescriptorType == USB_DT_DEVICE) {
+				rc = buf->bMaxPacketSize0;
+				break;
+			}
+			fallthrough;
+		default:
+			if (rc >= 0)
+				rc = -EPROTO;
+			break;
+		}
+
+		/*
+		 * Some devices time out if they are powered on
+		 * when already connected. They need a second
+		 * reset, so return early. But only on the first
+		 * attempt, lest we get into a time-out/reset loop.
+		 */
+		if (rc > 0 || (rc == -ETIMEDOUT && first_time &&
+				udev->speed > USB_SPEED_FULL))
+			break;
+	}
+	return rc;
+}
+
+#define GET_DESCRIPTOR_BUFSIZE	64
+
 /* Reset device, (re)assign address, get device descriptor.
  * Device connection must be stable, no more debouncing needed.
  * Returns device in USB_STATE_ADDRESS, except on error.
@@ -4742,6 +4803,12 @@ hub_port_init(struct usb_hub *hub, struc
 	int			devnum = udev->devnum;
 	const char		*driver_name;
 	bool			do_new_scheme;
+	int			maxp0;
+	struct usb_device_descriptor	*buf;
+
+	buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
+	if (!buf)
+		return -ENOMEM;
 
 	/* root hub ports have a slightly longer reset period
 	 * (from USB 2.0 spec, section 7.1.7.5)
@@ -4861,9 +4928,6 @@ hub_port_init(struct usb_hub *hub, struc
 		}
 
 		if (do_new_scheme) {
-			struct usb_device_descriptor *buf;
-			int r = 0;
-
 			retval = hub_enable_device(udev);
 			if (retval < 0) {
 				dev_err(&udev->dev,
@@ -4872,52 +4936,8 @@ hub_port_init(struct usb_hub *hub, struc
 				goto fail;
 			}
 
-#define GET_DESCRIPTOR_BUFSIZE	64
-			buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
-			if (!buf) {
-				retval = -ENOMEM;
-				continue;
-			}
-
-			/* Retry on all errors; some devices are flakey.
-			 * 255 is for WUSB devices, we actually need to use
-			 * 512 (WUSB1.0[4.8.1]).
-			 */
-			for (operations = 0; operations < GET_MAXPACKET0_TRIES;
-					++operations) {
-				buf->bMaxPacketSize0 = 0;
-				r = usb_control_msg(udev, usb_rcvaddr0pipe(),
-					USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
-					USB_DT_DEVICE << 8, 0,
-					buf, GET_DESCRIPTOR_BUFSIZE,
-					initial_descriptor_timeout);
-				switch (buf->bMaxPacketSize0) {
-				case 8: case 16: case 32: case 64: case 255:
-					if (buf->bDescriptorType ==
-							USB_DT_DEVICE) {
-						r = 0;
-						break;
-					}
-					fallthrough;
-				default:
-					if (r == 0)
-						r = -EPROTO;
-					break;
-				}
-				/*
-				 * Some devices time out if they are powered on
-				 * when already connected. They need a second
-				 * reset. But only on the first attempt,
-				 * lest we get into a time out/reset loop
-				 */
-				if (r == 0 || (r == -ETIMEDOUT &&
-						retries == 0 &&
-						udev->speed > USB_SPEED_FULL))
-					break;
-			}
-			udev->descriptor.bMaxPacketSize0 =
-					buf->bMaxPacketSize0;
-			kfree(buf);
+			maxp0 = get_bMaxPacketSize0(udev, buf,
+					GET_DESCRIPTOR_BUFSIZE, retries == 0);
 
 			retval = hub_port_reset(hub, port1, udev, delay, false);
 			if (retval < 0)		/* error or disconnect */
@@ -4928,14 +4948,13 @@ hub_port_init(struct usb_hub *hub, struc
 				retval = -ENODEV;
 				goto fail;
 			}
-			if (r) {
-				if (r != -ENODEV)
+			if (maxp0 < 0) {
+				if (maxp0 != -ENODEV)
 					dev_err(&udev->dev, "device descriptor read/64, error %d\n",
-							r);
-				retval = -EMSGSIZE;
+							maxp0);
+				retval = maxp0;
 				continue;
 			}
-#undef GET_DESCRIPTOR_BUFSIZE
 		}
 
 		/*
@@ -4981,19 +5000,17 @@ hub_port_init(struct usb_hub *hub, struc
 				break;
 		}
 
-		retval = usb_get_device_descriptor(udev, 8);
-		if (retval < 8) {
+		/* !do_new_scheme || wusb */
+		maxp0 = get_bMaxPacketSize0(udev, buf, 8, retries == 0);
+		if (maxp0 < 0) {
+			retval = maxp0;
 			if (retval != -ENODEV)
 				dev_err(&udev->dev,
 					"device descriptor read/8, error %d\n",
 					retval);
-			if (retval >= 0)
-				retval = -EMSGSIZE;
 		} else {
 			u32 delay;
 
-			retval = 0;
-
 			delay = udev->parent->hub_delay;
 			udev->hub_delay = min_t(u32, delay,
 						USB_TP_TRANSMISSION_DELAY_MAX);
@@ -5010,27 +5027,10 @@ hub_port_init(struct usb_hub *hub, struc
 	if (retval)
 		goto fail;
 
-	/*
-	 * Some superspeed devices have finished the link training process
-	 * and attached to a superspeed hub port, but the device descriptor
-	 * got from those devices show they aren't superspeed devices. Warm
-	 * reset the port attached by the devices can fix them.
-	 */
-	if ((udev->speed >= USB_SPEED_SUPER) &&
-			(le16_to_cpu(udev->descriptor.bcdUSB) < 0x0300)) {
-		dev_err(&udev->dev, "got a wrong device descriptor, "
-				"warm reset device\n");
-		hub_port_reset(hub, port1, udev,
-				HUB_BH_RESET_TIME, true);
-		retval = -EINVAL;
-		goto fail;
-	}
-
-	if (udev->descriptor.bMaxPacketSize0 == 0xff ||
-			udev->speed >= USB_SPEED_SUPER)
+	if (maxp0 == 0xff || udev->speed >= USB_SPEED_SUPER)
 		i = 512;
 	else
-		i = udev->descriptor.bMaxPacketSize0;
+		i = maxp0;
 	if (usb_endpoint_maxp(&udev->ep0.desc) != i) {
 		if (udev->speed == USB_SPEED_LOW ||
 				!(i == 8 || i == 16 || i == 32 || i == 64)) {
@@ -5056,6 +5056,20 @@ hub_port_init(struct usb_hub *hub, struc
 		goto fail;
 	}
 
+	/*
+	 * Some superspeed devices have finished the link training process
+	 * and attached to a superspeed hub port, but the device descriptor
+	 * got from those devices show they aren't superspeed devices. Warm
+	 * reset the port attached by the devices can fix them.
+	 */
+	if ((udev->speed >= USB_SPEED_SUPER) &&
+			(le16_to_cpu(udev->descriptor.bcdUSB) < 0x0300)) {
+		dev_err(&udev->dev, "got a wrong device descriptor, warm reset device\n");
+		hub_port_reset(hub, port1, udev, HUB_BH_RESET_TIME, true);
+		retval = -EINVAL;
+		goto fail;
+	}
+
 	usb_detect_quirks(udev);
 
 	if (udev->wusb == 0 && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201) {
@@ -5078,6 +5092,7 @@ fail:
 		hub_port_disable(hub, port1, 0);
 		update_devnum(udev, devnum);	/* for disconnect processing */
 	}
+	kfree(buf);
 	return retval;
 }
 

[PATCH 2/3] USB: core: Change usb_get_device_descriptor() API

[Alan Stern]

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>

---

 drivers/usb/core/hcd.c     |   10 +++++++---
 drivers/usb/core/hub.c     |   44 +++++++++++++++++++++++---------------------
 drivers/usb/core/message.c |   29 ++++++++++++-----------------
 drivers/usb/core/usb.h     |    4 ++--
 4 files changed, 44 insertions(+), 43 deletions(-)

Index: usb-devel/drivers/usb/core/hcd.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hcd.c
+++ usb-devel/drivers/usb/core/hcd.c
@@ -983,6 +983,7 @@ static int register_root_hub(struct usb_
 {
 	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_
 	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);
Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -2671,12 +2671,17 @@ int usb_authorize_device(struct usb_devi
 	}
 
 	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;
@@ -4804,7 +4809,7 @@ hub_port_init(struct usb_hub *hub, struc
 	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)
@@ -5046,15 +5051,16 @@ hub_port_init(struct usb_hub *hub, struc
 		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
@@ -5173,7 +5179,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;
@@ -5184,8 +5190,8 @@ static int descriptors_changed(struct us
 	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))
@@ -5510,9 +5516,8 @@ static void hub_port_connect_change(stru
 {
 	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));
@@ -5539,23 +5544,20 @@ static void hub_port_connect_change(stru
 			 * 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 &&
Index: usb-devel/drivers/usb/core/message.c
===================================================================
--- usb-devel.orig/drivers/usb/core/message.c
+++ usb-devel/drivers/usb/core/message.c
@@ -1040,40 +1040,35 @@ char *usb_cache_string(struct usb_device
 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);
 }
 
 /*
Index: usb-devel/drivers/usb/core/usb.h
===================================================================
--- usb-devel.orig/drivers/usb/core/usb.h
+++ usb-devel/drivers/usb/core/usb.h
@@ -43,8 +43,8 @@ extern bool usb_endpoint_is_ignored(stru
 		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);

[PATCH 3/3] USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()

[Alan Stern]

Syzbot reported an out-of-bounds read in sysfs.c:read_descriptors():


BUG: KASAN: slab-out-of-bounds in read_descriptors+0x263/0x280 drivers/usb/core/sysfs.c:883
Read of size 8 at addr ffff88801e78b8c8 by task udevd/5011

CPU: 0 PID: 5011 Comm: udevd Not tainted 6.4.0-rc6-syzkaller-00195-g40f71e7cd3c6 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/27/2023
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xd9/0x150 lib/dump_stack.c:106
 print_address_description.constprop.0+0x2c/0x3c0 mm/kasan/report.c:351
 print_report mm/kasan/report.c:462 [inline]
 kasan_report+0x11c/0x130 mm/kasan/report.c:572
 read_descriptors+0x263/0x280 drivers/usb/core/sysfs.c:883
...
Allocated by task 758:
...
 __do_kmalloc_node mm/slab_common.c:966 [inline]
 __kmalloc+0x5e/0x190 mm/slab_common.c:979
 kmalloc include/linux/slab.h:563 [inline]
 kzalloc include/linux/slab.h:680 [inline]
 usb_get_configuration+0x1f7/0x5170 drivers/usb/core/config.c:887
 usb_enumerate_device drivers/usb/core/hub.c:2407 [inline]
 usb_new_device+0x12b0/0x19d0 drivers/usb/core/hub.c:2545


As analyzed by Khazhy Kumykov, the cause of this bug is a race between
read_descriptors() and hub_port_init(): The first routine uses a field
in udev->descriptor, not expecting it to change, while the second
overwrites it.

Prior to commit 45bf39f8df7f ("USB: core: Don't hold device lock while
reading the "descriptors" sysfs file") this race couldn't occur,
because the routines were mutually exclusive thanks to the device
locking.  Removing that locking from read_descriptors() exposed it to
the race.

The best way to fix the bug is to keep hub_port_init() from changing
udev->descriptor once udev has been initialized and registered.
Drivers expect the descriptors stored in the kernel to be immutable;
we should not undermine this expectation.  In fact, this change should
have been made long ago.

So now hub_port_init() will take an additional argument, specifying a
buffer in which to store the device descriptor it reads.  (If udev has
not yet been initialized, the buffer pointer will be NULL and then
hub_port_init() will store the device descriptor in udev as before.)
This eliminates the data race responsible for the out-of-bounds read.

The changes to hub_port_init() appear more extensive than they really
are, because of indentation changes resulting from an attempt to avoid
writing to other parts of the usb_device structure after it has been
initialized.  Similar changes should be made to the code that reads
the BOS descriptor, but that can be handled in a separate patch later
on.  This patch is sufficient to fix the bug found by syzbot.

Reported-and-tested-by: syzbot+18996170f8096c6174d0@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-usb/000000000000c0ffe505fe86c9ca@google.com/#r
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: Khazhy Kumykov <khazhy@google.com>
Fixes: 45bf39f8df7f ("USB: core: Don't hold device lock while reading the "descriptors" sysfs file")
Cc: <stable@vger.kernel.org>

---

 drivers/usb/core/hub.c |  114 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 70 insertions(+), 44 deletions(-)

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4793,10 +4793,17 @@ static int get_bMaxPacketSize0(struct us
  * 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)
+		int retry_counter, struct usb_device_descriptor *dev_descr)
 {
 	struct usb_device	*hdev = hub->hdev;
 	struct usb_hcd		*hcd = bus_to_hcd(hdev->bus);
@@ -4808,6 +4815,7 @@ hub_port_init(struct usb_hub *hub, struc
 	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;
 
@@ -4846,32 +4854,34 @@ hub_port_init(struct usb_hub *hub, struc
 	}
 	oldspeed = udev->speed;
 
-	/* 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.
+	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].
 		 */
-		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;
+		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;
+		}
 	}
 
 	if (udev->speed == USB_SPEED_WIRELESS)
@@ -4894,22 +4904,24 @@ hub_port_init(struct usb_hub *hub, struc
 	if (udev->speed < USB_SPEED_SUPER)
 		dev_info(&udev->dev,
 				"%s %s USB device number %d using %s\n",
-				(udev->config) ? "reset" : "new", speed,
+				(initial ? "new" : "reset"), speed,
 				devnum, driver_name);
 
-	/* 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;
+	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;
 		}
-		udev->tt = &hub->tt;
-		udev->ttport = port1;
 	}
 
 	/* Why interleave GET_DESCRIPTOR and SET_ADDRESS this way?
@@ -4943,6 +4955,12 @@ hub_port_init(struct usb_hub *hub, struc
 
 			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 */
@@ -5016,6 +5034,12 @@ hub_port_init(struct usb_hub *hub, struc
 		} 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);
@@ -5059,7 +5083,10 @@ hub_port_init(struct usb_hub *hub, struc
 					retval);
 		goto fail;
 	}
-	udev->descriptor = *descr;
+	if (initial)
+		udev->descriptor = *descr;
+	else
+		*dev_descr = *descr;
 	kfree(descr);
 
 	/*
@@ -5369,7 +5396,7 @@ static void hub_port_connect(struct usb_
 		}
 
 		/* reset (non-USB 3.0 devices) and get descriptor */
-		status = hub_port_init(hub, udev, port1, i);
+		status = hub_port_init(hub, udev, port1, i, NULL);
 		if (status < 0)
 			goto loop;
 
@@ -5999,7 +6026,7 @@ static int usb_reset_and_verify_device(s
 	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 = udev->descriptor;
+	struct usb_device_descriptor	descriptor;
 	struct usb_host_bos		*bos;
 	int				i, j, ret = 0;
 	int				port1 = udev->portnum;
@@ -6035,7 +6062,7 @@ static int usb_reset_and_verify_device(s
 		/* 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);
+		ret = hub_port_init(parent_hub, udev, port1, i, &descriptor);
 		if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
 			break;
 	}
@@ -6047,7 +6074,6 @@ static int usb_reset_and_verify_device(s
 	/* 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;
 	}
 

[REGRESSION] Re: [PATCH 1/3] USB: core: Unite old scheme and new scheme descriptor reads

[Christian Eggers]

> In preparation for reworking the usb_get_device_descriptor() routine,
> it is desirable to unite the two different code paths responsible for
> initially determining endpoint 0's maximum packet size in a newly
> discovered USB device.  Making this determination presents a
> chicken-and-egg sort of problem, in that the only way to learn the
> maxpacket value is to get it from the device descriptor retrieved from
> the device, but communicating with the device to retrieve a descriptor
> requires us to know beforehand the ep0 maxpacket size.
> 
> In practice this problem is solved in two different ways, referred to
> in hub.c as the "old scheme" and the "new scheme".  The old scheme
> (which is the approach recommended by the USB-2 spec) involves asking
> the device to send just the first eight bytes of its device
> descriptor.  Such a transfer uses packets containing no more than
> eight bytes each, and every USB device must have an ep0 maxpacket size
> 
> >= 8, so this should succeed.  Since the bMaxPacketSize0 field of the
> 
> device descriptor lies within the first eight bytes, this is all we
> need.
> 
> The new scheme is an imitation of the technique used in an early
> Windows USB implementation, giving it the happy advantage of working
> with a wide variety of devices (some of them at the time would not
> work with the old scheme, although that's probably less true now).  It
> involves making an initial guess of the ep0 maxpacket size, asking the
> device to send up to 64 bytes worth of its device descriptor (which is
> only 18 bytes long), and then resetting the device to clear any error
> condition that might have resulted from the guess being wrong.  The
> initial guess is determined by the connection speed; it should be
> correct in all cases other than full speed, for which the allowed
> values are 8, 16, 32, and 64 (in this case the initial guess is 64).
> 
> The reason for this patch is that the old- and new-scheme parts of
> hub_port_init() use different code paths, one involving
> usb_get_device_descriptor() and one not, for their initial reads of
> the device descriptor.  Since these reads have essentially the same
> purpose and are made under essentially the same circumstances, this is
> illogical.  It makes more sense to have both of them use a common
> subroutine.
> 
> This subroutine does basically what the new scheme's code did, because
> that approach is more general than the one used by the old scheme.  It
> only needs to know how many bytes to transfer and whether or not it is
> being called for the first iteration of a retry loop (in case of
> certain time-out errors).  There are two main differences from the
> 
> former code:
> 	We initialize the bDescriptorType field of the transfer buffer
> 	to 0 before performing the transfer, to avoid possibly
> 	accessing an uninitialized value afterward.
> 	
> 	We read the device descriptor into a temporary buffer rather
> 	than storing it directly into udev->descriptor, which the old
> 	scheme implementation used to do.
> 
> Since the whole point of this first read of the device descriptor is
> to determine the bMaxPacketSize0 value, that is what the new routine
> returns (or an error code).  The value is stored in a local variable
> rather than in udev->descriptor.  As a side effect, this necessitates
> moving a section of code that checks the bcdUSB field for SuperSpeed
> devices until after the full device descriptor has been retrieved.
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Cc: Oliver Neukum <oneukum@suse.com>

Last week I upgraded within the 5.15-stable branch. Since upstream commit
85d07c556216 ("USB: core: Unite old scheme and new scheme descriptor reads"),
there are problems detecting a directly attached USB hub. I identified this
commit by bisecting and get the same result during upgrading within the 6.1-stable
branch.

Hardware: ARMv7 NXP i.MX6ULL with directly attached USB hub (Microchip USB4916).
Log messages:

[    6.150881] usb 1-1: new high-speed USB device number 2 using ci_hdrc
[    6.215484] usb 1-1: device descriptor read/8, error -71
[    6.377532] usb 1-1: device descriptor read/8, error -71
[    6.581934] usb 1-1: new high-speed USB device number 3 using ci_hdrc
[    6.642853] usb 1-1: device descriptor read/8, error -71
[    6.803355] usb 1-1: device descriptor read/8, error -71
[    6.920418] usb usb1-port1: attempt power cycle
[    7.051419] usb 1-1: new high-speed USB device number 4 using ci_hdrc
[    7.192320] usb 1-1: New USB device found, idVendor=0424, idProduct=4916, bcdDevice= 8.02
[    7.192348] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[    7.192363] usb 1-1: Product: Orbiter USB hub
[    7.192376] usb 1-1: Manufacturer: ARRI
[    7.193263] hub 1-1:1.0: USB hub found
[    7.193951] hub 1-1:1.0: 7 ports detected

The "device descriptor read" and "attempt power cycle" error messages definitely
haven't been there before the commit mentioned above. Disregarding the additional
log messages, the USB hub (and its devices) seem to work.

I didn't try reverting this single commit as it seems that later changes depends
on it.

regards
Christian

Re: [REGRESSION] Re: [PATCH 1/3] USB: core: Unite old scheme and new scheme descriptor reads

[Alan Stern]

On Mon, Dec 11, 2023 at 11:40:38AM +0100, Christian Eggers wrote:
> Last week I upgraded within the 5.15-stable branch. Since upstream commit
> 85d07c556216 ("USB: core: Unite old scheme and new scheme descriptor reads"),
> there are problems detecting a directly attached USB hub. I identified this
> commit by bisecting and get the same result during upgrading within the 6.1-stable
> branch.
> 
> Hardware: ARMv7 NXP i.MX6ULL with directly attached USB hub (Microchip USB4916).
> Log messages:
> 
> [    6.150881] usb 1-1: new high-speed USB device number 2 using ci_hdrc
> [    6.215484] usb 1-1: device descriptor read/8, error -71
> [    6.377532] usb 1-1: device descriptor read/8, error -71

Is the old_scheme_first module parameter for the usbcore module set?  

That and the USB_PORT_QUIRK_OLD_SCHEME are the only things I can see 
which would cause those errors -- and apparently this quirk never gets 
set in current kernels.

In particular, I'm not aware of anything in the commit which would cause 
this sort of change in behavior.

> [    6.581934] usb 1-1: new high-speed USB device number 3 using ci_hdrc
> [    6.642853] usb 1-1: device descriptor read/8, error -71
> [    6.803355] usb 1-1: device descriptor read/8, error -71
> [    6.920418] usb usb1-port1: attempt power cycle
> [    7.051419] usb 1-1: new high-speed USB device number 4 using ci_hdrc
> [    7.192320] usb 1-1: New USB device found, idVendor=0424, idProduct=4916, bcdDevice= 8.02
> [    7.192348] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> [    7.192363] usb 1-1: Product: Orbiter USB hub
> [    7.192376] usb 1-1: Manufacturer: ARRI
> [    7.193263] hub 1-1:1.0: USB hub found
> [    7.193951] hub 1-1:1.0: 7 ports detected
> 
> The "device descriptor read" and "attempt power cycle" error messages definitely
> haven't been there before the commit mentioned above.

Are you certain of this?  That is, have you tried booting your current 
system with an earlier kernel?  And have you tried comparing logs and 
usbmon traces for kernels with and without the commit?

The very best test would be to run a kernel _at_ that commit (i.e., with 
no later commits installed) and then revert the commit and see how the 
resulting kernel behaves.  In other words, compare two kernels whose 
only difference is that one commit.

>  Disregarding the additional
> log messages, the USB hub (and its devices) seem to work.
> 
> I didn't try reverting this single commit as it seems that later changes depends
> on it.

Yeah; if there really are problems they will be fixed by updating that 
commit, not by dropping it.

Alan Stern

Re: [REGRESSION] Re: [PATCH 1/3] USB: core: Unite old scheme and new scheme descriptor reads

[Christian Eggers]

Hi Alan,

On Monday, 11 December 2023, 17:21:56 CET, Alan Stern wrote:
> On Mon, Dec 11, 2023 at 11:40:38AM +0100, Christian Eggers wrote:
> > Last week I upgraded within the 5.15-stable branch. Since upstream commit
> > 85d07c556216 ("USB: core: Unite old scheme and new scheme descriptor reads"),
> > there are problems detecting a directly attached USB hub. I identified this
> > commit by bisecting and get the same result during upgrading within the 6.1-stable
> > branch.
> > 
> > Hardware: ARMv7 NXP i.MX6ULL with directly attached USB hub (Microchip USB4916).
> > Log messages:
> > 
> > [    6.150881] usb 1-1: new high-speed USB device number 2 using ci_hdrc
> > [    6.215484] usb 1-1: device descriptor read/8, error -71
> > [    6.377532] usb 1-1: device descriptor read/8, error -71
> 
> Is the old_scheme_first module parameter for the usbcore module set?  
> 
> That and the USB_PORT_QUIRK_OLD_SCHEME are the only things I can see 
> which would cause those errors -- and apparently this quirk never gets 
> set in current kernels.
that was the problem! In previous attempts to reduce the boot time of my system
I have set USB_PORT_QUIRK_OLD_SCHEME for specific combinations of bus/port numbers.

After reverting this, the warnings don't appear anymore, so everything is fine now.

Thanks for identifying the problem and sorry for the noise.

regards,
Christian

Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Greg KH]

On Fri, Aug 04, 2023 at 03:09:04PM -0400, Alan Stern wrote:
> An outstanding syzbot bug report has been traced to a race between the
> routine that reads in the device descriptor for a device being
> reinitialized and the routine that writes the descriptors to a sysfs
> attribute file.  The problem is that reinitializing a device, like
> initializing it for the first time, stores the device descriptor
> directly in the usb_device structure, where it may be accessed
> concurrently as part of sending the descriptors to the sysfs reader.
> 
> This three-part series fixes the problem:
> 
> 	The first patch unites the code paths responsible for first
> 	reading the device descriptor in hub.c's old scheme and new
> 	scheme, so that neither of them will call
> 	usb_get_device_descriptor().
> 
> 	The second patch changes usb_get_device_descriptor(), making
> 	it return the descriptor in a dynamically allocated buffer
> 	rather than storing it directly in the device structure.
> 
> 	The third patch changes hub_port_init(), adding a new argument
> 	that specifies a buffer in which to store the device
> 	descriptor for devices being reinitialized.
> 
> As a result of these changes, the copy of the device descriptor stored
> in the usb_device structure will never be overwritten once it has been
> initialized.  This eliminates the data race causing the bug identified
> by syzbot.
> 
> It would be nice at some point to make a similar change to the code
> that reads the device's BOS descriptor; reinitialization should not
> overwrite its existing data either.  This series doesn't attempt to do
> that, but it would be a good thing to do.

Thanks for cleaning this up, all now applied.

greg k-h

Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Thinh Nguyen]

Hi Alan,

On Fri, Aug 04, 2023, Alan Stern wrote:
> An outstanding syzbot bug report has been traced to a race between the
> routine that reads in the device descriptor for a device being
> reinitialized and the routine that writes the descriptors to a sysfs
> attribute file.  The problem is that reinitializing a device, like
> initializing it for the first time, stores the device descriptor
> directly in the usb_device structure, where it may be accessed
> concurrently as part of sending the descriptors to the sysfs reader.
> 
> This three-part series fixes the problem:
> 
> 	The first patch unites the code paths responsible for first
> 	reading the device descriptor in hub.c's old scheme and new
> 	scheme, so that neither of them will call
> 	usb_get_device_descriptor().
> 
> 	The second patch changes usb_get_device_descriptor(), making
> 	it return the descriptor in a dynamically allocated buffer
> 	rather than storing it directly in the device structure.
> 
> 	The third patch changes hub_port_init(), adding a new argument
> 	that specifies a buffer in which to store the device
> 	descriptor for devices being reinitialized.
> 
> As a result of these changes, the copy of the device descriptor stored
> in the usb_device structure will never be overwritten once it has been
> initialized.  This eliminates the data race causing the bug identified
> by syzbot.
> 
> It would be nice at some point to make a similar change to the code
> that reads the device's BOS descriptor; reinitialization should not
> overwrite its existing data either.  This series doesn't attempt to do
> that, but it would be a good thing to do.
> 

Testing from Greg's usb-next branch, this series causes problem with
device enumeration:

[   31.650759] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 2 using xhci_hcd
[   31.663107] usb 2-1: device descriptor read/8, error -71
[   31.952697] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 3 using xhci_hcd
[   31.965122] usb 2-1: device descriptor read/8, error -71
[   32.080991] usb usb2-port1: attempt power cycle
[   34.826893] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 5 using xhci_hcd
[   34.839241] usb 2-1: device descriptor read/8, error -71
[   35.129908] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 6 using xhci_hcd
[   35.142264] usb 2-1: device descriptor read/8, error -71
[   35.257155] usb usb2-port1: attempt power cycle
[   37.258922] usb 1-1: new high-speed USB device number 5 using xhci_hcd
[   38.115053] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 8 using xhci_hcd
[   38.127407] usb 2-1: device descriptor read/8, error -71
[   38.417066] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 9 using xhci_hcd
[   38.429428] usb 2-1: device descriptor read/8, error -71
[   38.545315] usb usb2-port1: attempt power cycle
[   43.347312] usb 2-2: new SuperSpeed Plus Gen 2x1 USB device number 11 using xhci_hcd
[   43.359659] usb 2-2: device descriptor read/8, error -71
[   43.649323] usb 2-2: new SuperSpeed Plus Gen 2x1 USB device number 12 using xhci_hcd
[   43.661681] usb 2-2: device descriptor read/8, error -71
[   43.777566] usb usb2-port2: attempt power cycle

I was testing with our host along with other common vendor hosts with a
few 3.x devices. Reverting this series resolves the issue. I didn't do
extensive tests for various speeds or debug it. I just want to notify
this first perhaps you can figure out the issue right away.

I can look into it and provide more info later this week. If you want to
print any debug, let me know and I can provide later this week.

Thanks,
Thinh

Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Thu, Aug 10, 2023 at 12:28:27AM +0000, Thinh Nguyen wrote:
> Hi Alan,
> 
> On Fri, Aug 04, 2023, Alan Stern wrote:
> > An outstanding syzbot bug report has been traced to a race between the
> > routine that reads in the device descriptor for a device being
> > reinitialized and the routine that writes the descriptors to a sysfs
> > attribute file.  The problem is that reinitializing a device, like
> > initializing it for the first time, stores the device descriptor
> > directly in the usb_device structure, where it may be accessed
> > concurrently as part of sending the descriptors to the sysfs reader.
> > 
> > This three-part series fixes the problem:
> > 
> > 	The first patch unites the code paths responsible for first
> > 	reading the device descriptor in hub.c's old scheme and new
> > 	scheme, so that neither of them will call
> > 	usb_get_device_descriptor().
> > 
> > 	The second patch changes usb_get_device_descriptor(), making
> > 	it return the descriptor in a dynamically allocated buffer
> > 	rather than storing it directly in the device structure.
> > 
> > 	The third patch changes hub_port_init(), adding a new argument
> > 	that specifies a buffer in which to store the device
> > 	descriptor for devices being reinitialized.
> > 
> > As a result of these changes, the copy of the device descriptor stored
> > in the usb_device structure will never be overwritten once it has been
> > initialized.  This eliminates the data race causing the bug identified
> > by syzbot.
> > 
> > It would be nice at some point to make a similar change to the code
> > that reads the device's BOS descriptor; reinitialization should not
> > overwrite its existing data either.  This series doesn't attempt to do
> > that, but it would be a good thing to do.
> > 
> 
> Testing from Greg's usb-next branch, this series causes problem with
> device enumeration:
> 
> [   31.650759] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 2 using xhci_hcd
> [   31.663107] usb 2-1: device descriptor read/8, error -71
> [   31.952697] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 3 using xhci_hcd
> [   31.965122] usb 2-1: device descriptor read/8, error -71
> [   32.080991] usb usb2-port1: attempt power cycle
> [   34.826893] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 5 using xhci_hcd
> [   34.839241] usb 2-1: device descriptor read/8, error -71
> [   35.129908] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 6 using xhci_hcd
> [   35.142264] usb 2-1: device descriptor read/8, error -71
> [   35.257155] usb usb2-port1: attempt power cycle
> [   37.258922] usb 1-1: new high-speed USB device number 5 using xhci_hcd
> [   38.115053] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 8 using xhci_hcd
> [   38.127407] usb 2-1: device descriptor read/8, error -71
> [   38.417066] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 9 using xhci_hcd
> [   38.429428] usb 2-1: device descriptor read/8, error -71
> [   38.545315] usb usb2-port1: attempt power cycle
> [   43.347312] usb 2-2: new SuperSpeed Plus Gen 2x1 USB device number 11 using xhci_hcd
> [   43.359659] usb 2-2: device descriptor read/8, error -71
> [   43.649323] usb 2-2: new SuperSpeed Plus Gen 2x1 USB device number 12 using xhci_hcd
> [   43.661681] usb 2-2: device descriptor read/8, error -71
> [   43.777566] usb usb2-port2: attempt power cycle
> 
> I was testing with our host along with other common vendor hosts with a
> few 3.x devices. Reverting this series resolves the issue. I didn't do
> extensive tests for various speeds or debug it. I just want to notify
> this first perhaps you can figure out the issue right away.
> 
> I can look into it and provide more info later this week. If you want to
> print any debug, let me know and I can provide later this week.

Thanks for the notification.  The problem is almost certainly caused by 
the first patch in the series, which changes the way that the initial 
read/8 thing is done.  However, I have no idea what part of the patch 
could be causing these errors.  I would appreciate anything you can find 
out.

You should concentrate your initial investigation on the new 
get_bMaxPacketSize0() routine.  In particular, does the -EPROTO error 
value arise as the return value from the usb_control_msg() call, or does 
it happen because of the values stored in buf?  In the first case, how 
does this call differ from the one that used to be made by 
usb_get_device_descriptor()?  And in the second case, what are the 
values of rc, buf->bMaxPacketSize0, and buf->bDescriptorType?

Alan Stern

Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Wed, Aug 09, 2023 at 09:47:14PM -0400, Alan Stern wrote:
> On Thu, Aug 10, 2023 at 12:28:27AM +0000, Thinh Nguyen wrote:

> > Testing from Greg's usb-next branch, this series causes problem with
> > device enumeration:
> > 
> > [   31.650759] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 2 using xhci_hcd
> > [   31.663107] usb 2-1: device descriptor read/8, error -71
> > [   31.952697] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 3 using xhci_hcd
> > [   31.965122] usb 2-1: device descriptor read/8, error -71
> > [   32.080991] usb usb2-port1: attempt power cycle
> > [   34.826893] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 5 using xhci_hcd
> > [   34.839241] usb 2-1: device descriptor read/8, error -71
> > [   35.129908] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 6 using xhci_hcd
> > [   35.142264] usb 2-1: device descriptor read/8, error -71
> > [   35.257155] usb usb2-port1: attempt power cycle
> > [   37.258922] usb 1-1: new high-speed USB device number 5 using xhci_hcd
> > [   38.115053] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 8 using xhci_hcd
> > [   38.127407] usb 2-1: device descriptor read/8, error -71
> > [   38.417066] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 9 using xhci_hcd
> > [   38.429428] usb 2-1: device descriptor read/8, error -71
> > [   38.545315] usb usb2-port1: attempt power cycle
> > [   43.347312] usb 2-2: new SuperSpeed Plus Gen 2x1 USB device number 11 using xhci_hcd
> > [   43.359659] usb 2-2: device descriptor read/8, error -71
> > [   43.649323] usb 2-2: new SuperSpeed Plus Gen 2x1 USB device number 12 using xhci_hcd
> > [   43.661681] usb 2-2: device descriptor read/8, error -71
> > [   43.777566] usb usb2-port2: attempt power cycle
> > 
> > I was testing with our host along with other common vendor hosts with a
> > few 3.x devices. Reverting this series resolves the issue. I didn't do
> > extensive tests for various speeds or debug it. I just want to notify
> > this first perhaps you can figure out the issue right away.
> > 
> > I can look into it and provide more info later this week. If you want to
> > print any debug, let me know and I can provide later this week.
> 
> Thanks for the notification.  The problem is almost certainly caused by 
> the first patch in the series, which changes the way that the initial 
> read/8 thing is done.  However, I have no idea what part of the patch 
> could be causing these errors.  I would appreciate anything you can find 
> out.
> 
> You should concentrate your initial investigation on the new 
> get_bMaxPacketSize0() routine.  In particular, does the -EPROTO error 
> value arise as the return value from the usb_control_msg() call, or does 
> it happen because of the values stored in buf?  In the first case, how 
> does this call differ from the one that used to be made by 
> usb_get_device_descriptor()?  And in the second case, what are the 
> values of rc, buf->bMaxPacketSize0, and buf->bDescriptorType?

Never mind; I found the problem.  I had forgotten that at SuperSpeed or 
faster, the device descriptor uses a logarithmic encoding for 
bMaxPacketSize0.

The patch below should fix things up.  Let me know how it goes.

Alan Stern



 drivers/usb/core/hub.c |   36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4705,7 +4705,7 @@ static int get_bMaxPacketSize0(struct us
 				buf, size,
 				initial_descriptor_timeout);
 		switch (buf->bMaxPacketSize0) {
-		case 8: case 16: case 32: case 64: case 255:
+		case 8: case 16: case 32: case 64: case 9:
 			if (buf->bDescriptorType == USB_DT_DEVICE) {
 				rc = buf->bMaxPacketSize0;
 				break;
@@ -4992,23 +4992,35 @@ hub_port_init(struct usb_hub *hub, struc
 	if (retval)
 		goto fail;
 
-	if (maxp0 == 0xff || udev->speed >= USB_SPEED_SUPER)
-		i = 512;
-	else
-		i = maxp0;
-	if (usb_endpoint_maxp(&udev->ep0.desc) != i) {
-		if (udev->speed == USB_SPEED_LOW ||
-				!(i == 8 || i == 16 || i == 32 || i == 64)) {
-			dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", i);
-			retval = -EMSGSIZE;
-			goto fail;
-		}
+	/*
+	 * Check the ep0 maxpacket guess and correct it if necessary.
+	 * maxp0 is the value stored in the device descriptor;
+	 * i is the value it encodes (logarithmic for SuperSpeed or greater).
+	 */
+	i = maxp0;
+	if (udev->speed >= USB_SPEED_SUPER) {
+		if (maxp0 <= 16)
+			i = 1 << maxp0;
+		else
+			i = 0;		/* Invalid */
+	}
+	if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
+		;	/* Initial ep0 maxpacket guess is right */
+	} else if ((udev->speed == USB_SPEED_FULL ||
+				udev->speed == USB_SPEED_HIGH) &&
+			(i == 8 || i == 16 || i == 32 || i == 64)) {
+		/* Initial guess is wrong; use the descriptor's value */
 		if (udev->speed == USB_SPEED_FULL)
 			dev_dbg(&udev->dev, "ep0 maxpacket = %d\n", i);
 		else
 			dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
 		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
 		usb_ep0_reinit(udev);
+	} else {
+		/* Initial guess is wrong and descriptor's value is invalid */
+		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
+		retval = -EMSGSIZE;
+		goto fail;
 	}
 
 	descr = usb_get_device_descriptor(udev);

Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Thinh Nguyen]

On Thu, Aug 10, 2023, Alan Stern wrote:
> On Wed, Aug 09, 2023 at 09:47:14PM -0400, Alan Stern wrote:
> > On Thu, Aug 10, 2023 at 12:28:27AM +0000, Thinh Nguyen wrote:
> 
> > > Testing from Greg's usb-next branch, this series causes problem with
> > > device enumeration:
> > > 
> > > [   31.650759] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 2 using xhci_hcd
> > > [   31.663107] usb 2-1: device descriptor read/8, error -71
> > > [   31.952697] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 3 using xhci_hcd
> > > [   31.965122] usb 2-1: device descriptor read/8, error -71
> > > [   32.080991] usb usb2-port1: attempt power cycle
> > > [   34.826893] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 5 using xhci_hcd
> > > [   34.839241] usb 2-1: device descriptor read/8, error -71
> > > [   35.129908] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 6 using xhci_hcd
> > > [   35.142264] usb 2-1: device descriptor read/8, error -71
> > > [   35.257155] usb usb2-port1: attempt power cycle
> > > [   37.258922] usb 1-1: new high-speed USB device number 5 using xhci_hcd
> > > [   38.115053] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 8 using xhci_hcd
> > > [   38.127407] usb 2-1: device descriptor read/8, error -71
> > > [   38.417066] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 9 using xhci_hcd
> > > [   38.429428] usb 2-1: device descriptor read/8, error -71
> > > [   38.545315] usb usb2-port1: attempt power cycle
> > > [   43.347312] usb 2-2: new SuperSpeed Plus Gen 2x1 USB device number 11 using xhci_hcd
> > > [   43.359659] usb 2-2: device descriptor read/8, error -71
> > > [   43.649323] usb 2-2: new SuperSpeed Plus Gen 2x1 USB device number 12 using xhci_hcd
> > > [   43.661681] usb 2-2: device descriptor read/8, error -71
> > > [   43.777566] usb usb2-port2: attempt power cycle
> > > 
> > > I was testing with our host along with other common vendor hosts with a
> > > few 3.x devices. Reverting this series resolves the issue. I didn't do
> > > extensive tests for various speeds or debug it. I just want to notify
> > > this first perhaps you can figure out the issue right away.
> > > 
> > > I can look into it and provide more info later this week. If you want to
> > > print any debug, let me know and I can provide later this week.
> > 
> > Thanks for the notification.  The problem is almost certainly caused by 
> > the first patch in the series, which changes the way that the initial 
> > read/8 thing is done.  However, I have no idea what part of the patch 
> > could be causing these errors.  I would appreciate anything you can find 
> > out.
> > 
> > You should concentrate your initial investigation on the new 
> > get_bMaxPacketSize0() routine.  In particular, does the -EPROTO error 
> > value arise as the return value from the usb_control_msg() call, or does 
> > it happen because of the values stored in buf?  In the first case, how 
> > does this call differ from the one that used to be made by 
> > usb_get_device_descriptor()?  And in the second case, what are the 
> > values of rc, buf->bMaxPacketSize0, and buf->bDescriptorType?
> 
> Never mind; I found the problem.  I had forgotten that at SuperSpeed or 
> faster, the device descriptor uses a logarithmic encoding for 
> bMaxPacketSize0.
> 
> The patch below should fix things up.  Let me know how it goes.
> 

Quick test for Gen 1 and 2 devices work fine now. Highspeed also works
as expected before. I didn't test Fullspeed with various MPS, but I
don't expect any problem looking at the change.

Thanks for the fix,
Thinh

Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Thu, Aug 10, 2023 at 10:39:36PM +0000, Thinh Nguyen wrote:
> On Thu, Aug 10, 2023, Alan Stern wrote:
> > Never mind; I found the problem.  I had forgotten that at SuperSpeed or 
> > faster, the device descriptor uses a logarithmic encoding for 
> > bMaxPacketSize0.
> > 
> > The patch below should fix things up.  Let me know how it goes.
> > 
> 
> Quick test for Gen 1 and 2 devices work fine now. Highspeed also works
> as expected before. I didn't test Fullspeed with various MPS, but I
> don't expect any problem looking at the change.
> 
> Thanks for the fix,

And thanks for testing it.  Is it okay to put your 
"Reported-and-tested-by:" tag on the submitted patch?

Alan Stern

Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Thinh Nguyen]

On Thu, Aug 10, 2023, Alan Stern wrote:
> On Thu, Aug 10, 2023 at 10:39:36PM +0000, Thinh Nguyen wrote:
> > On Thu, Aug 10, 2023, Alan Stern wrote:
> > > Never mind; I found the problem.  I had forgotten that at SuperSpeed or 
> > > faster, the device descriptor uses a logarithmic encoding for 
> > > bMaxPacketSize0.
> > > 
> > > The patch below should fix things up.  Let me know how it goes.
> > > 
> > 
> > Quick test for Gen 1 and 2 devices work fine now. Highspeed also works
> > as expected before. I didn't test Fullspeed with various MPS, but I
> > don't expect any problem looking at the change.
> > 
> > Thanks for the fix,
> 
> And thanks for testing it.  Is it okay to put your 
> "Reported-and-tested-by:" tag on the submitted patch?
> 

Yes.

BR,
Thinh

[PATCH] USB: core: Fix oversight in SuperSpeed initialization

[Alan Stern]

Commit 85d07c556216 ("USB: core: Unite old scheme and new scheme
descriptor reads") altered the way USB devices are enumerated
following detection, and in the process it messed up the
initialization of SuperSpeed (or faster) devices:

[   31.650759] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 2 using xhci_hcd
[   31.663107] usb 2-1: device descriptor read/8, error -71
[   31.952697] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 3 using xhci_hcd
[   31.965122] usb 2-1: device descriptor read/8, error -71
[   32.080991] usb usb2-port1: attempt power cycle
...

The problem was caused by the commit forgetting that in SuperSpeed or
faster devices, the device descriptor uses a logarithmic encoding of
the bMaxPacketSize0 value.  (For some reason I thought the 255 case in
the switch statement was meant for these devices, but it isn't -- it
was meant for Wireless USB and is no longer needed.)

We can fix the oversight by testing for buf->bMaxPacketSize0 = 9
(meaning 512, the actual maxpacket size for ep0 on all SuperSpeed
devices) and straightening out the logic that checks and adjusts our
initial guesses of the maxpacket value.

Reported-and-tested-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Closes: https://lore.kernel.org/linux-usb/20230810002257.nadxmfmrobkaxgnz@synopsys.com/
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Fixes: 85d07c556216 ("USB: core: Unite old scheme and new scheme descriptor reads")

---

 drivers/usb/core/hub.c |   36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4705,7 +4705,7 @@ static int get_bMaxPacketSize0(struct us
 				buf, size,
 				initial_descriptor_timeout);
 		switch (buf->bMaxPacketSize0) {
-		case 8: case 16: case 32: case 64: case 255:
+		case 8: case 16: case 32: case 64: case 9:
 			if (buf->bDescriptorType == USB_DT_DEVICE) {
 				rc = buf->bMaxPacketSize0;
 				break;
@@ -4992,23 +4992,35 @@ hub_port_init(struct usb_hub *hub, struc
 	if (retval)
 		goto fail;
 
-	if (maxp0 == 0xff || udev->speed >= USB_SPEED_SUPER)
-		i = 512;
-	else
-		i = maxp0;
-	if (usb_endpoint_maxp(&udev->ep0.desc) != i) {
-		if (udev->speed == USB_SPEED_LOW ||
-				!(i == 8 || i == 16 || i == 32 || i == 64)) {
-			dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", i);
-			retval = -EMSGSIZE;
-			goto fail;
-		}
+	/*
+	 * Check the ep0 maxpacket guess and correct it if necessary.
+	 * maxp0 is the value stored in the device descriptor;
+	 * i is the value it encodes (logarithmic for SuperSpeed or greater).
+	 */
+	i = maxp0;
+	if (udev->speed >= USB_SPEED_SUPER) {
+		if (maxp0 <= 16)
+			i = 1 << maxp0;
+		else
+			i = 0;		/* Invalid */
+	}
+	if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
+		;	/* Initial ep0 maxpacket guess is right */
+	} else if ((udev->speed == USB_SPEED_FULL ||
+				udev->speed == USB_SPEED_HIGH) &&
+			(i == 8 || i == 16 || i == 32 || i == 64)) {
+		/* Initial guess is wrong; use the descriptor's value */
 		if (udev->speed == USB_SPEED_FULL)
 			dev_dbg(&udev->dev, "ep0 maxpacket = %d\n", i);
 		else
 			dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
 		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
 		usb_ep0_reinit(udev);
+	} else {
+		/* Initial guess is wrong and descriptor's value is invalid */
+		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
+		retval = -EMSGSIZE;
+		goto fail;
 	}
 
 	descr = usb_get_device_descriptor(udev);

Re: [PATCH] USB: core: Fix oversight in SuperSpeed initialization

[Greg KH]

On Fri, Aug 11, 2023 at 01:38:46PM -0400, Alan Stern wrote:
> Commit 85d07c556216 ("USB: core: Unite old scheme and new scheme
> descriptor reads") altered the way USB devices are enumerated
> following detection, and in the process it messed up the
> initialization of SuperSpeed (or faster) devices:
> 
> [   31.650759] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 2 using xhci_hcd
> [   31.663107] usb 2-1: device descriptor read/8, error -71
> [   31.952697] usb 2-1: new SuperSpeed Plus Gen 2x1 USB device number 3 using xhci_hcd
> [   31.965122] usb 2-1: device descriptor read/8, error -71
> [   32.080991] usb usb2-port1: attempt power cycle
> ...
> 
> The problem was caused by the commit forgetting that in SuperSpeed or
> faster devices, the device descriptor uses a logarithmic encoding of
> the bMaxPacketSize0 value.  (For some reason I thought the 255 case in
> the switch statement was meant for these devices, but it isn't -- it
> was meant for Wireless USB and is no longer needed.)
> 
> We can fix the oversight by testing for buf->bMaxPacketSize0 = 9
> (meaning 512, the actual maxpacket size for ep0 on all SuperSpeed
> devices) and straightening out the logic that checks and adjusts our
> initial guesses of the maxpacket value.
> 
> Reported-and-tested-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> Closes: https://lore.kernel.org/linux-usb/20230810002257.nadxmfmrobkaxgnz@synopsys.com/
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Fixes: 85d07c556216 ("USB: core: Unite old scheme and new scheme descriptor reads")
> 
> ---
> 
>  drivers/usb/core/hub.c |   36 ++++++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 12 deletions(-)

Process nit, when you add a patch to an existing patch series like this,
when I run `b4` to download and apply the patch, it sucks in the
original patch series, not this add-on patch (which then of course fails
as the original patch series was already applied.)

So, if it's easy enough, can you just send add-on patches for stuff that
has been applied, as a new thread?  That way the tools handle it
automatically and I don't have to hand edit/apply the patch?

thanks,

greg k-h

Re: [PATCH] USB: core: Fix oversight in SuperSpeed initialization

[Alan Stern]

On Sat, Aug 12, 2023 at 10:05:27AM +0200, Greg KH wrote:
> Process nit, when you add a patch to an existing patch series like this,
> when I run `b4` to download and apply the patch, it sucks in the
> original patch series, not this add-on patch (which then of course fails
> as the original patch series was already applied.)
> 
> So, if it's easy enough, can you just send add-on patches for stuff that
> has been applied, as a new thread?  That way the tools handle it
> automatically and I don't have to hand edit/apply the patch?

Sure.  I don't use b4, so I wasn't aware of this behavior.

If I ever forget in the future, feel free to yell at me.  :-)

Alan Stern

[REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Jan Čermák]

Hi Alan, Greg, everyone,

On 04. 08. 23 21:09, Alan Stern wrote:
> An outstanding syzbot bug report has been traced to a race between the
> routine that reads in the device descriptor for a device being
> reinitialized and the routine that writes the descriptors to a sysfs
> attribute file.  The problem is that reinitializing a device, like
> initializing it for the first time, stores the device descriptor
> directly in the usb_device structure, where it may be accessed
> concurrently as part of sending the descriptors to the sysfs reader.

I have a suspicion that some of these patches (three from the original 
series, plus the "Fix oversight..." one) introduced a regression we see 
with some USB devices in Home Assistant OS (but in mainstream distro as 
well, see below). In particular it's Z-Wave.me UZB stick (0658:0200), 
however roughly at the time of introduction of these patches, we started 
to see a few more reports of issues with USB devices (in general radios 
for IoT protocols), so I can't rule out it's source of more regressions. 
For this particular device, we have most detailed tracing of the issue, 
confirming it also manifests on mainstream distribution (Debian) which 
included these patches in its kernel. Most issue reports come from RPi 3 
but we also got them on amd64, and both on HAOS and Debian.

I'm a layman in terms of the USB stack, so I might be wrong about some 
assumptions, but anyway, the device seemed to always misbehave due to 
poor HW (?) implementation - every time it's plugged into an USB slot, 
the following messages appear:

[ 1134.073005] usb 1-1.4: new full-speed USB device number 12 using dwc_otg
[ 1134.153006] usb 1-1.4: device descriptor read/64, error -32
[ 1134.341003] usb 1-1.4: device descriptor read/64, error -32
[ 1134.529004] usb 1-1.4: new full-speed USB device number 13 using dwc_otg
[ 1134.609063] usb 1-1.4: device descriptor read/64, error -32
[ 1134.797005] usb 1-1.4: device descriptor read/64, error -32
[ 1134.905181] usb 1-1-port4: attempt power cycle

However, kernel versions prior to 6.1.52, or 6.1.73 with these patches 
reverted, were able to recover:

[ 1135.717049] usb 1-1.4: new full-speed USB device number 14 using dwc_otg
[ 1135.741234] usb 1-1.4: New USB device found, idVendor=0658, 
idProduct=0200, bcdDevice= 0.00
[ 1135.741275] usb 1-1.4: New USB device strings: Mfr=0, Product=0, 
SerialNumber=0
[ 1135.743959] cdc_acm 1-1.4:1.0: ttyACM0: USB ACM device

Without these patches reverted, 6.1.73 goes 2 another rounds of device 
descriptor read errors, and ends with:

[ 263.705865] usb 1-1-port4: unable to enumerate USB device

Also it should be noted that it seems that this only happens on USB 2 
ports, on USB 3/SS ports, the descriptor read errors are "protocol 
error" instead of "broken pipe", and the driver recovers (realizing 
this, I am now finally able to reproduce the issue in my environment):

[ 38.244292] usb 2-3: new full-speed USB device number 3 using xhci_hcd
[ 38.372319] usb 2-3: device descriptor read/64, error -71
[ 38.608317] usb 2-3: device descriptor read/64, error -71
[ 38.844287] usb 2-3: new full-speed USB device number 4 using xhci_hcd
[ 38.972317] usb 2-3: device descriptor read/64, error -71
[ 39.208325] usb 2-3: device descriptor read/64, error -71
[ 39.316405] usb usb2-port3: attempt power cycle
[ 39.936295] usb 2-3: new full-speed USB device number 5 using xhci_hcd
[ 39.957228] usb 2-3: New USB device found, idVendor=0658, 
idProduct=0200, bcdDevice= 0.00
[ 39.957241] usb 2-3: New USB device strings: Mfr=0, Product=0, 
SerialNumber=0
[ 39.999591] cdc_acm 2-3:1.0: ttyACM0: USB ACM device
[ 39.999639] usbcore: registered new interface driver cdc_acm
[ 39.999641] cdc_acm: USB Abstract Control Model driver for USB modems 
and ISDN adapters

This is the gist of the problem, more detailed findings can be found in 
reports by @FredrikFornstad in the GH issue [1], who managed to 
reproduce and pinpoint the likely source of the problem.

Let me know if you need any more details, or if there's something more 
to try, I'll be happy to help with getting this resolved.

Thanks,
Jan


[1] 
https://github.com/home-assistant/operating-system/issues/2995#issuecomment-1973507518

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Tue, Mar 05, 2024 at 09:20:22AM +0100, Jan Čermák wrote:
> Hi Alan, Greg, everyone,
> 
> On 04. 08. 23 21:09, Alan Stern wrote:
> > An outstanding syzbot bug report has been traced to a race between the
> > routine that reads in the device descriptor for a device being
> > reinitialized and the routine that writes the descriptors to a sysfs
> > attribute file.  The problem is that reinitializing a device, like
> > initializing it for the first time, stores the device descriptor
> > directly in the usb_device structure, where it may be accessed
> > concurrently as part of sending the descriptors to the sysfs reader.
> 
> I have a suspicion that some of these patches (three from the original
> series, plus the "Fix oversight..." one) introduced a regression we see with
> some USB devices in Home Assistant OS (but in mainstream distro as well, see
> below). In particular it's Z-Wave.me UZB stick (0658:0200), however roughly
> at the time of introduction of these patches, we started to see a few more
> reports of issues with USB devices (in general radios for IoT protocols), so
> I can't rule out it's source of more regressions. For this particular
> device, we have most detailed tracing of the issue, confirming it also
> manifests on mainstream distribution (Debian) which included these patches
> in its kernel. Most issue reports come from RPi 3 but we also got them on
> amd64, and both on HAOS and Debian.
> 
> I'm a layman in terms of the USB stack, so I might be wrong about some
> assumptions, but anyway, the device seemed to always misbehave due to poor
> HW (?) implementation - every time it's plugged into an USB slot, the
> following messages appear:
> 
> [ 1134.073005] usb 1-1.4: new full-speed USB device number 12 using dwc_otg
> [ 1134.153006] usb 1-1.4: device descriptor read/64, error -32
> [ 1134.341003] usb 1-1.4: device descriptor read/64, error -32
> [ 1134.529004] usb 1-1.4: new full-speed USB device number 13 using dwc_otg
> [ 1134.609063] usb 1-1.4: device descriptor read/64, error -32
> [ 1134.797005] usb 1-1.4: device descriptor read/64, error -32
> [ 1134.905181] usb 1-1-port4: attempt power cycle
> 
> However, kernel versions prior to 6.1.52, or 6.1.73 with these patches
> reverted, were able to recover:
> 
> [ 1135.717049] usb 1-1.4: new full-speed USB device number 14 using dwc_otg
> [ 1135.741234] usb 1-1.4: New USB device found, idVendor=0658,
> idProduct=0200, bcdDevice= 0.00
> [ 1135.741275] usb 1-1.4: New USB device strings: Mfr=0, Product=0,
> SerialNumber=0
> [ 1135.743959] cdc_acm 1-1.4:1.0: ttyACM0: USB ACM device
> 
> Without these patches reverted, 6.1.73 goes 2 another rounds of device
> descriptor read errors, and ends with:
> 
> [ 263.705865] usb 1-1-port4: unable to enumerate USB device
> 
> Also it should be noted that it seems that this only happens on USB 2 ports,
> on USB 3/SS ports, the descriptor read errors are "protocol error" instead
> of "broken pipe", and the driver recovers (realizing this, I am now finally
> able to reproduce the issue in my environment):
> 
> [ 38.244292] usb 2-3: new full-speed USB device number 3 using xhci_hcd
> [ 38.372319] usb 2-3: device descriptor read/64, error -71
> [ 38.608317] usb 2-3: device descriptor read/64, error -71
> [ 38.844287] usb 2-3: new full-speed USB device number 4 using xhci_hcd
> [ 38.972317] usb 2-3: device descriptor read/64, error -71
> [ 39.208325] usb 2-3: device descriptor read/64, error -71
> [ 39.316405] usb usb2-port3: attempt power cycle
> [ 39.936295] usb 2-3: new full-speed USB device number 5 using xhci_hcd
> [ 39.957228] usb 2-3: New USB device found, idVendor=0658, idProduct=0200,
> bcdDevice= 0.00
> [ 39.957241] usb 2-3: New USB device strings: Mfr=0, Product=0,
> SerialNumber=0
> [ 39.999591] cdc_acm 2-3:1.0: ttyACM0: USB ACM device
> [ 39.999639] usbcore: registered new interface driver cdc_acm
> [ 39.999641] cdc_acm: USB Abstract Control Model driver for USB modems and
> ISDN adapters
> 
> This is the gist of the problem, more detailed findings can be found in
> reports by @FredrikFornstad in the GH issue [1], who managed to reproduce
> and pinpoint the likely source of the problem.
> 
> Let me know if you need any more details, or if there's something more to
> try, I'll be happy to help with getting this resolved.

Can you provide two usbmon traces, one showing the problem with those 
patches present and the other (on the same system but with the patches 
reverted) showing the recovery?  Comparison of the two should indicate 
what's happening differently.

Alan Stern

PS: I would avoid the Raspberry Pi for this sort of testing, because 
it uses a nonstandard USB host controller driver (dwc-otg) by default.

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Jan Čermák]

Hi Alan,

On 06. 03. 24 22:08, Alan Stern wrote:
> Can you provide two usbmon traces, one showing the problem with those
> patches present and the other (on the same system but with the patches
> reverted) showing the recovery?  Comparison of the two should indicate
> what's happening differently.

I reproduced the issue on my old ThinkPad X220 with 6.6.20 kernel, you 
can find the usbmon captures below.

Regards,
Jan


#############################
# Failing to recover (6.6.20)
#############################

ffff9f9a012cae40 297301265 C Ii:1:002:1 0:2048 1 = 04
ffff9f9a012cae40 297301305 S Ii:1:002:1 -115:2048 1 <
ffff9f9a29b3f300 297301346 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 297301530 C Ci:1:002:0 0 4 = 01010100
ffff9f9a29b3f300 297301555 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff9f9a29b3f300 297301789 C Co:1:002:0 0 0
ffff9f9a29b3f300 297301803 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 297302073 C Ci:1:002:0 0 4 = 01010000
ffff9f9a29b3f300 297329100 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 297329348 C Ci:1:002:0 0 4 = 01010000
ffff9f9a29b3f300 297356109 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 297356276 C Ci:1:002:0 0 4 = 01010000
ffff9f9a29b3f300 297383108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 297383392 C Ci:1:002:0 0 4 = 01010000
ffff9f9a29b3f300 297410108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 297410303 C Ci:1:002:0 0 4 = 01010000
ffff9f9a29b3f300 297410342 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f300 297410560 C Co:1:002:0 0 0
ffff9f9a29b3f300 297422109 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 297422231 C Ci:1:002:0 0 4 = 03011000
ffff9f9a29b3f300 297422249 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff9f9a29b3f300 297422492 C Co:1:002:0 0 0
ffff9f9a29b3f300 297474142 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f300 297474386 C Ci:1:000:0 -32 0
ffff9f9a29b3f300 297474654 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f300 297474910 C Ci:1:000:0 -32 0
ffff9f9a29b3f300 297474959 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f300 297475295 C Ci:1:000:0 -32 0
ffff9f9a29b3f300 297475361 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f300 297475613 C Co:1:002:0 0 0
ffff9f9a29b3f300 297488055 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 297488336 C Ci:1:002:0 0 4 = 03011000
ffff9f9a29b3f300 297488365 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff9f9a29b3f300 297488593 C Co:1:002:0 0 0
ffff9f9a29b3f300 297646110 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f300 297646603 C Ci:1:000:0 -32 0
ffff9f9a29b3f300 297646635 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f300 297646934 C Ci:1:000:0 -32 0
ffff9f9a29b3f300 297646998 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f300 297647326 C Ci:1:000:0 -32 0
ffff9f9a29b3f300 297647390 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f300 297647649 C Co:1:002:0 0 0
ffff9f9a29b3f300 297659102 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 297659257 C Ci:1:002:0 0 4 = 03011000
ffff9f9a29b3f300 297659272 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff9f9a29b3f300 297659512 C Co:1:002:0 0 0
ffff9f9a29b3f300 297814110 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff9f9a29b3f300 297814487 C Co:1:002:0 0 0
ffff9f9a29b3f240 297814536 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f240 297814758 C Co:1:002:0 0 0
ffff9f9a29b3f240 297826111 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f240 297826417 C Ci:1:002:0 0 4 = 03011000
ffff9f9a29b3f240 297826444 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff9f9a29b3f240 297826680 C Co:1:002:0 0 0
ffff9f9a29b3f240 297878173 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f240 297878367 C Ci:1:000:0 -32 0
ffff9f9a29b3f240 297878391 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f240 297878992 C Ci:1:000:0 -32 0
ffff9f9a29b3f240 297879258 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f240 297879521 C Ci:1:000:0 -32 0
ffff9f9a29b3f240 297879608 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f240 297879852 C Co:1:002:0 0 0
ffff9f9a29b3f240 297891106 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f240 297891266 C Ci:1:002:0 0 4 = 03011000
ffff9f9a29b3f240 297891294 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff9f9a29b3f240 297891528 C Co:1:002:0 0 0
ffff9f9a29b3f240 298046109 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f240 298046587 C Ci:1:000:0 -32 0
ffff9f9a29b3f240 298046610 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f240 298046926 C Ci:1:000:0 -32 0
ffff9f9a29b3f240 298046998 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff9f9a29b3f240 298047338 C Ci:1:000:0 -32 0
ffff9f9a29b3f240 298047353 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f240 298047600 C Co:1:002:0 0 0
ffff9f9a29b3f240 298059108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f240 298059372 C Ci:1:002:0 0 4 = 03011000
ffff9f9a29b3f240 298059398 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff9f9a29b3f240 298059630 C Co:1:002:0 0 0
ffff9f9a29b3f240 298214111 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff9f9a29b3f240 298214247 C Co:1:002:0 0 0
ffff9f9a29b3f300 298214304 S Co:1:002:0 s 23 01 0008 0002 0000 0
ffff9f9a29b3f300 298214501 C Co:1:002:0 0 0
ffff9f9a29b3f300 298422107 S Co:1:002:0 s 23 03 0008 0002 0000 0
ffff9f9a29b3f300 298422285 C Co:1:002:0 0 0
ffff9f9a29b3f300 298526126 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f300 298526358 C Co:1:002:0 0 0
ffff9f9a29b3f300 298538105 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 298538360 C Ci:1:002:0 0 4 = 03011100
ffff9f9a29b3f300 298538388 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff9f9a29b3f300 298538633 C Co:1:002:0 0 0
ffff9f9a29b3f300 298538675 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f300 298538893 C Co:1:002:0 0 0
ffff9f9a012cae40 298581342 C Ii:1:002:1 0:2048 1 = 04
ffff9f9a012cae40 298581372 S Ii:1:002:1 -115:2048 1 <
ffff9f9a29b3f300 298742112 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 298742459 C Ci:1:002:0 0 4 = 03011000
ffff9f9a29b3f300 298742483 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff9f9a29b3f300 298742715 C Co:1:002:0 0 0
ffff9f9a29b3f300 298794144 S Co:1:000:0 s 00 05 0006 0000 0000 0
ffff9f9a29b3f300 298794387 C Co:1:000:0 -32 0
ffff9f9a29b3f300 298998127 S Co:1:000:0 s 00 05 0006 0000 0000 0
ffff9f9a29b3f300 298998509 C Co:1:000:0 -32 0
ffff9f9a012cae40 299093323 C Ii:1:002:1 0:2048 1 = 04
ffff9f9a012cae40 299093338 S Ii:1:002:1 -115:2048 1 <
ffff9f9a29b3f300 299206193 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff9f9a29b3f300 299206523 C Co:1:002:0 0 0
ffff9f9a29b3f240 299206583 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f240 299206778 C Co:1:002:0 0 0
ffff9f9a29b3f240 299218068 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f240 299218239 C Ci:1:002:0 0 4 = 03011100
ffff9f9a29b3f240 299218261 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff9f9a29b3f240 299218503 C Co:1:002:0 0 0
ffff9f9a29b3f240 299218534 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f240 299218765 C Co:1:002:0 0 0
ffff9f9a012cae40 299349383 C Ii:1:002:1 0:2048 1 = 04
ffff9f9a012cae40 299349398 S Ii:1:002:1 -115:2048 1 <
ffff9f9a29b3f240 299422112 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f240 299422523 C Ci:1:002:0 0 4 = 03011000
ffff9f9a29b3f240 299422547 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff9f9a29b3f240 299422781 C Co:1:002:0 0 0
ffff9f9a29b3f240 299474143 S Co:1:000:0 s 00 05 0007 0000 0000 0
ffff9f9a29b3f240 299474452 C Co:1:000:0 0 0
ffff9f9a29b3f300 299488113 S Ci:1:007:0 s 80 06 0100 0000 0008 8 <
ffff9f9a29b3f300 299488395 C Ci:1:007:0 -32 0
ffff9f9a29b3f300 299488479 S Ci:1:007:0 s 80 06 0100 0000 0008 8 <
ffff9f9a29b3f300 299488798 C Ci:1:007:0 -32 0
ffff9f9a29b3f300 299488868 S Ci:1:007:0 s 80 06 0100 0000 0008 8 <
ffff9f9a29b3f300 299489193 C Ci:1:007:0 -32 0
ffff9f9a29b3f300 299610112 S Ci:1:007:0 s 80 06 0100 0000 0008 8 <
ffff9f9a29b3f300 299610456 C Ci:1:007:0 -32 0
ffff9f9a29b3f300 299610480 S Ci:1:007:0 s 80 06 0100 0000 0008 8 <
ffff9f9a29b3f300 299610792 C Ci:1:007:0 -32 0
ffff9f9a29b3f300 299610817 S Ci:1:007:0 s 80 06 0100 0000 0008 8 <
ffff9f9a29b3f300 299611376 C Ci:1:007:0 -32 0
ffff9f9a29b3f300 299718111 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff9f9a29b3f300 299718382 C Co:1:002:0 0 0
ffff9f9a29b3f240 299718493 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff9f9a29b3f240 299718640 C Co:1:002:0 0 0
ffff9f9a29b3f240 299718675 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f240 299718902 C Ci:1:002:0 0 4 = 01010000

#############################
# Recovered (6.6.20 + revert)
#############################

ffff8fc4c0c5ac00 367063066 C Ii:1:002:1 0:2048 1 = 04
ffff8fc4c0c5ac00 367063105 S Ii:1:002:1 -115:2048 1 <
ffff8fc4ee367300 367063140 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367300 367063319 C Ci:1:002:0 0 4 = 01010100
ffff8fc4ee367300 367063325 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff8fc4ee367300 367063590 C Co:1:002:0 0 0
ffff8fc4ee367300 367063619 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367300 367063850 C Ci:1:002:0 0 4 = 01010000
ffff8fc4ee367240 367090072 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 367090360 C Ci:1:002:0 0 4 = 01010000
ffff8fc4ee367240 367117131 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 367117280 C Ci:1:002:0 0 4 = 01010000
ffff8fc4ee367240 367144088 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 367144414 C Ci:1:002:0 0 4 = 01010000
ffff8fc4ee367240 367171102 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 367171308 C Ci:1:002:0 0 4 = 01010000
ffff8fc4ee367240 367171358 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8fc4ee367240 367171559 C Co:1:002:0 0 0
ffff8fc4ee367240 367183082 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 367183422 C Ci:1:002:0 0 4 = 03011000
ffff8fc4ee367240 367183452 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff8fc4ee367240 367183693 C Co:1:002:0 0 0
ffff8fc4ee367240 367235143 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367240 367235569 C Ci:1:000:0 -32 0
ffff8fc4ee367240 367235656 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367240 367235907 C Ci:1:000:0 -32 0
ffff8fc4ee367240 367235922 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367240 367236569 C Ci:1:000:0 -32 0
ffff8fc4ee367240 367236656 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8fc4ee367240 367236915 C Co:1:002:0 0 0
ffff8fc4ee367240 367248033 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 367248326 C Ci:1:002:0 0 4 = 03011000
ffff8fc4ee367240 367248343 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff8fc4ee367240 367248585 C Co:1:002:0 0 0
ffff8fc4ee367240 367406114 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367240 367406405 C Ci:1:000:0 -32 0
ffff8fc4ee367240 367406432 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367240 367406778 C Ci:1:000:0 -32 0
ffff8fc4ee367240 367406796 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367240 367407117 C Ci:1:000:0 -32 0
ffff8fc4ee367240 367407188 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8fc4ee367240 367407446 C Co:1:002:0 0 0
ffff8fc4ee367240 367419108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 367419418 C Ci:1:002:0 0 4 = 03011000
ffff8fc4ee367240 367419451 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff8fc4ee367240 367419869 C Co:1:002:0 0 0
ffff8fc4ee367240 367574110 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff8fc4ee367240 367574446 C Co:1:002:0 0 0
ffff8fc4ee367180 367574511 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8fc4ee367180 367574700 C Co:1:002:0 0 0
ffff8fc4ee367180 367586105 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367180 367586361 C Ci:1:002:0 0 4 = 03011000
ffff8fc4ee367180 367586390 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff8fc4ee367180 367586619 C Co:1:002:0 0 0
ffff8fc4ee367180 367638098 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367180 367638506 C Ci:1:000:0 -32 0
ffff8fc4ee367180 367638524 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367180 367639020 C Ci:1:000:0 -32 0
ffff8fc4ee367180 367639037 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367180 367639390 C Ci:1:000:0 -32 0
ffff8fc4ee367180 367639406 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8fc4ee367180 367639654 C Co:1:002:0 0 0
ffff8fc4ee367180 367651093 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367180 367651455 C Ci:1:002:0 0 4 = 03011000
ffff8fc4ee367180 367651483 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff8fc4ee367180 367651706 C Co:1:002:0 0 0
ffff8fc4ee367180 367806113 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367180 367806291 C Ci:1:000:0 -32 0
ffff8fc4ee367180 367806319 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367180 367806947 C Ci:1:000:0 -32 0
ffff8fc4ee367180 367807039 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8fc4ee367180 367807536 C Ci:1:000:0 -32 0
ffff8fc4ee367180 367807569 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8fc4ee367180 367808060 C Co:1:002:0 0 0
ffff8fc4ee367180 367820112 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367180 367820430 C Ci:1:002:0 0 4 = 03011000
ffff8fc4ee367180 367820456 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff8fc4ee367180 367820694 C Co:1:002:0 0 0
ffff8fc4ee367180 367974107 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff8fc4ee367180 367974289 C Co:1:002:0 0 0
ffff8fc4ee367240 367974353 S Co:1:002:0 s 23 01 0008 0002 0000 0
ffff8fc4ee367240 367974556 C Co:1:002:0 0 0
ffff8fc4ee367240 368182107 S Co:1:002:0 s 23 03 0008 0002 0000 0
ffff8fc4ee367240 368182386 C Co:1:002:0 0 0
ffff8fc4ee367240 368286131 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8fc4ee367240 368286298 C Co:1:002:0 0 0
ffff8fc4ee367240 368298105 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 368298310 C Ci:1:002:0 0 4 = 03011100
ffff8fc4ee367240 368298332 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff8fc4ee367240 368298571 C Co:1:002:0 0 0
ffff8fc4ee367240 368298641 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8fc4ee367240 368298823 C Co:1:002:0 0 0
ffff8fc4c0c5ac00 368343025 C Ii:1:002:1 0:2048 1 = 04
ffff8fc4c0c5ac00 368343056 S Ii:1:002:1 -115:2048 1 <
ffff8fc4ee367240 368502095 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 368502372 C Ci:1:002:0 0 4 = 01011100
ffff8fc4ee367240 368502407 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff8fc4ee367240 368502627 C Co:1:002:0 0 0
ffff8fc4ee367240 368502648 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8fc4ee367240 368502886 C Co:1:002:0 0 0
ffff8fc4c0c5ac00 368598913 C Ii:1:002:1 0:2048 1 = 04
ffff8fc4c0c5ac00 368598925 S Ii:1:002:1 -115:2048 1 <
ffff8fc4ee367240 368710109 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 368710339 C Ci:1:002:0 0 4 = 03011000
ffff8fc4ee367240 368710367 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff8fc4ee367240 368710594 C Co:1:002:0 0 0
ffff8fc4ee367240 368762141 S Co:1:000:0 s 00 05 0006 0000 0000 0
ffff8fc4ee367240 368762451 C Co:1:000:0 0 0
ffff8fc4ee367180 368776096 S Ci:1:006:0 s 80 06 0100 0000 0008 8 <
ffff8fc4ee367180 368776399 C Ci:1:006:0 0 8 = 12010002 02000008
ffff8fc4ee367240 368778058 S Ci:1:006:0 s 80 06 0100 0000 0012 18 <
ffff8fc4ee367240 368778361 C Ci:1:006:0 0 18 = 12010002 02000008 
58060002 00000000 0001
ffff8fc4ee367240 368778400 S Ci:1:006:0 s 80 06 0600 0000 000a 10 <
ffff8fc4ee367240 368778629 C Ci:1:006:0 -32 0
ffff8fc4ee367240 368778644 S Ci:1:006:0 s 80 06 0600 0000 000a 10 <
ffff8fc4ee367240 368778967 C Ci:1:006:0 -32 0
ffff8fc4ee367240 368778996 S Ci:1:006:0 s 80 06 0600 0000 000a 10 <
ffff8fc4ee367240 368779315 C Ci:1:006:0 -32 0
ffff8fc4ee367240 368779397 S Ci:1:006:0 s 80 06 0200 0000 0009 9 <
ffff8fc4ee367240 368779667 C Ci:1:006:0 0 9 = 09024300 02010080 32
ffff8fc4ee367240 368779679 S Ci:1:006:0 s 80 06 0200 0000 0043 67 <
ffff8fc4ee367240 368780133 C Ci:1:006:0 0 67 = 09024300 02010080 
32090400 00010202 01000524 00100105 24010001 04240200
ffff8fc4ee367000 368780596 S Co:1:006:0 s 00 09 0001 0000 0000 0
ffff8fc4ee367000 368780976 C Co:1:006:0 0 0
ffff8fc4ee367000 368781766 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367000 368782044 C Ci:1:002:0 0 4 = 03010000
ffff8fc505e4b480 368805117 S Co:1:006:0 s 21 20 0000 0000 0007 7 = 
80250000 000008
ffff8fc505e4b480 368805270 C Co:1:006:0 0 7 >

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Thu, Mar 07, 2024 at 05:17:20PM +0100, Jan Čermák wrote:
> Hi Alan,
> 
> On 06. 03. 24 22:08, Alan Stern wrote:
> > Can you provide two usbmon traces, one showing the problem with those
> > patches present and the other (on the same system but with the patches
> > reverted) showing the recovery?  Comparison of the two should indicate
> > what's happening differently.
> 
> I reproduced the issue on my old ThinkPad X220 with 6.6.20 kernel, you can
> find the usbmon captures below.

This is quite strange; the two traces are the same up to this point:

ffff9f9a29b3f300 298538675 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff9f9a29b3f300 298538893 C Co:1:002:0 0 0
ffff9f9a012cae40 298581342 C Ii:1:002:1 0:2048 1 = 04
ffff9f9a012cae40 298581372 S Ii:1:002:1 -115:2048 1 <
ffff9f9a29b3f300 298742112 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff9f9a29b3f300 298742459 C Ci:1:002:0 0 4 = 03011000

------------------------------------------------------------------

ffff8fc4ee367240 368298641 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8fc4ee367240 368298823 C Co:1:002:0 0 0
ffff8fc4c0c5ac00 368343025 C Ii:1:002:1 0:2048 1 = 04
ffff8fc4c0c5ac00 368343056 S Ii:1:002:1 -115:2048 1 <
ffff8fc4ee367240 368502095 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8fc4ee367240 368502372 C Ci:1:002:0 0 4 = 01011100

The difference is in the last line: 03011000 vs. 01011100.  This means
that in the "working" scenario the device disconnected itself from
the USB bus for no apparent reason and then reconnected, whereas in
the "nonworking" scenario it didn't.  The computer did nothing
different before then, so I have no idea why the device's behavior
changed.  It's a mystery.

Another thing the traces showed is that the device doesn't like the
"new" initialization scheme; it wants the "old" one.  You can test
this by setting the old_scheme_first module parameter for usbcore
before plugging in the device:

	echo 1 >/sys/module/usbcore/parameters/old_scheme_first

Also, you can try the patch below (without the module parameter set).
I suspect it should be applied in any case, but it would be nice to
know if it makes any difference in your case.

Alan Stern



Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -5481,6 +5481,7 @@ loop:
 			msleep(2 * hub_power_on_good_delay(hub));
 			usb_hub_set_port_power(hdev, hub, port1, true);
 			msleep(hub_power_on_good_delay(hub));
+			hub_port_debounce_be_stable(hub, port1);
 		}
 	}
 	if (hub->hdev->parent ||

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Jan Čermák]

On 07. 03. 24 20:34, Alan Stern wrote:
> Another thing the traces showed is that the device doesn't like the
> "new" initialization scheme; it wants the "old" one.  You can test
> this by setting the old_scheme_first module parameter for usbcore
> before plugging in the device:
> 
> 	echo 1 >/sys/module/usbcore/parameters/old_scheme_first
> 

You are right, this indeed works and probing is successful after the 
power-cycling attempt.

> Also, you can try the patch below (without the module parameter set).
> I suspect it should be applied in any case, but it would be nice to
> know if it makes any difference in your case.
> 
> Index: usb-devel/drivers/usb/core/hub.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/hub.c
> +++ usb-devel/drivers/usb/core/hub.c
> @@ -5481,6 +5481,7 @@ loop:
>   			msleep(2 * hub_power_on_good_delay(hub));
>   			usb_hub_set_port_power(hdev, hub, port1, true);
>   			msleep(hub_power_on_good_delay(hub));
> +			hub_port_debounce_be_stable(hub, port1);
>   		}
>   	}
>   	if (hub->hdev->parent ||

Unfortunately, this doesn't fix it. It changes the log output a bit but 
still does not enumerate:

[  199.295695] usb 1-1.2: new full-speed USB device number 25 using ehci-pci
[  199.360684] usb 1-1.2: device descriptor read/64, error -32
[  199.534678] usb 1-1.2: device descriptor read/64, error -32
[  199.708671] usb 1-1.2: new full-speed USB device number 26 using ehci-pci
[  199.773666] usb 1-1.2: device descriptor read/64, error -32
[  199.941602] usb 1-1.2: device descriptor read/64, error -32
[  200.044871] usb 1-1-port2: attempt power cycle
[  200.528631] usb 1-1.2: new full-speed USB device number 27 using ehci-pci
[  200.940603] usb 1-1.2: device not accepting address 27, error -32
[  201.208592] usb 1-1.2: new full-speed USB device number 28 using ehci-pci
[  201.223783] usb 1-1.2: device descriptor read/8, error -32
[  201.345772] usb 1-1.2: device descriptor read/8, error -32
[  201.452899] usb 1-1-port2: unable to enumerate USB device

I'm attaching usbmon traces with this patch.

Regards,
Jan

####################################################


ffff94ff414f8a80 199261119 C Ii:1:002:1 0:2048 1 = 04
ffff94ff414f8a80 199261158 S Ii:1:002:1 -115:2048 1 <
ffff94ff882bd840 199261199 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 199261378 C Ci:1:002:0 0 4 = 01010100
ffff94ff882bd840 199261383 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff94ff882bd840 199261635 C Co:1:002:0 0 0
ffff94ff882bd840 199261661 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 199261902 C Ci:1:002:0 0 4 = 01010000
ffff94ff882bd840 199288109 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 199288463 C Ci:1:002:0 0 4 = 01010000
ffff94ff882bd840 199315108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 199315374 C Ci:1:002:0 0 4 = 01010000
ffff94ff882bd840 199342113 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 199342504 C Ci:1:002:0 0 4 = 01010000
ffff94ff882bd840 199369107 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 199369447 C Ci:1:002:0 0 4 = 01010000
ffff94ff882bd840 199369498 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff94ff882bd840 199369903 C Co:1:002:0 0 0
ffff94ff882bd840 199381106 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 199381375 C Ci:1:002:0 0 4 = 03011000
ffff94ff882bd840 199381410 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff94ff882bd840 199381633 C Co:1:002:0 0 0
ffff94ff882bd840 199433148 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd840 199433339 C Ci:1:000:0 -32 0
ffff94ff882bd840 199433360 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd840 199433971 C Ci:1:000:0 -32 0
ffff94ff882bd840 199433988 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd840 199434342 C Ci:1:000:0 -32 0
ffff94ff882bd840 199434363 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff94ff882bd840 199434609 C Co:1:002:0 0 0
ffff94ff882bd840 199446107 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 199446423 C Ci:1:002:0 0 4 = 03011000
ffff94ff882bd840 199446448 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff94ff882bd840 199446679 C Co:1:002:0 0 0
ffff94ff882bd840 199606111 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd840 199606348 C Ci:1:000:0 -32 0
ffff94ff882bd840 199606372 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd840 199607003 C Ci:1:000:0 -32 0
ffff94ff882bd840 199607090 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd840 199607593 C Ci:1:000:0 -32 0
ffff94ff882bd840 199607623 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff94ff882bd840 199608123 C Co:1:002:0 0 0
ffff94ff882bd840 199620108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 199620506 C Ci:1:002:0 0 4 = 03011000
ffff94ff882bd840 199620531 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff94ff882bd840 199620770 C Co:1:002:0 0 0
ffff94ff882bd840 199782114 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff94ff882bd840 199782380 C Co:1:002:0 0 0
ffff94ff882bd900 199782433 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff94ff882bd900 199782639 C Co:1:002:0 0 0
ffff94ff882bd900 199794109 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd900 199794511 C Ci:1:002:0 0 4 = 03011000
ffff94ff882bd900 199794536 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff94ff882bd900 199794769 C Co:1:002:0 0 0
ffff94ff882bd900 199846142 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd900 199846554 C Ci:1:000:0 -32 0
ffff94ff882bd900 199846574 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd900 199846828 C Ci:1:000:0 -32 0
ffff94ff882bd900 199846842 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd900 199847170 C Ci:1:000:0 -32 0
ffff94ff882bd900 199847236 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff94ff882bd900 199847490 C Co:1:002:0 0 0
ffff94ff882bd900 199859108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd900 199859470 C Ci:1:002:0 0 4 = 03011000
ffff94ff882bd900 199859496 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff94ff882bd900 199859726 C Co:1:002:0 0 0
ffff94ff882bd900 200014111 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd900 200014492 C Ci:1:000:0 -32 0
ffff94ff882bd900 200014516 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd900 200014828 C Ci:1:000:0 -32 0
ffff94ff882bd900 200014854 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff94ff882bd900 200015180 C Ci:1:000:0 -32 0
ffff94ff882bd900 200015251 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff94ff882bd900 200015502 C Co:1:002:0 0 0
ffff94ff882bd900 200027108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd900 200027318 C Ci:1:002:0 0 4 = 03011000
ffff94ff882bd900 200027345 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff94ff882bd900 200027579 C Co:1:002:0 0 0
ffff94ff882bd900 200182109 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff94ff882bd900 200182296 C Co:1:002:0 0 0
ffff94ff882bd840 200182354 S Co:1:002:0 s 23 01 0008 0002 0000 0
ffff94ff882bd840 200182551 C Co:1:002:0 0 0
ffff94ff882bd840 200390109 S Co:1:002:0 s 23 03 0008 0002 0000 0
ffff94ff882bd840 200390345 C Co:1:002:0 0 0
ffff94ff882bd840 200494114 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 200494318 C Ci:1:002:0 0 4 = 01010100
ffff94ff882bd840 200494345 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff94ff882bd840 200494571 C Co:1:002:0 0 0
ffff94ff882bd840 200521107 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 200521316 C Ci:1:002:0 0 4 = 01010000
ffff94ff882bd840 200548107 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 200548426 C Ci:1:002:0 0 4 = 01010000
ffff94ff882bd840 200575100 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 200575338 C Ci:1:002:0 0 4 = 01010000
ffff94ff882bd840 200602100 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 200602260 C Ci:1:002:0 0 4 = 01010000
ffff94ff882bd840 200602311 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff94ff882bd840 200602714 C Co:1:002:0 0 0
ffff94ff882bd840 200614101 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd840 200614389 C Ci:1:002:0 0 4 = 03011000
ffff94ff882bd840 200614416 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff94ff882bd840 200614842 C Co:1:002:0 0 0
ffff94ff882bd840 200666141 S Co:1:000:0 s 00 05 001b 0000 0000 0
ffff94ff882bd840 200666529 C Co:1:000:0 -32 0
ffff94ff414f8a80 200797181 C Ii:1:002:1 0:2048 1 = 04
ffff94ff414f8a80 200797212 S Ii:1:002:1 -115:2048 1 <
ffff94ff882bd840 200870110 S Co:1:000:0 s 00 05 001b 0000 0000 0
ffff94ff882bd840 200870423 C Co:1:000:0 -32 0
ffff94ff414f8a80 201053174 C Ii:1:002:1 0:2048 1 = 04
ffff94ff414f8a80 201053190 S Ii:1:002:1 -115:2048 1 <
ffff94ff882bd840 201079556 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff94ff882bd840 201079768 C Co:1:002:0 0 0
ffff94ff882bd900 201079814 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff94ff882bd900 201080045 C Co:1:002:0 0 0
ffff94ff882bd900 201092106 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd900 201092293 C Ci:1:002:0 0 4 = 03011100
ffff94ff882bd900 201092321 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff94ff882bd900 201092559 C Co:1:002:0 0 0
ffff94ff882bd900 201092596 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff94ff882bd900 201092819 C Co:1:002:0 0 0
ffff94ff882bd900 201294110 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd900 201294319 C Ci:1:002:0 0 4 = 03011000
ffff94ff882bd900 201294346 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff94ff882bd900 201294575 C Co:1:002:0 0 0
ffff94ff882bd900 201346134 S Co:1:000:0 s 00 05 001c 0000 0000 0
ffff94ff882bd900 201346293 C Co:1:000:0 0 0
ffff94ff882bd840 201360112 S Ci:1:028:0 s 80 06 0100 0000 0008 8 <
ffff94ff882bd840 201360521 C Ci:1:028:0 -32 0
ffff94ff882bd840 201360558 S Ci:1:028:0 s 80 06 0100 0000 0008 8 <
ffff94ff882bd840 201360922 C Ci:1:028:0 -32 0
ffff94ff882bd840 201360938 S Ci:1:028:0 s 80 06 0100 0000 0008 8 <
ffff94ff882bd840 201361276 C Ci:1:028:0 -32 0
ffff94ff882bd840 201482107 S Ci:1:028:0 s 80 06 0100 0000 0008 8 <
ffff94ff882bd840 201482304 C Ci:1:028:0 -32 0
ffff94ff882bd840 201482330 S Ci:1:028:0 s 80 06 0100 0000 0008 8 <
ffff94ff882bd840 201482934 C Ci:1:028:0 -32 0
ffff94ff882bd840 201482954 S Ci:1:028:0 s 80 06 0100 0000 0008 8 <
ffff94ff882bd840 201483278 C Ci:1:028:0 -32 0
ffff94ff882bd840 201590112 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff94ff882bd840 201590387 C Co:1:002:0 0 0
ffff94ff882bd900 201591835 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff94ff882bd900 201592045 C Co:1:002:0 0 0
ffff94ff882bd900 201592073 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff94ff882bd900 201592497 C Ci:1:002:0 0 4 = 01010000

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Mon, Mar 11, 2024 at 10:58:18AM +0100, Jan Čermák wrote:
> On 07. 03. 24 20:34, Alan Stern wrote:
> > Another thing the traces showed is that the device doesn't like the
> > "new" initialization scheme; it wants the "old" one.  You can test
> > this by setting the old_scheme_first module parameter for usbcore
> > before plugging in the device:
> > 
> > 	echo 1 >/sys/module/usbcore/parameters/old_scheme_first
> > 
> 
> You are right, this indeed works and probing is successful after the
> power-cycling attempt.

Well, at least this means you do have a way of using the device, even
if it is rather awkward.  It might even work on the Raspberry Pi machine.

> > Also, you can try the patch below (without the module parameter set).
> > I suspect it should be applied in any case, but it would be nice to
> > know if it makes any difference in your case.
> > 
> > Index: usb-devel/drivers/usb/core/hub.c
> > ===================================================================
> > --- usb-devel.orig/drivers/usb/core/hub.c
> > +++ usb-devel/drivers/usb/core/hub.c
> > @@ -5481,6 +5481,7 @@ loop:
> >   			msleep(2 * hub_power_on_good_delay(hub));
> >   			usb_hub_set_port_power(hdev, hub, port1, true);
> >   			msleep(hub_power_on_good_delay(hub));
> > +			hub_port_debounce_be_stable(hub, port1);
> >   		}
> >   	}
> >   	if (hub->hdev->parent ||
> 
> Unfortunately, this doesn't fix it. It changes the log output a bit but
> still does not enumerate:
> 
> [  199.295695] usb 1-1.2: new full-speed USB device number 25 using ehci-pci
> [  199.360684] usb 1-1.2: device descriptor read/64, error -32
> [  199.534678] usb 1-1.2: device descriptor read/64, error -32
> [  199.708671] usb 1-1.2: new full-speed USB device number 26 using ehci-pci
> [  199.773666] usb 1-1.2: device descriptor read/64, error -32
> [  199.941602] usb 1-1.2: device descriptor read/64, error -32
> [  200.044871] usb 1-1-port2: attempt power cycle
> [  200.528631] usb 1-1.2: new full-speed USB device number 27 using ehci-pci
> [  200.940603] usb 1-1.2: device not accepting address 27, error -32
> [  201.208592] usb 1-1.2: new full-speed USB device number 28 using ehci-pci
> [  201.223783] usb 1-1.2: device descriptor read/8, error -32
> [  201.345772] usb 1-1.2: device descriptor read/8, error -32
> [  201.452899] usb 1-1-port2: unable to enumerate USB device
> 
> I'm attaching usbmon traces with this patch.

The device is so non-responsive, I'm amazed it ever works at all.
Judging by the usbmon traces, it doesn't look as if it would work on a
Windows system.

Actually, if you have access to a computer running Windows or Mac OSX
and you can try out the device on that computer, it would be good to
get the equivalent of a usbmon trace (something like a Wireshark
capture log would do).  If those systems manage to do something that
Linux doesn't, we ought to know what it is.

Alan Stern

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Jan Čermák]

Hi Alan

On 11. 03. 24 15:43, Alan Stern wrote:
> Well, at least this means you do have a way of using the device, even
> if it is rather awkward.  It might even work on the Raspberry Pi machine.

Still, I'm looking for a more permanent and robust solution. If it were 
only a single device I'm using myself, I could come up with a 
workaround. However, this is one of the very few available Z-Wave USB 
interfaces and there are more users affected. So far we went with 
reverting the patches, but that's surely not the way we want to go forward.

> The device is so non-responsive, I'm amazed it ever works at all.
> Judging by the usbmon traces, it doesn't look as if it would work on a
> Windows system.
> 
> Actually, if you have access to a computer running Windows or Mac OSX
> and you can try out the device on that computer, it would be good to
> get the equivalent of a usbmon trace (something like a Wireshark
> capture log would do).  If those systems manage to do something that
> Linux doesn't, we ought to know what it is.

Fredrik (one of the original reporters) is following this conversation, 
here [1] are logs from his machine with some details in the ticket [2]. 
He also wonders why the initialization doesn't work only on USB2 ports 
but works on USB3 if the initialization code is shared between those two.

Also, if needed, I can get more logs from the X220 I was using for my 
usbmon traces - I believe I have an HDD with Windows 7 install lying 
somewhere.

Regards,
Jan


[1] 
https://github.com/home-assistant/operating-system/files/14563813/usbPcap1_USB2-port.txt
[2] 
https://github.com/home-assistant/operating-system/issues/2995#issuecomment-1989329739

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Tue, Mar 12, 2024 at 09:57:06AM +0100, Jan Čermák wrote:
> Hi Alan
> 
> On 11. 03. 24 15:43, Alan Stern wrote:
> > Well, at least this means you do have a way of using the device, even
> > if it is rather awkward.  It might even work on the Raspberry Pi machine.
> 
> Still, I'm looking for a more permanent and robust solution. If it were only
> a single device I'm using myself, I could come up with a workaround.
> However, this is one of the very few available Z-Wave USB interfaces and
> there are more users affected. So far we went with reverting the patches,
> but that's surely not the way we want to go forward.

I agree, this should be fixed if we can.  I'm still puzzled about how 
commits you located could cause any difference in the device's behavior, 
given that they don't seem to cause any change's to the computer's 
behavior.

> > The device is so non-responsive, I'm amazed it ever works at all.
> > Judging by the usbmon traces, it doesn't look as if it would work on a
> > Windows system.
> > 
> > Actually, if you have access to a computer running Windows or Mac OSX
> > and you can try out the device on that computer, it would be good to
> > get the equivalent of a usbmon trace (something like a Wireshark
> > capture log would do).  If those systems manage to do something that
> > Linux doesn't, we ought to know what it is.
> 
> Fredrik (one of the original reporters) is following this conversation, here
> [1] are logs from his machine with some details in the ticket [2]. He also
> wonders why the initialization doesn't work only on USB2 ports but works on
> USB3 if the initialization code is shared between those two.

Good.  I've been quite busy recently, but I'll look at the logs when 
there is time.

The USB-2 and USB-3 drivers do share some initialization code, but not 
all of it.  The first couple of steps are quite different: With a USB-2 
controller they are handled by the kernel driver, but with a USB-3 
controller they are handled directly by the controller's hardware with 
no software influence.

Alan Stern

> Also, if needed, I can get more logs from the X220 I was using for my usbmon
> traces - I believe I have an HDD with Windows 7 install lying somewhere.
> 
> Regards,
> Jan
> 
> 
> [1] https://github.com/home-assistant/operating-system/files/14563813/usbPcap1_USB2-port.txt
> [2] https://github.com/home-assistant/operating-system/issues/2995#issuecomment-1989329739

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Tue, Mar 12, 2024 at 09:57:06AM +0100, Jan Čermák wrote:
> Hi Alan
> 
> On 11. 03. 24 15:43, Alan Stern wrote:
> > Well, at least this means you do have a way of using the device, even
> > if it is rather awkward.  It might even work on the Raspberry Pi machine.
> 
> Still, I'm looking for a more permanent and robust solution. If it were only
> a single device I'm using myself, I could come up with a workaround.
> However, this is one of the very few available Z-Wave USB interfaces and
> there are more users affected. So far we went with reverting the patches,
> but that's surely not the way we want to go forward.
> 
> > The device is so non-responsive, I'm amazed it ever works at all.
> > Judging by the usbmon traces, it doesn't look as if it would work on a
> > Windows system.
> > 
> > Actually, if you have access to a computer running Windows or Mac OSX
> > and you can try out the device on that computer, it would be good to
> > get the equivalent of a usbmon trace (something like a Wireshark
> > capture log would do).  If those systems manage to do something that
> > Linux doesn't, we ought to know what it is.
> 
> Fredrik (one of the original reporters) is following this conversation, here
> [1] are logs from his machine with some details in the ticket [2]. He also
> wonders why the initialization doesn't work only on USB2 ports but works on
> USB3 if the initialization code is shared between those two.

It's very difficult to compare the Windows log with the usbmon trace.  
However, something Frederik mentioned about the number of resets led me 
to check more closely.  There are some extra unnecessary resets in the 
usbmon trace, and a reset that should be there is missing.

Below is a patch meant to get the number of resets back to what it 
should be.  I'd appreciate it if you can test this, and report the 
kernel log output along with the usbmon output for the normal case and 
also with the "old_scheme_first" parameter set.

I'm not very hopeful that this will solve the problem, but there's a 
good chance it will help point us in the right direction by removing 
extraneous complications.

Alan Stern



 drivers/usb/core/hub.c |   27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4961,6 +4961,18 @@ hub_port_init(struct usb_hub *hub, struc
 			break;
 		}
 
+		if (retries > 0) {
+			retval = hub_port_reset(hub, port1, udev, delay, false);
+			if (retval < 0)		/* error or disconnect */
+				goto fail;
+			if (oldspeed != udev->speed) {
+				dev_dbg(&udev->dev,
+					"device reset changed speed!\n");
+				retval = -ENODEV;
+				goto fail;
+			}
+		}
+
 		if (do_new_scheme) {
 			retval = hub_enable_device(udev);
 			if (retval < 0) {
@@ -4972,6 +4984,13 @@ hub_port_init(struct usb_hub *hub, struc
 
 			maxp0 = get_bMaxPacketSize0(udev, buf,
 					GET_DESCRIPTOR_BUFSIZE, retries == 0);
+			if (maxp0 < 0) {
+				if (maxp0 != -ENODEV)
+					dev_err(&udev->dev, "device descriptor read/64, error %d\n",
+							maxp0);
+				retval = maxp0;
+				continue;
+			}
 			if (maxp0 > 0 && !initial &&
 					maxp0 != udev->descriptor.bMaxPacketSize0) {
 				dev_err(&udev->dev, "device reset changed ep0 maxpacket size!\n");
@@ -4988,13 +5007,6 @@ hub_port_init(struct usb_hub *hub, struc
 				retval = -ENODEV;
 				goto fail;
 			}
-			if (maxp0 < 0) {
-				if (maxp0 != -ENODEV)
-					dev_err(&udev->dev, "device descriptor read/64, error %d\n",
-							maxp0);
-				retval = maxp0;
-				continue;
-			}
 		}
 
 		for (operations = 0; operations < SET_ADDRESS_TRIES; ++operations) {
@@ -5533,6 +5545,7 @@ loop:
 			msleep(2 * hub_power_on_good_delay(hub));
 			usb_hub_set_port_power(hdev, hub, port1, true);
 			msleep(hub_power_on_good_delay(hub));
+			hub_port_debounce_be_stable(hub, port1);
 		}
 	}
 	if (hub->hdev->parent ||

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Jan Čermák]

Hello Alan,

On 16. 03. 24 21:35, Alan Stern wrote:
> Below is a patch meant to get the number of resets back to what it
> should be.  I'd appreciate it if you can test this, and report the
> kernel log output along with the usbmon output for the normal case and
> also with the "old_scheme_first" parameter set.
> 
> I'm not very hopeful that this will solve the problem, but there's a
> good chance it will help point us in the right direction by removing
> extraneous complications.

unfortunately you were right, the problem is still unresolved with your 
patch. I hope the traces will provide some new insights then.

Regards
Jan


###############################
# Patched - new scheme (6.6.21)
###############################

[  149.638997] usb 1-1.2: new full-speed USB device number 8 using ehci-pci
[  149.640602] usb 1-1.2: device descriptor read/64, error -32
[  149.811493] usb 1-1.2: device descriptor read/64, error -32
[  149.986051] usb 1-1.2: new full-speed USB device number 9 using ehci-pci
[  149.987370] usb 1-1.2: device descriptor read/64, error -32
[  150.163337] usb 1-1.2: device descriptor read/64, error -32
[  150.266422] usb 1-1-port2: attempt power cycle
[  150.750045] usb 1-1.2: new full-speed USB device number 10 using ehci-pci
[  151.162030] usb 1-1.2: device not accepting address 10, error -32
[  151.430029] usb 1-1.2: new full-speed USB device number 11 using ehci-pci
[  151.445492] usb 1-1.2: device descriptor read/8, error -32
[  151.633637] usb 1-1.2: device descriptor read/8, error -32
[  151.739227] usb 1-1-port2: unable to enumerate USB device

ffff8881003dc9c0 149518477 C Ii:1:002:1 0:2048 1 = 04
ffff8881003dc9c0 149518517 S Ii:1:002:1 -115:2048 1 <
ffff888139ff8480 149518558 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888139ff8480 149518935 C Ci:1:002:0 0 4 = 01010100
ffff888139ff8480 149518951 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff888139ff8480 149519199 C Co:1:002:0 0 0
ffff888139ff8480 149519229 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888139ff8480 149519467 C Ci:1:002:0 0 4 = 01010000
ffff888139ff8480 149546106 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888139ff8480 149546393 C Ci:1:002:0 0 4 = 01010000
ffff888139ff8480 149573113 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888139ff8480 149573442 C Ci:1:002:0 0 4 = 01010000
ffff888139ff8480 149600103 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888139ff8480 149600309 C Ci:1:002:0 0 4 = 01010000
ffff888139ff8480 149627105 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888139ff8480 149627441 C Ci:1:002:0 0 4 = 01010000
ffff888139ff8480 149627495 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888139ff8480 149627699 C Co:1:002:0 0 0
ffff888139ff8480 149639104 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888139ff8480 149639364 C Ci:1:002:0 0 4 = 03011000
ffff888139ff8480 149639401 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff888139ff8480 149639619 C Co:1:002:0 0 0
ffff888139ff8480 149691082 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff888139ff8480 149691572 C Ci:1:000:0 -32 0
ffff8881419950c0 149691656 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8881419950c0 149691905 C Ci:1:000:0 -32 0
ffff8881419950c0 149691917 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8881419950c0 149692565 C Ci:1:000:0 -32 0
ffff8881419950c0 149798110 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff8881419950c0 149798431 C Co:1:002:0 0 0
ffff8881419950c0 149810115 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8881419950c0 149810401 C Ci:1:002:0 0 4 = 03011000
ffff8881419950c0 149810427 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff8881419950c0 149810864 C Co:1:002:0 0 0
ffff8881419950c0 149862123 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8881419950c0 149862618 C Ci:1:000:0 -32 0
ffff8881419950c0 149862653 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8881419950c0 149863135 C Ci:1:000:0 -32 0
ffff8881419950c0 149863165 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff8881419950c0 149863534 C Ci:1:000:0 -32 0
ffff8881419950c0 149974112 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff8881419950c0 149974320 C Co:1:002:0 0 0
ffff888141995000 149974393 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888141995000 149974575 C Co:1:002:0 0 0
ffff888141995000 149986107 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888141995000 149986437 C Ci:1:002:0 0 4 = 03011000
ffff888141995000 149986471 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff888141995000 149986696 C Co:1:002:0 0 0
ffff888141995000 150038142 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff888141995000 150038648 C Ci:1:000:0 -32 0
ffff888141995000 150038672 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff888141995000 150039032 C Ci:1:000:0 -32 0
ffff888141995000 150039046 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff888141995000 150039358 C Ci:1:000:0 -32 0
ffff888141995000 150150113 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888141995000 150150417 C Co:1:002:0 0 0
ffff888141995000 150162111 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888141995000 150162387 C Ci:1:002:0 0 4 = 03011000
ffff888141995000 150162418 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff888141995000 150162648 C Co:1:002:0 0 0
ffff888141995000 150214119 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff888141995000 150214598 C Ci:1:000:0 -32 0
ffff888141995000 150214632 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff888141995000 150214912 C Ci:1:000:0 -32 0
ffff888141995000 150214930 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff888141995000 150215314 C Ci:1:000:0 -32 0
ffff888141995000 150318106 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff888141995000 150318446 C Co:1:002:0 0 0
ffff8881419950c0 150318517 S Co:1:002:0 s 23 01 0008 0002 0000 0
ffff8881419950c0 150318704 C Co:1:002:0 0 0
ffff8881419950c0 150526102 S Co:1:002:0 s 23 03 0008 0002 0000 0
ffff8881419950c0 150526430 C Co:1:002:0 0 0
ffff8881003dc9c0 150542516 C Ii:1:002:1 0:2048 1 = 04
ffff8881003dc9c0 150542548 S Ii:1:002:1 -115:2048 1 <
ffff8881419950c0 150630112 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8881419950c0 150630417 C Ci:1:002:0 0 4 = 01010100
ffff8881419950c0 150630450 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff8881419950c0 150630685 C Co:1:002:0 0 0
ffff8881419950c0 150657105 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff8881419950c0 150657423 C Ci:1:002:0 0 4 = 01010000
ffff888141995000 150684024 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888141995000 150684290 C Ci:1:002:0 0 4 = 01010000
ffff888141995000 150711108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888141995000 150711365 C Ci:1:002:0 0 4 = 01010000
ffff888141995000 150738112 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888141995000 150738437 C Ci:1:002:0 0 4 = 01010000
ffff888141995000 150738494 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888141995000 150738692 C Co:1:002:0 0 0
ffff888141995000 150750108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888141995000 150750364 C Ci:1:002:0 0 4 = 03011000
ffff888141995000 150750399 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff888141995000 150750621 C Co:1:002:0 0 0
ffff888141995000 150802147 S Co:1:000:0 s 00 05 000a 0000 0000 0
ffff888141995000 150802527 C Co:1:000:0 -32 0
ffff888141995000 151006106 S Co:1:000:0 s 00 05 000a 0000 0000 0
ffff888141995000 151006560 C Co:1:000:0 -32 0
ffff8881003dc9c0 151054453 C Ii:1:002:1 0:2048 1 = 04
ffff8881003dc9c0 151054467 S Ii:1:002:1 -115:2048 1 <
ffff888141995000 151215694 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff888141995000 151216068 C Co:1:002:0 0 0
ffff888141995180 151216115 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888141995180 151216315 C Co:1:002:0 0 0
ffff888141995180 151228102 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888141995180 151228380 C Ci:1:002:0 0 4 = 03011100
ffff888141995180 151228417 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff888141995180 151228638 C Co:1:002:0 0 0
ffff888141995180 151228664 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888141995180 151228899 C Co:1:002:0 0 0
ffff8881003dc9c0 151310482 C Ii:1:002:1 0:2048 1 = 04
ffff8881003dc9c0 151310497 S Ii:1:002:1 -115:2048 1 <
ffff888141995180 151430108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888141995180 151430419 C Ci:1:002:0 0 4 = 03011000
ffff888141995180 151430455 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff888141995180 151430693 C Co:1:002:0 0 0
ffff888141995180 151482140 S Co:1:000:0 s 00 05 000b 0000 0000 0
ffff888141995180 151482405 C Co:1:000:0 0 0
ffff888141995000 151496106 S Ci:1:011:0 s 80 06 0100 0000 0008 8 <
ffff888141995000 151496622 C Ci:1:011:0 -32 0
ffff888141995000 151496658 S Ci:1:011:0 s 80 06 0100 0000 0008 8 <
ffff888141995000 151497138 C Ci:1:011:0 -32 0
ffff888141995000 151497169 S Ci:1:011:0 s 80 06 0100 0000 0008 8 <
ffff888141995000 151497486 C Ci:1:011:0 -32 0
ffff888141995000 151606108 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888141995000 151606374 C Co:1:002:0 0 0
ffff888141995000 151618105 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888141995000 151618344 C Ci:1:002:0 0 4 = 03011000
ffff888141995000 151618379 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff888141995000 151618599 C Co:1:002:0 0 0
ffff888141995000 151670136 S Co:1:000:0 s 00 05 000b 0000 0000 0
ffff888141995000 151670488 C Co:1:000:0 0 0
ffff888141995180 151684039 S Ci:1:011:0 s 80 06 0100 0000 0008 8 <
ffff888141995180 151684454 C Ci:1:011:0 -32 0
ffff888141995180 151684532 S Ci:1:011:0 s 80 06 0100 0000 0008 8 <
ffff888141995180 151684785 C Ci:1:011:0 -32 0
ffff888141995180 151684834 S Ci:1:011:0 s 80 06 0100 0000 0008 8 <
ffff888141995180 151685618 C Ci:1:011:0 -32 0
ffff888141995180 151791035 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff888141995180 151791290 C Co:1:002:0 0 0
ffff888141995000 151792277 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff888141995000 151792534 C Co:1:002:0 0 0
ffff888141995000 151792564 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888141995000 151792796 C Ci:1:002:0 0 4 = 01010000


###############################
# Patched - old scheme (6.6.21)
###############################

[  272.774542] usb 1-1.2: new full-speed USB device number 12 using ehci-pci
[  273.185546] usb 1-1.2: device not accepting address 12, error -32
[  273.251550] usb 1-1.2: new full-speed USB device number 13 using ehci-pci
[  273.665491] usb 1-1.2: device not accepting address 13, error -32
[  273.667236] usb 1-1-port2: attempt power cycle
[  274.203558] usb 1-1.2: new full-speed USB device number 14 using ehci-pci
[  274.283841] usb 1-1.2: New USB device found, idVendor=0658, 
idProduct=0200, bcdDevice= 0.00
[  274.283867] usb 1-1.2: New USB device strings: Mfr=0, Product=0, 
SerialNumber=0
[  274.309310] cdc_acm 1-1.2:1.0: ttyACM0: USB ACM device
[  274.309355] usbcore: registered new interface driver cdc_acm
[  274.309357] cdc_acm: USB Abstract Control Model driver for USB modems 
and ISDN adapters

ffff8881003dc9c0 272654268 C Ii:1:002:1 0:2048 1 = 04
ffff8881003dc9c0 272654316 S Ii:1:002:1 -115:2048 1 <
ffff888145ae1780 272654483 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 272654726 C Ci:1:002:0 0 4 = 01010100
ffff888145ae1780 272654739 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff888145ae1780 272654993 C Co:1:002:0 0 0
ffff888145ae1780 272655020 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 272655255 C Ci:1:002:0 0 4 = 01010000
ffff888145ae1780 272682096 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 272682378 C Ci:1:002:0 0 4 = 01010000
ffff888145ae1780 272709128 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 272709311 C Ci:1:002:0 0 4 = 01010000
ffff888145ae1780 272736109 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 272736445 C Ci:1:002:0 0 4 = 01010000
ffff888145ae1780 272763140 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 272763338 C Ci:1:002:0 0 4 = 01010000
ffff888145ae1780 272763393 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888145ae1780 272763595 C Co:1:002:0 0 0
ffff888145ae1780 272775074 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 272775275 C Ci:1:002:0 0 4 = 03011000
ffff888145ae1780 272775314 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff888145ae1780 272775729 C Co:1:002:0 0 0
ffff888145ae1780 272827142 S Co:1:000:0 s 00 05 000c 0000 0000 0
ffff888145ae1780 272827417 C Co:1:000:0 -32 0
ffff888145ae1780 273030123 S Co:1:000:0 s 00 05 000c 0000 0000 0
ffff888145ae1780 273030664 C Co:1:000:0 -32 0
ffff888145ae1780 273239645 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff888145ae1780 273239905 C Co:1:002:0 0 0
ffff888145ae1d80 273239949 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888145ae1d80 273240163 C Co:1:002:0 0 0
ffff888145ae1d80 273252126 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1d80 273252423 C Ci:1:002:0 0 4 = 03011000
ffff888145ae1d80 273252447 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff888145ae1d80 273252680 C Co:1:002:0 0 0
ffff888145ae1d80 273304146 S Co:1:000:0 s 00 05 000d 0000 0000 0
ffff888145ae1d80 273304402 C Co:1:000:0 -32 0
ffff888145ae1d80 273510101 S Co:1:000:0 s 00 05 000d 0000 0000 0
ffff888145ae1d80 273510377 C Co:1:000:0 -32 0
ffff888145ae1d80 273719545 S Co:1:002:0 s 23 01 0001 0002 0000 0
ffff888145ae1d80 273719772 C Co:1:002:0 0 0
ffff888145ae1780 273719812 S Co:1:002:0 s 23 01 0008 0002 0000 0
ffff888145ae1780 273720047 C Co:1:002:0 0 0
ffff888145ae1780 273926112 S Co:1:002:0 s 23 03 0008 0002 0000 0
ffff888145ae1780 273926406 C Co:1:002:0 0 0
ffff8881003dc9c0 273934050 C Ii:1:002:1 0:2048 1 = 04
ffff8881003dc9c0 273934081 S Ii:1:002:1 -115:2048 1 <
ffff888145ae1780 274030115 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 274030418 C Ci:1:002:0 0 4 = 01010100
ffff888145ae1780 274030442 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff888145ae1780 274030677 C Co:1:002:0 0 0
ffff888145ae1780 274057089 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 274057416 C Ci:1:002:0 0 4 = 01010000
ffff888145ae1780 274084090 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 274084322 C Ci:1:002:0 0 4 = 01010100
ffff888145ae1780 274084353 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff888145ae1780 274084578 C Co:1:002:0 0 0
ffff888145ae1780 274111090 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 274111317 C Ci:1:002:0 0 4 = 01010000
ffff888145ae1780 274138114 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 274138418 C Ci:1:002:0 0 4 = 01010000
ffff888145ae1780 274165108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 274165350 C Ci:1:002:0 0 4 = 01010000
ffff888145ae1780 274192122 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 274192263 C Ci:1:002:0 0 4 = 01010000
ffff888145ae1780 274192320 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888145ae1780 274192521 C Co:1:002:0 0 0
ffff888145ae1780 274204052 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 274204382 C Ci:1:002:0 0 4 = 03011000
ffff888145ae1780 274204405 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff888145ae1780 274204652 C Co:1:002:0 0 0
ffff888145ae1780 274256143 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff888145ae1780 274256517 C Ci:1:000:0 0 8 = 12010002 02000008
ffff888145ae1780 274256546 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff888145ae1780 274256743 C Co:1:002:0 0 0
ffff888145ae1780 274268088 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1780 274268362 C Ci:1:002:0 0 4 = 03011000
ffff888145ae1780 274268388 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff888145ae1780 274268618 C Co:1:002:0 0 0
ffff888145ae1780 274320112 S Co:1:000:0 s 00 05 000e 0000 0000 0
ffff888145ae1780 274320449 C Co:1:000:0 0 0
ffff888145ae1d80 274334110 S Ci:1:014:0 s 80 06 0100 0000 0012 18 <
ffff888145ae1d80 274334545 C Ci:1:014:0 0 18 = 12010002 02000008 
58060002 00000000 0001
ffff888145ae1d80 274334577 S Ci:1:014:0 s 80 06 0600 0000 000a 10 <
ffff888145ae1d80 274334813 C Ci:1:014:0 -32 0
ffff888145ae1d80 274334843 S Ci:1:014:0 s 80 06 0600 0000 000a 10 <
ffff888145ae1d80 274335150 C Ci:1:014:0 -32 0
ffff888145ae1d80 274335168 S Ci:1:014:0 s 80 06 0600 0000 000a 10 <
ffff888145ae1d80 274335581 C Ci:1:014:0 -32 0
ffff888145ae1d80 274335610 S Ci:1:014:0 s 80 06 0200 0000 0009 9 <
ffff888145ae1d80 274335914 C Ci:1:014:0 0 9 = 09024300 02010080 32
ffff888145ae1d80 274335932 S Ci:1:014:0 s 80 06 0200 0000 0043 67 <
ffff888145ae1d80 274336373 C Ci:1:014:0 0 67 = 09024300 02010080 
32090400 00010202 01000524 00100105 24010001 04240200
ffff888145ae1900 274336803 S Co:1:014:0 s 00 09 0001 0000 0000 0
ffff888145ae1900 274337062 C Co:1:014:0 0 0
ffff888145ae1900 274340120 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145ae1900 274340243 C Ci:1:002:0 0 4 = 03010000
ffff888121658a80 274361461 S Co:1:014:0 s 21 20 0000 0000 0007 7 = 
80250000 000008
ffff888121658a80 274361751 C Co:1:014:0 0 7 >

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Tue, Mar 19, 2024 at 12:54:37PM +0100, Jan Čermák wrote:
> Hello Alan,
> 
> On 16. 03. 24 21:35, Alan Stern wrote:
> > Below is a patch meant to get the number of resets back to what it
> > should be.  I'd appreciate it if you can test this, and report the
> > kernel log output along with the usbmon output for the normal case and
> > also with the "old_scheme_first" parameter set.
> > 
> > I'm not very hopeful that this will solve the problem, but there's a
> > good chance it will help point us in the right direction by removing
> > extraneous complications.
> 
> unfortunately you were right, the problem is still unresolved with your
> patch. I hope the traces will provide some new insights then.

I get the strong impression that this device just takes a long time to
initialize when it is plugged in.  A lot longer than the current
debounce time of 150 ms -- more like 2000 ms!  The usbmon traces show
the device disconnecting and reconnecting about 1500 ms after it is
first plugged in, and then it starts working about 300-400 ms later.

Try doing this: Keep the patch applied, but make the following changes
in addition.  In drivers/usb/core/hub.c, around line 128 the code says:

#define HUB_DEBOUNCE_TIMEOUT	2000
#define HUB_DEBOUNCE_STEP	  25
#define HUB_DEBOUNCE_STABLE	 100

Change the HUB_DEBOUNCE_TIMEOUT value to 4500, the HUB_DEBOUNCE_STEP
value to 250 and the HUB_DEBOUNCE_STABLE value to 2000.  That just
might give the device enough time to settle down and start working
before the computer tries using it.

This is not something we would want to do for ordinary kernels; it
would cause new USB devices to be ignored for more than 2 seconds
after they are plugged in, which would annoy many people.  But if my
theory is right, it may be what your device needs.

Alan Stern

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Jan Čermák]

Hi Alan,

On 19. 03. 24 17:03, Alan Stern wrote:
> Change the HUB_DEBOUNCE_TIMEOUT value to 4500, the HUB_DEBOUNCE_STEP
> value to 250 and the HUB_DEBOUNCE_STABLE value to 2000.  That just
> might give the device enough time to settle down and start working
> before the computer tries using it.

sorry for the delay, I only managed to test it today. You are right, 
with the timeouts adjusted, it enumerates fine after a while, without 
any descriptor read errors or anything like that:

[  210.957371] usb 1-1.2: new full-speed USB device number 5 using ehci-pci
[  211.037728] usb 1-1.2: New USB device found, idVendor=0658, 
idProduct=0200, bcdDevice= 0.00
[  211.037747] usb 1-1.2: New USB device strings: Mfr=0, Product=0, 
SerialNumber=0
[  211.039764] cdc_acm 1-1.2:1.0: ttyACM0: USB ACM device

If it's worth anything, usbmon trace is attached below. Anyway, do you 
have any ideas what could be done to make it work without doing any 
detrimental changes? I was thinking I'll try to reach out to the vendor 
at this point - they should be aware their device might stop working 
with recent kernels, and they could explain the quirky behavior, or 
implement any changes on the firmware side (if it's even possible).

Regards,
Jan



ffff888101391300 207454583 C Ii:1:002:1 0:2048 1 = 04
ffff888101391300 207454617 S Ii:1:002:1 -115:2048 1 <
ffff88814d461180 207454651 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 207454845 C Ci:1:002:0 0 4 = 01010100
ffff88814d461180 207454860 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff88814d461180 207455110 C Co:1:002:0 0 0
ffff88814d461180 207455119 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 207455368 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 207710110 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 207710378 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 207966112 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 207966378 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 208222112 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 208222401 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 208478109 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 208478561 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 208734110 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 208734342 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 208990111 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 208990297 C Ci:1:002:0 0 4 = 01010100
ffff88814d461180 208990317 S Co:1:002:0 s 23 01 0010 0002 0000 0
ffff888101391300 208990557 C Ii:1:002:1 0:2048 1 = 04
ffff888101391300 208990574 S Ii:1:002:1 -115:2048 1 <
ffff88814d461180 208990583 C Co:1:002:0 0 0
ffff88814d461180 209246112 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 209246405 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 209502108 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 209502393 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 209758115 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 209758358 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 210014113 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 210014318 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 210270107 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 210270327 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 210526114 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 210526337 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 210782106 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 210782340 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 211038111 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 211038365 C Ci:1:002:0 0 4 = 01010000
ffff88814d461180 211038419 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff88814d461180 211038621 C Co:1:002:0 0 0
ffff88814d461180 211050109 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 211050473 C Ci:1:002:0 0 4 = 03011000
ffff88814d461180 211050508 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff88814d461180 211050732 C Co:1:002:0 0 0
ffff88814d461180 211102145 S Ci:1:000:0 s 80 06 0100 0000 0040 64 <
ffff88814d461180 211102383 C Ci:1:000:0 0 8 = 12010002 02000008
ffff88814d461180 211102410 S Co:1:002:0 s 23 03 0004 0002 0000 0
ffff88814d461180 211102640 C Co:1:002:0 0 0
ffff88814d461180 211114111 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff88814d461180 211114411 C Ci:1:002:0 0 4 = 03011000
ffff88814d461180 211114446 S Co:1:002:0 s 23 01 0014 0002 0000 0
ffff88814d461180 211114667 C Co:1:002:0 0 0
ffff88814d461180 211166117 S Co:1:000:0 s 00 05 0005 0000 0000 0
ffff88814d461180 211166509 C Co:1:000:0 0 0
ffff88814d461240 211180097 S Ci:1:005:0 s 80 06 0100 0000 0012 18 <
ffff88814d461240 211180460 C Ci:1:005:0 0 18 = 12010002 02000008 
58060002 00000000 0001
ffff88814d461240 211180500 S Ci:1:005:0 s 80 06 0600 0000 000a 10 <
ffff88814d461240 211180722 C Ci:1:005:0 -32 0
ffff88814d461240 211180989 S Ci:1:005:0 s 80 06 0600 0000 000a 10 <
ffff88814d461240 211181294 C Ci:1:005:0 -32 0
ffff88814d461240 211181324 S Ci:1:005:0 s 80 06 0600 0000 000a 10 <
ffff88814d461240 211181647 C Ci:1:005:0 -32 0
ffff88814d461240 211181669 S Ci:1:005:0 s 80 06 0200 0000 0009 9 <
ffff88814d461240 211181972 C Ci:1:005:0 0 9 = 09024300 02010080 32
ffff88814d461240 211181999 S Ci:1:005:0 s 80 06 0200 0000 0043 67 <
ffff88814d461240 211182437 C Ci:1:005:0 0 67 = 09024300 02010080 
32090400 00010202 01000524 00100105 24010001 04240200
ffff88814d461300 211182850 S Co:1:005:0 s 00 09 0001 0000 0000 0
ffff88814d461300 211183117 C Co:1:005:0 0 0
ffff888145f1cb40 211184128 S Co:1:005:0 s 21 20 0000 0000 0007 7 = 
80250000 000008
ffff888145f1cb40 211184335 C Co:1:005:0 0 7 >
ffff888145f1cb40 211184995 S Ci:1:002:0 s a3 00 0000 0002 0004 4 <
ffff888145f1cb40 211185187 C Ci:1:002:0 0 4 = 03010000

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Wed, Mar 27, 2024 at 02:24:34PM +0100, Jan Čermák wrote:
> Hi Alan,
> 
> On 19. 03. 24 17:03, Alan Stern wrote:
> > Change the HUB_DEBOUNCE_TIMEOUT value to 4500, the HUB_DEBOUNCE_STEP
> > value to 250 and the HUB_DEBOUNCE_STABLE value to 2000.  That just
> > might give the device enough time to settle down and start working
> > before the computer tries using it.
> 
> sorry for the delay, I only managed to test it today. You are right, with
> the timeouts adjusted, it enumerates fine after a while, without any
> descriptor read errors or anything like that:
> 
> [  210.957371] usb 1-1.2: new full-speed USB device number 5 using ehci-pci
> [  211.037728] usb 1-1.2: New USB device found, idVendor=0658,
> idProduct=0200, bcdDevice= 0.00
> [  211.037747] usb 1-1.2: New USB device strings: Mfr=0, Product=0,
> SerialNumber=0
> [  211.039764] cdc_acm 1-1.2:1.0: ttyACM0: USB ACM device

Great!

> If it's worth anything, usbmon trace is attached below. Anyway, do you have
> any ideas what could be done to make it work without doing any detrimental
> changes? I was thinking I'll try to reach out to the vendor at this point -
> they should be aware their device might stop working with recent kernels,
> and they could explain the quirky behavior, or implement any changes on the
> firmware side (if it's even possible).

The ideal solution would be if the vendor updates the firmware to
prevent the device from turning on its pull-up (thereby telling the
host computer that it is connected to the bus) until it is ready to
operate.  There's no good reason to have that > 1-second period during
which the device claims to be connected but does not work.

Another possible solution, a lot less attractive, would be to change
the initialization code in the hub driver so that if it sees the
device disconnect itself from the bus, it restarts the entire
procedure from the beginning.  You'd end up getting a bunch of error
messages during the initial non-working period, just as you do now,
but afterwards the device should be detected and initialized okay.

Alan Stern

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alexander Dahl]

Hei hei,

following this discussion with some kind of curiosity, because I own
such a device and depent on it, but my firmware version does not seem
to be affected.  Remarks below.

Am Wed, Mar 27, 2024 at 10:21:19AM -0400 schrieb Alan Stern:
> On Wed, Mar 27, 2024 at 02:24:34PM +0100, Jan Čermák wrote:
> > Hi Alan,
> > 
> > On 19. 03. 24 17:03, Alan Stern wrote:
> > > Change the HUB_DEBOUNCE_TIMEOUT value to 4500, the HUB_DEBOUNCE_STEP
> > > value to 250 and the HUB_DEBOUNCE_STABLE value to 2000.  That just
> > > might give the device enough time to settle down and start working
> > > before the computer tries using it.
> > 
> > sorry for the delay, I only managed to test it today. You are right, with
> > the timeouts adjusted, it enumerates fine after a while, without any
> > descriptor read errors or anything like that:
> > 
> > [  210.957371] usb 1-1.2: new full-speed USB device number 5 using ehci-pci
> > [  211.037728] usb 1-1.2: New USB device found, idVendor=0658,
> > idProduct=0200, bcdDevice= 0.00
> > [  211.037747] usb 1-1.2: New USB device strings: Mfr=0, Product=0,
> > SerialNumber=0
> > [  211.039764] cdc_acm 1-1.2:1.0: ttyACM0: USB ACM device
> 
> Great!
> 
> > If it's worth anything, usbmon trace is attached below. Anyway, do you have
> > any ideas what could be done to make it work without doing any detrimental
> > changes? I was thinking I'll try to reach out to the vendor at this point -
> > they should be aware their device might stop working with recent kernels,
> > and they could explain the quirky behavior, or implement any changes on the
> > firmware side (if it's even possible).
> 
> The ideal solution would be if the vendor updates the firmware to
> prevent the device from turning on its pull-up (thereby telling the
> host computer that it is connected to the bus) until it is ready to
> operate.  There's no good reason to have that > 1-second period during
> which the device claims to be connected but does not work.

As pointed out in the GitHub ticket already:  Firmware update from a
users point of view is difficult to impossible.  There's no easy "take
the latest firmware" and update and you're done.  It is not clear
which tools are necessary, and even worse there are only certain
combinations of upgrade paths.  For example upgrading to x.z is only
possible from x.x but not from x.y, if I understood it correctly.  And
you don't know if you will brick the device or not.  And I'm speaking
as an embedded developer.  The ordinary home user is probably not even
going to try it.

> Another possible solution, a lot less attractive, would be to change
> the initialization code in the hub driver so that if it sees the
> device disconnect itself from the bus, it restarts the entire
> procedure from the beginning.  You'd end up getting a bunch of error
> messages during the initial non-working period, just as you do now,
> but afterwards the device should be detected and initialized okay.

Is it possible to hook in with some kind of quirk if this device ID is
seen on the bus (and wait longer just for this device), or can you
only access that after successful init?

Greets
Alex

Re: [REGRESSION] Re: [PATCH 0/3] USB: core: Don't overwrite device descriptor during reinitialization

[Alan Stern]

On Thu, Mar 28, 2024 at 04:44:26PM +0100, Alexander Dahl wrote:
> Hei hei,
> 
> following this discussion with some kind of curiosity, because I own
> such a device and depent on it, but my firmware version does not seem
> to be affected.  Remarks below.

> > The ideal solution would be if the vendor updates the firmware to
> > prevent the device from turning on its pull-up (thereby telling the
> > host computer that it is connected to the bus) until it is ready to
> > operate.  There's no good reason to have that > 1-second period during
> > which the device claims to be connected but does not work.
> 
> As pointed out in the GitHub ticket already:  Firmware update from a
> users point of view is difficult to impossible.  There's no easy "take
> the latest firmware" and update and you're done.  It is not clear
> which tools are necessary, and even worse there are only certain
> combinations of upgrade paths.  For example upgrading to x.z is only
> possible from x.x but not from x.y, if I understood it correctly.  And
> you don't know if you will brick the device or not.  And I'm speaking
> as an embedded developer.  The ordinary home user is probably not even
> going to try it.

Oh, okay.  Too bad.  Although in many cases, manufacturers tend not to 
be eager to change their devices' firmwares just to suit Linux users...

> > Another possible solution, a lot less attractive, would be to change
> > the initialization code in the hub driver so that if it sees the
> > device disconnect itself from the bus, it restarts the entire
> > procedure from the beginning.  You'd end up getting a bunch of error
> > messages during the initial non-working period, just as you do now,
> > but afterwards the device should be detected and initialized okay.
> 
> Is it possible to hook in with some kind of quirk if this device ID is
> seen on the bus (and wait longer just for this device), or can you
> only access that after successful init?

The latter, unfortunately.  Before initialization there's no way to 
communicate with the device.

Alan Stern