hal_usb.c: truncate GET_DESCRIPTOR reply to wLength Topic is solved

[Dron_Gus]

Some host may request first two bytes of descriptor to check it size and then do full-size request.
Respect request wLenght

Code: Select all

Subject: [PATCH] usb: truncate string descriptor response to match
 setup.wLength

Windows requests the first 2 bytes of a string descriptor to read
its size before asking for the full string. This fix ensures the
firmware truncates the response data to match wLength, preventing
endpoint stalls and timeouts on strict hosts.

---
 os/hal/src/hal_usb.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/os/hal/src/hal_usb.c b/os/hal/src/hal_usb.c
index 033977449..173a83d54 100644
--- a/os/hal/src/hal_usb.c
+++ b/os/hal/src/hal_usb.c
@@ -126,6 +126,8 @@ static bool default_handler(USBDriver *usbp) {
     return true;
   case (uint32_t)USB_RTYPE_RECIPIENT_DEVICE | ((uint32_t)USB_REQ_GET_DESCRIPTOR << 8):
   case (uint32_t)USB_RTYPE_RECIPIENT_INTERFACE | ((uint32_t)USB_REQ_GET_DESCRIPTOR << 8):
+  {
+    size_t wLength;
     /* Handling descriptor requests from the host.*/
     dp = usbp->config->get_descriptor_cb(usbp, usbp->setup[3],
                                          usbp->setup[2],
@@ -133,10 +135,12 @@ static bool default_handler(USBDriver *usbp) {
     if (dp == NULL) {
       return false;
     }
+    wLength = get_hword(&usbp->setup[6]);
     /*lint -save -e9005 [11.8] Removing const is fine.*/
-    usbSetupTransfer(usbp, (uint8_t *)dp->ud_string, dp->ud_size, NULL);
+    usbSetupTransfer(usbp, (uint8_t *)dp->ud_string, wLength < dp->ud_size ? wLength : dp->ud_size, NULL);
     /*lint -restore*/
     return true;
+  }
   case (uint32_t)USB_RTYPE_RECIPIENT_DEVICE | ((uint32_t)USB_REQ_GET_CONFIGURATION << 8):
     /* Returning the last selected configuration.*/
     usbSetupTransfer(usbp, &usbp->configuration, 1, NULL);
-- 
2.43.0

[Dron_Gus]

https://learn.microsoft.com/en-us/windo ... escriptors

[Giovanni]

Hi,

From our "sheriff" reviewer:

Thanks for the analysis. In mainline ChibiOS this is already handled — EP0 data-stage replies are truncated to the host's wLength by the EP0 engine, not the request handler. In _usb_ep0setup() (os/hal/src/hal_usb.c):

max = (size_t)get_hword(&usbp->setup[6]); /* wLength */
/* The transfer size cannot exceed the specified amount. */
if (usbp->ep0n > max) {
usbp->ep0n = max;
}

So when default_handler() calls usbSetupTransfer(usbp, dp->ud_string, dp->ud_size, NULL) for GET_DESCRIPTOR, the engine still sends only min(ud_size, wLength) — a wLength-limited request (e.g. Windows reading the first 2 bytes for the size) gets exactly 2 bytes. The newer usbEp0Reply()/ep0_reply_or_ack() path and the XHAL USB driver cap the same way. This cap has been in place since ~16.1.x, so it is on main and all stable-* branches; the handler-level truncation you propose would be redundant here.

If you're seeing the stall, it's most likely a downstream/vendored ChibiOS or an older version that predates this cap — check _usb_ep0setup() in your hal_usb.c for the "transfer size cannot exceed the specified amount" block, or update.

BTW, now it is also possible to submit PRs on the new github repository now (forum is still fine): https://github.com/chibios-upstream/chibios

Giovanni

[Dron_Gus]

Hi,

From our "sheriff" reviewer:

...

BTW, now it is also possible to submit PRs on the new github repository now (forum is still fine): https://github.com/chibios-upstream/chibios

Giovanni

Hi,
My apologies. I overlooked checking the lower layer.