USBH config descriptor parsing buggy

[maggu2810]

Describe the set-up

• board: stm32f4-discovery
• compiler: arm-none-eabi-gcc (Fedora 12.2.0-1.fc36) 12.2.0

Describe the bug

My OnePlus 8 Pro provides different USB config descriptor dependent on its current situation. I would expect that all the descriptor could be parsed but this seems not to be the case. The problem itself is not hardware dependent. I captured the different config descriptor provided by my phone and extract the parsing code to a stand alone project. So I could inspect the failure in a simple test.

How To Reproduce

1. Use one of the captured USB config descriptor.
2. Use the USBH_ParseCfgDesc function to parse the data.
3. For two of the three different descriptor all is fine, for the third one the function will never return.

Additional context

Here the three USB config descriptors:

const uint8_t dataLength62Status0[] =
        {0x09, 0x02, 0x3E, 0x00, 0x02, 0x01, 0x04, 0x80, 0xFA,//
         0x09, 0x04, 0x00, 0x00, 0x03, 0xFF, 0xFF, 0x00, 0x05,//
         0x07, 0x05, 0x81, 0x02, 0x40, 0x00, 0x00,            //
         0x07, 0x05, 0x01, 0x02, 0x40, 0x00, 0x00,            //
         0x07, 0x05, 0x82, 0x03, 0x1C, 0x00, 0x06,            //
         0x09, 0x04, 0x01, 0x00, 0x02, 0xFF, 0x42, 0x01, 0x06,//
         0x07, 0x05, 0x02, 0x02, 0x40, 0x00, 0x00,            //
         0x07, 0x05, 0x83, 0x02, 0x40, 0x00, 0x00};
const uint8_t dataLength55Status0[] =
        {0x09, 0x02, 0x37, 0x00, 0x02, 0x01, 0x04, 0x80, 0xFA,//
         0x09, 0x04, 0x00, 0x00, 0x02, 0xFF, 0xFF, 0x00, 0x05,//
         0x07, 0x05, 0x81, 0x02, 0x40, 0x00, 0x00,            //
         0x07, 0x05, 0x01, 0x02, 0x40, 0x00, 0x00,            //
         0x09, 0x04, 0x01, 0x00, 0x02, 0xFF, 0x42, 0x01, 0x05,//
         0x07, 0x05, 0x02, 0x02, 0x40, 0x00, 0x00,            //
         0x07, 0x05, 0x82, 0x02, 0x40, 0x00, 0x00};
const uint8_t dataLength124Status4[] =
        {0x09, 0x02, 0x7C, 0x00, 0x03, 0x01, 0x04, 0x80, 0xFA,// USB_DESC_TYPE_CONFIGURATION
         0x09, 0x04, 0x00, 0x00, 0x00, 0x01, 0x01, 0x00, 0x05,// USB_DESC_TYPE_INTERFACE
         0x09, 0x24, 0x01, 0x00, 0x01, 0x09, 0x00, 0x01, 0x01,//
         0x09, 0x04, 0x01, 0x00, 0x02, 0x01, 0x03, 0x00, 0x00,// USB_DESC_TYPE_INTERFACE
         0x07, 0x24, 0x01, 0x00, 0x01, 0x25, 0x00,            //
         0x06, 0x24, 0x02, 0x02, 0x01, 0x00,                  //
         0x09, 0x24, 0x03, 0x01, 0x02, 0x01, 0x01, 0x01, 0x00,//
         0x06, 0x24, 0x02, 0x01, 0x03, 0x00,                  //
         0x09, 0x24, 0x03, 0x02, 0x04, 0x01, 0x03, 0x01, 0x00,//
         0x09, 0x05, 0x01, 0x02, 0x40, 0x00, 0x00, 0x00, 0x00,// USB_DESC_TYPE_ENDPOINT
         0x05, 0x25, 0x01, 0x01, 0x03,                        //
         0x09, 0x05, 0x81, 0x02, 0x40, 0x00, 0x00, 0x00, 0x00,// USB_DESC_TYPE_ENDPOINT
         0x05, 0x25, 0x01, 0x01, 0x02,                        //
         0x09, 0x04, 0x02, 0x00, 0x02, 0xFF, 0x42, 0x01, 0x07,// USB_DESC_TYPE_INTERFACE
         0x07, 0x05, 0x02, 0x02, 0x40, 0x00, 0x00,            // USB_DESC_TYPE_ENDPOINT
         0x07, 0x05, 0x82, 0x02, 0x40, 0x00, 0x00};           // USB_DESC_TYPE_ENDPOINT

The function does not return because the current implementation will lead to the situation that a call to

pdesc = USBH_GetNextDesc((uint8_t *)(void *)pdesc, &ptr);

returns the same pdesc pointer as has been provided. This is caused because pdesc->bLength == 0. This is currently not really be caused by the provided config descriptor but how the descriptor is handled by your parsing code. Regardless which situation results into a length of zero, the function should be hardened to not enter and endless loop. This could be done simple by replacing the two calls to get next desc with code like this:

if (pdesc->bLength == 0)
{
    return USBH_NOT_SUPPORTED;
}
pdesc = USBH_GetNextDesc((uint8_t *)(void *)pdesc, &ptr);

But IMHO the real buggy behaviour is that the config descriptor is not parsed as given but there are some magic corrections

/* Make sure that the endpoint descriptor's bLength is equal to
   USB_ENDPOINT_DESC_SIZE for all other endpoints types */
else if (pdesc->bLength != USB_ENDPOINT_DESC_SIZE)
{
  pdesc->bLength = USB_ENDPOINT_DESC_SIZE;
}

This seems strange to me.

As you can see there are 4 endpoints (type 0x05) in the third descriptor:

         0x09, 0x05, 0x01, 0x02, 0x40, 0x00, 0x00, 0x00, 0x00
         0x09, 0x05, 0x81, 0x02, 0x40, 0x00, 0x00, 0x00, 0x00
         0x07, 0x05, 0x02, 0x02, 0x40, 0x00, 0x00
         0x07, 0x05, 0x82, 0x02, 0x40, 0x00, 0x00

two uses a length of 0x09 and two uses a length of 0x07.

Your code ignores the given length of 0x09 and set USB_ENDPOINT_DESC_SIZE (0x07U) as length. This will corrupt the whole data handling as pdesc / ptr will not be increased by the 9 bytes but only by 7 and so the whole further data is not inspected at the correct byte places.

[NimaAgm]

I have similar problem with this part of the code. Connecting an android as midi usb to stm32 would result in endless loop in this part of the code. I posted my issue in stm32_mw_usb_host .