Re: [PATCH] usbcore: Add quirk for 255-bytes initial config read

[Alan Stern]

On Fri, Jun 19, 2026 at 03:29:36PM +0530, Nikhil Solanke wrote:
> Certain third-party USB game controllers exposing (or spoofing) an Xbox
> 360-compatible interface (VID:PID 045e:028e) fail to enumerate under Linux.
> The device disconnects from the bus without responding to the initial
> GET_DESCRIPTOR(CONFIGURATION) request, and the kernel logs 'unable to read
> config index 0 descriptor/start: -71'.
> 
> The device then falls back to a secondary Android HID mode (with a
> different VID:PID), losing XInput functionality including rumble support.
> The failure reproduces across multiple machines, host controller types, and
> kernel versions including current mainline and LTS. The device enumerates
> correctly and remains in XInput mode under Windows. Notably, the device
> enumerates correctly in Android mode when the same aklsjdasd 9-byte request
> is issued for that mode's configuration descriptor, confirming the firmware
> bug is specific to the XInput mode.
> 
> usbmon traces from Linux and Wireshark/USBPcap traces from Windows are
> identical up to the point of failure, with no visible protocol-level
> difference explaining the divergence. The root cause was identified when
> Michal Pecio discovered via a QEMU bus-level capture that Windows does not
> use wLength=9 for the initial config descriptor request; it uses
> wLength=255. This is not visible in Windows Wireshark/USBPcap traces
> because Windows routes enumeration-phase traffic to sniffers only after
> initialization completes. Alan Stern subsequently confirmed this with a bus
> analyzer on a different USB 2.0 device, and Michal verified the behavior
> goes back to Windows 95 OSR2.1.
> 
> So, add a new quirk flag USB_QUIRK_CONFIG_SIZE which causes
> usb_get_configuration() to issue a 255 byte sized configuration request
> instead of USB_DT_CONFIG_SIZE (9) for the initial
> GET_DESCRIPTOR(CONFIGURATION) request, mimicking long-standing Windows
> behavior.
> 
> Suggested-by: Nikhil Solanke <nikhilsolanke5@xxxxxxxxx>

You don't need Suggested-by here.  It's redundant; we always assume that 
people are responsible for authorship of the patches they write and 
submit, unless they say otherwise.

> Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Suggested-by: Michal Pecio <michal.pecio@xxxxxxxxx>
> Closes: https://lore.kernel.org/linux-usb/CAFgddh+JWdT4LLwMc5qjM8q_pBu-fRo2qADR5ovAKoGHWMQrRw@xxxxxxxxxxxxxx/
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: <stable@xxxxxxxxxxxxxxx>
> 
> Signed-off-by: Nikhil Solanke <nikhilsolanke5@xxxxxxxxx>
> ---
>  drivers/usb/core/config.c  | 56 +++++++++++++++++++++++++++-----------
>  drivers/usb/core/quirks.c  |  3 ++
>  include/linux/usb/quirks.h |  4 +++
>  3 files changed, 47 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 45e20c6d76c0..623425cef085 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -912,6 +912,8 @@ int usb_get_configuration(struct usb_device *dev)
>  	unsigned char *bigbuffer;
>  	struct usb_config_descriptor *desc;
>  	int result;
> +	size_t usb_dt_config_size = (dev->quirks & USB_QUIRK_CONFIG_SIZE)
> +		? USB_DT_CONFIG_SIZE_QUIRK : USB_DT_CONFIG_SIZE;

I wouldn't call the variable usb_dt_config_size.  It isn't always the 
size of a USB configuration descriptor; it isn't even always the size 
you expect for the response.  Rather, it is the size you intend to ask 
for.

>  
>  	if (ncfg > USB_MAXCONFIG) {
>  		dev_notice(ddev, "too many configurations: %d, "
> @@ -938,7 +940,8 @@ int usb_get_configuration(struct usb_device *dev)
>  	if (!dev->rawdescriptors)
>  		return -ENOMEM;
>  
> -	desc = kmalloc(USB_DT_CONFIG_SIZE, GFP_KERNEL);
> +	desc = kmalloc(usb_dt_config_size, GFP_KERNEL);
> +
>  	if (!desc)
>  		return -ENOMEM;
>  
> @@ -946,7 +949,7 @@ int usb_get_configuration(struct usb_device *dev)
>  		/* We grab just the first descriptor so we know how long
>  		 * the whole configuration is */

This comment is now out of date.  It should be rewritten to explain why 
the quirk does and why.

>  		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> -		    desc, USB_DT_CONFIG_SIZE);
> +		    desc, usb_dt_config_size);
>  		if (result < 0) {
>  			dev_err(ddev, "unable to read config index %d "
>  			    "descriptor/%s: %d\n", cfgno, "start", result);
> @@ -957,26 +960,39 @@ int usb_get_configuration(struct usb_device *dev)
>  			break;
>  		} else if (result < 4) {
>  			dev_err(ddev, "config index %d descriptor too short "
> -			    "(expected %i, got %i)\n", cfgno,
> -			    USB_DT_CONFIG_SIZE, result);
> +			    "(expected %zu, got %i)\n", cfgno,

Likewise, "expected" here is wrong.  It should be "asked for" or 
something like that.

> +			    usb_dt_config_size, result);
>  			result = -EINVAL;
>  			goto err;
>  		}
> -		length = max_t(int, le16_to_cpu(desc->wTotalLength),
> -		    USB_DT_CONFIG_SIZE);
> +		/* If the device does returns the full length configuration
> +		 * descriptor, skip the second read. Fallback to default
> +		 * behavior otherwise.
> +		 */

New multiline comments (or ones that are rewritten) should use the same 
format as the rest of the USB stack:

	/*
	 * Blah, blah, blah
	 * Blah, blah, blah
	 */

> +		if (dev->quirks & USB_QUIRK_CONFIG_SIZE
> +				&& result == le16_to_cpu(desc->wTotalLength)
> +				&& result < USB_DT_CONFIG_SIZE_QUIRK) {

Whether the quirk flag is set doesn't matter.  All you care about is 
whether the information received earlier contains the entire descriptor 
set.  The first and third tests here should be removed.

There is some question about what to do if wTotalLength < result.  My 
advice is to use the smaller value in this case, but not smaller than 
USB_DT_CONFIG_SIZE.

>  
> -		/* Now that we know the length, get the whole thing */
> -		bigbuffer = kmalloc(length, GFP_KERNEL);
> -		if (!bigbuffer) {
> -			result = -ENOMEM;
> -			goto err;
> -		}
> +			bigbuffer = (unsigned char *) desc;
> +			desc = NULL;
> +			length = result;

Don't keep the entire 255-byte buffer.  Use krealloc() to shrink the 
buffer down to the right size.

> +		} else {
> +			length = max_t(int, le16_to_cpu(desc->wTotalLength),
> +			    usb_dt_config_size);
> +
> +			/* Now that we know the length, get the whole thing */
> +			bigbuffer = kmalloc(length, GFP_KERNEL);
> +			if (!bigbuffer) {
> +				result = -ENOMEM;
> +				goto err;
> +			}
>  
> -		if (dev->quirks & USB_QUIRK_DELAY_INIT)
> -			msleep(200);
> +			if (dev->quirks & USB_QUIRK_DELAY_INIT)
> +				msleep(200);
>  
> -		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> -		    bigbuffer, length);
> +			result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> +			    bigbuffer, length);
> +		}
>  		if (result < 0) {
>  			dev_err(ddev, "unable to read config index %d "
>  			    "descriptor/%s\n", cfgno, "all");
> @@ -1000,6 +1016,14 @@ int usb_get_configuration(struct usb_device *dev)
>  	}
>  
>  err:
> +	/* Log failed device's VID:PID pair to make it easy to debug and fix
> +	 * enumeration and initialization issues
> +	 */
> +	if (result < 0) {
> +		dev_err(ddev, "Failed to initialize device %04x:%04x due to above errors.",

The "due to above errors" part isn't needed, since the errors will be 
obvious in the kernel log.  In fact, it probably would be better not to 
put this information here at all but instead modify the error message in 
usb_enumerate_device() (the caller).

> +		    le16_to_cpu(dev->descriptor.idVendor), le16_to_cpu(dev->descriptor.idProduct));
> +	}
> +
>  	kfree(desc);
>  	dev->descriptor.bNumConfigurations = cfgno;
>  
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index 87810eff974e..92219684a604 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -142,6 +142,9 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp)
>  				break;
>  			case 'q':
>  				flags |= USB_QUIRK_FORCE_ONE_CONFIG;
> +				break;
> +			case 'r':
> +				flags |= USB_QUIRK_CONFIG_SIZE;

For good style, there should be a "break" statement here.

Also, you need to document the new flag under the usbcore.quirks entry 
in Documentation/admin-guide/kernel-parameters.txt.

>  			/* Ignore unrecognized flag characters */
>  			}
>  		}
> diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
> index b3cc7beab4a3..f864571da870 100644
> --- a/include/linux/usb/quirks.h
> +++ b/include/linux/usb/quirks.h
> @@ -81,4 +81,8 @@
>  /* Device claims zero configurations, forcing to 1 */
>  #define USB_QUIRK_FORCE_ONE_CONFIG		BIT(18)
>  
> +/* Use a 255 byte sized config descriptor request */
> +#define USB_QUIRK_CONFIG_SIZE			BIT(19)
> +#define USB_DT_CONFIG_SIZE_QUIRK		255

Again, I don't like this name.  It's not a quirk in the size of the 
configuration descriptor type, which is what "USB_DT_CONFIG_SIZE" stands 
for; it's a quirk in the way the kernel asks for config descriptors.  
(Or in what size request the device will accept, if you prefer.)

And the 255 value doesn't belong in this header file anyway.  It should 
be defined in config.c since that's the only place it gets used.

Alan Stern

> +
>  #endif /* __LINUX_USB_QUIRKS_H */
> -- 
> 2.54.0
>