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

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

[Nikhil Solanke]

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@gmail.com>
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>
---
 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;
 
 	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 */
 		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,
+			    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.
+		 */
+		if (dev->quirks & USB_QUIRK_CONFIG_SIZE
+				&& result == le16_to_cpu(desc->wTotalLength)
+				&& result < USB_DT_CONFIG_SIZE_QUIRK) {
 
-		/* 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;
+		} 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.",
+		    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;
 			/* 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
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.54.0

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@gmail.com>

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@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>
> ---
>  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
> 

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

[Nikhil Solanke]

> 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@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>
> > ---
> >  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.

I will explain the quirk above where the variable is defined and reference
it here. A reader would probably question about the quirk there.

>
> >               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.

For this branch tho, we are expecting atleast 9 bytes. if we don't get
those we simply bail out. expected is the right word here. But this was the
originial implementation. With the introduction of the quirk, the wording
of it does fall apart. Instead of adding more branches just to rename a log
message, let's just keep it as "expected"? In a similar branch later on,
when we ask for the bigbuffer, it does make sense to use "asked for" like
you suggested. I will make the change there.

>
> > +                         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
>          */
>

Alright. About the comments formatting, all other comments were not
following a consistent format in the same file. so I didn't bother to fix
such small style changes. But I will still fix them in the way you (and
kernel code style guidelines) say.

> 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.

In the case where wTotalLength < result, wouldn't it be better to consider
the result value as the truth? Or are there scenarios where the device or
the buffer will contain gibberish just to fill it, which is why you
suggested a smaller value. I did understand the part that it should be
atleast USB_DT_CONFIG_SIZE because its the header, but isn't that part
already handled above with result < 4? It does ensure all the critical
fields are actually present. And with just the second test, the code will
naturally jump to the else branch for any cases like you mentioned. I will
change the max_t back to use USB_DT_CONFIG_SIZE and everything seems to be
covered now? Also looking at the 3rd test now, it is actually redundant.
Thanks for pointing that out.


> >
> > -             /* 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.

I did intially though of using krealloc(), but when looked at existing
implementation, bigbuffer is alloced with wTotalLength while ensuring its
atleast USB_DT_CONFIG_SIZE (9) bytes. Then when we receive the result, the
bigbuffer isn't realloced as per the size we received. So I tried to mirror
this exising behavior in fear that I might mess up something else while
trying to be smart. (Although yea, it is waste of memory).

> 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).

Alright

> > 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.

Yea I do agree. Then again I tried to keep my changes in respect to what
was like originally. I will add the missing break then.

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

Oh sorry, totally missed it. Working on it.

> 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.

So what about USB_CONFIG_REQ_SIZE? it's what you had suggested before in
earlier conversations (without the QUIRK word in between naming our magic
number 255).

Nikhil Solanke

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

[Alan Stern]

On Sat, Jun 20, 2026 at 12:38:00PM +0530, Nikhil Solanke wrote:
> > > @@ -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.
> 
> I will explain the quirk above where the variable is defined and reference
> it here. A reader would probably question about the quirk there.

Okay, so long as you don't keep a comment that is now incorrect.

> > > @@ -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.
> 
> For this branch tho, we are expecting atleast 9 bytes. if we don't get
> those we simply bail out. expected is the right word here. But this was the
> originial implementation. With the introduction of the quirk, the wording
> of it does fall apart. Instead of adding more branches just to rename a log
> message, let's just keep it as "expected"? In a similar branch later on,
> when we ask for the bigbuffer, it does make sense to use "asked for" like
> you suggested. I will make the change there.

Again, I don't care too much about the exact wording; I just want to 
make sure that the log message isn't actually wrong.  Saying "expected 
255" would definitely be wrong, but saying "expected at least 9" would 
be okay.

> > > +                         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
> >          */
> >
> 
> Alright. About the comments formatting, all other comments were not
> following a consistent format in the same file. so I didn't bother to fix
> such small style changes. But I will still fix them in the way you (and
> kernel code style guidelines) say.

Yeah, don't bother to fix up comments that the patch doesn't otherwise 
touch -- that would be mere pointless churn and would make reviewing the 
patch more difficult for no good reason.  But comments that you _do_ 
alter are fair game for reformatting.

> > 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.
> 
> In the case where wTotalLength < result, wouldn't it be better to consider
> the result value as the truth? Or are there scenarios where the device or
> the buffer will contain gibberish just to fill it, which is why you
> suggested a smaller value.

Sort of.  Software that parses the descriptors later on will ignore the 
extra stuff in any case, limiting itself to just the first wTotalLength 
bytes.  So there's not really any need to keep the excess.

>  I did understand the part that it should be
> atleast USB_DT_CONFIG_SIZE because its the header, but isn't that part
> already handled above with result < 4? It does ensure all the critical
> fields are actually present.

The critical fields are not just the first 4 bytes, but the first 9 
bytes.  If you want, you can change that test against 4 above, making it 
against USB_DT_CONFIG_SIZE.  The reason the existing code only tests 
against 4 is because it knows that it will issue another request for the 
full length, but that won't always be true after your changes.

>  And with just the second test, the code will
> naturally jump to the else branch for any cases like you mentioned. I will
> change the max_t back to use USB_DT_CONFIG_SIZE and everything seems to be
> covered now? Also looking at the 3rd test now, it is actually redundant.
> Thanks for pointing that out.
> 
> 
> > >
> > > -             /* 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.
> 
> I did intially though of using krealloc(), but when looked at existing
> implementation, bigbuffer is alloced with wTotalLength while ensuring its
> atleast USB_DT_CONFIG_SIZE (9) bytes. Then when we receive the result, the
> bigbuffer isn't realloced as per the size we received. So I tried to mirror
> this exising behavior in fear that I might mess up something else while
> trying to be smart. (Although yea, it is waste of memory).

You're right that the existing code could have reallocated the buffer if 
it received less than it asked for.  I believe it doesn't bother to do 
this because that possibility is considered to be very unlikely.

In fact, you could arrange to do the check and reallocation after the 
two code paths merge back together.  That would be the best solution.

> > 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.
> 
> So what about USB_CONFIG_REQ_SIZE? it's what you had suggested before in
> earlier conversations (without the QUIRK word in between naming our magic
> number 255).

Yes, that seems like a better choice for the name.  Or maybe 
USB_CONFIG_BIG_REQ_SIZE or USB_CONFIG_WINDOWS_REQ_SIZE, to indicate this 
isn't the value we normally use but copies the behavior of Windows.

Alan Stern