The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: Nikhil Solanke <nikhilsolanke5@gmail.com>
Cc: linux-usb@vger.kernel.org, gregkh@linuxfoundation.org,
	linux-kernel@vger.kernel.org, stern@rowland.harvard.edu,
	corbet@lwn.net, skhan@linuxfoundation.org,
	linux-doc@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH 1/2] usbcore: Add quirk for 255-bytes initial config read
Date: Sun, 26 Jul 2026 18:29:46 +0200	[thread overview]
Message-ID: <20260726182946.79a4ef98.michal.pecio@gmail.com> (raw)
In-Reply-To: <20260717195336.98500-2-nikhilsolanke5@gmail.com>

On Sat, 18 Jul 2026 01:23:35 +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 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. 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_WINDOWS_CONFIG_REQ_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: Alan Stern <stern@rowland.harvard.edu>
> Suggested-by: Michal Pecio <michal.pecio@gmail.com>
> Closes: https://lore.kernel.org/linux-usb/CAFgddh+JWdT4LLwMc5qjM8q_pBu-fRo2qADR5ovAKoGHWMQrRw@mail.gmail.com/
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Nikhil Solanke <nikhilsolanke5@gmail.com>
> ---

The code looks generally correct now. Some small issues noted below.

>  .../admin-guide/kernel-parameters.txt         | 10 +++++
>  drivers/usb/core/config.c                     | 39 +++++++++++++++----
>  drivers/usb/core/quirks.c                     |  4 ++
>  include/linux/usb/quirks.h                    |  3 ++
>  4 files changed, 49 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index b5493a7f8f22..14121458a0c0 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -8169,6 +8169,16 @@ Kernel parameters
>  				q = USB_QUIRK_FORCE_ONE_CONFIG (Device
>  					claims zero configurations,
>  					forcing to 1);
> +				r = USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE (Device
> +					fails during initialization when asked for
> +					9-bytes configuration descriptor request.
> +					Ask for 255-bytes request instead to mirror
> +					Windows' behavior. This quirk is originally
> +					meant to fix some quirky gamepads that refuse
> +					to connect in their XInput mode. But it can
> +					also potentially fix issues with other USB
> +					devices that work on Windows but not on
> +					Linux);

I still think this text is needlessly long. Could be:

				r = USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE (Device
					doesn't respond to 9-byte configuration
					descriptor requests, ask for 255 bytes like
					Windows does. Required by some gamepads);

>  			Example: quirks=0781:5580:bk,0a5c:5834:gij
>  
>  	usbhid.mousepoll=
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 45e20c6d76c0..442c15f92ccd 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -912,6 +912,17 @@ int usb_get_configuration(struct usb_device *dev)
>  	unsigned char *bigbuffer;
>  	struct usb_config_descriptor *desc;
>  	int result;
> +	size_t usb_config_req_size;
> +
> +	/*
> +	 * Devices with quirky firmware will stall or reset when the initial

Has anyone actually seen a device that stalls?

> +	 * config descriptor request uses wLength=9. If the quirk is set, use
> +	 * 255 instead, mirroring the behavior of Windows.
> +	 */

If the USB_DT_CONFIG_SIZE constant is moved here then it could make
sense to also move the related comment and merge with this one. Say,

	/*
	 * We start with grabbing the first descriptor so we know how
	 * long the whole configuration is. Some devices don't respond,
	 * request 255 bytes instead, mirroring the behavior of Windows.
	 */

> +	if (dev->quirks & USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE)
> +		usb_config_req_size = 255;
> +	else
> +		usb_config_req_size = USB_DT_CONFIG_SIZE;
>  
>  	if (ncfg > USB_MAXCONFIG) {
>  		dev_notice(ddev, "too many configurations: %d, "
> @@ -938,15 +949,19 @@ int usb_get_configuration(struct usb_device *dev)
>  	if (!dev->rawdescriptors)
>  		return -ENOMEM;
>  
> -	desc = kmalloc(USB_DT_CONFIG_SIZE, GFP_KERNEL);
> +	desc = kmalloc(usb_config_req_size, GFP_KERNEL);
>  	if (!desc)
>  		return -ENOMEM;
>  
>  	for (cfgno = 0; cfgno < ncfg; cfgno++) {
> -		/* We grab just the first descriptor so we know how long
> -		 * the whole configuration is */
> +		/*
> +		 * Normally we request only the configuration descriptor header
> +		 * so we can determine the total configuration length. For
> +		 * devices with USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE set, try to
> +		 * grab the full descriptor set instead.
> +		 */

... and then this comment wouldn't really be necessary, or it could
be shorter like
		/* First request for an initial prefix */

Also, the full descriptor set may be up to 65535 bytes long and in
some classes like UVC it commonly exceeds 255 bytes

>  		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> -		    desc, USB_DT_CONFIG_SIZE);
> +		    desc, usb_config_req_size);
>  		if (result < 0) {
>  			dev_err(ddev, "unable to read config index %d "
>  			    "descriptor/%s: %d\n", cfgno, "start", result);
> @@ -956,9 +971,8 @@ int usb_get_configuration(struct usb_device *dev)
>  			dev->descriptor.bNumConfigurations = cfgno;
>  			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);
> +			dev_err(ddev, "config index %d descriptor too short (asked for %zu, got %i, need at least %i)\n",
> +			    cfgno, usb_config_req_size, result, 4);

There is no need to use %i if you always pass a constant 4.

There is probably no need to print "need at least 4" at all. Users
don't care that we request 9 or 255 bytes but tolarate just 4.

No compliant device will ever respond with less than 9, so it's not
clear if accepting 4 is even necessary for some very bad devices, or
was merely done out of paranoia. It's an irrelevant detail.

>  			result = -EINVAL;
>  			goto err;
>  		}
> @@ -972,6 +986,16 @@ int usb_get_configuration(struct usb_device *dev)
>  			goto err;
>  		}
>  
> +		/*
> +		 * If the device returns the full configuration descriptor set,
> +		 * skip the second read. Otherwise, send a second request
> +		 * asking for the full set.
> +		 */

There is already a comment about "getting the whole thing" a few lines
above, so half of this is redundant.

> +		if (result >= length) {
> +			memcpy(bigbuffer, desc, length);
> +			goto store_and_parse;
> +		}
> +

Even if no device currently uses both quirks, I think this belongs
after the msleep() below, to minimize interaction between the quirks.

>  		if (dev->quirks & USB_QUIRK_DELAY_INIT)
>  			msleep(200);
>  
> @@ -989,6 +1013,7 @@ int usb_get_configuration(struct usb_device *dev)
>  			length = result;
>  		}
>  
> +store_and_parse:
>  		dev->rawdescriptors[cfgno] = bigbuffer;
>  
>  		result = usb_parse_configuration(dev, cfgno,
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index 87ee2d938bc0..f5a60ccf21d3 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -142,6 +142,10 @@ 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_WINDOWS_CONFIG_REQ_SIZE;
> +				break;
>  			/* Ignore unrecognized flag characters */
>  			}
>  		}
> diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
> index b3cc7beab4a3..a4043b33c2c2 100644
> --- a/include/linux/usb/quirks.h
> +++ b/include/linux/usb/quirks.h
> @@ -81,4 +81,7 @@
>  /* Device claims zero configurations, forcing to 1 */
>  #define USB_QUIRK_FORCE_ONE_CONFIG		BIT(18)
>  
> +/* Use a 255 bytes config descriptor request mirroring windows behavior */
> +#define USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE	BIT(19)
> +
>  #endif /* __LINUX_USB_QUIRKS_H */
> -- 
> 2.55.0
> 

  reply	other threads:[~2026-07-26 16:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-17 19:53 [PATCH 0/2] usbcore: Add quirk for 255-byte initial config read Nikhil Solanke
2026-07-17 19:53 ` [PATCH 1/2] usbcore: Add quirk for 255-bytes " Nikhil Solanke
2026-07-26 16:29   ` Michal Pecio [this message]
2026-07-17 19:53 ` [PATCH 2/2] USB: hub: Split announce_device() to log device identity before enumeration Nikhil Solanke
2026-07-18  5:30 ` [PATCH 0/2] usbcore: Add quirk for 255-byte initial config read Nikhil Solanke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260726182946.79a4ef98.michal.pecio@gmail.com \
    --to=michal.pecio@gmail.com \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=nikhilsolanke5@gmail.com \
    --cc=skhan@linuxfoundation.org \
    --cc=stable@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox