All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.