Potential bug in USB_DeviceGetDescriptor() function used in many usb_examples projects...

EdSutter — Tue, 18 May 2021 16:18:28 GMT

I was looking at the function USB_DeviceGetDescriptor() in the file usb_examlpes/usb_device_composite_hid_audio_unified_lite/usb_device_descriptor.c...

(note that this probably applies to all the projects that have this function)

The code that is used in the switch case USB_DESCRIPTOR_TYPE_STRING seems at best to be confusing, and potentially hazardous if the device supports more than one language...

As best I can tell, the host passes the device its language ID in setup->wIndex. The device looks for a match in the languageList, and if found, it returns the requested string. If this is not the purpose of this snippet of code then everything that I say below is bogus so stop here (and reply with an explanation please, and feel free to call me an idiot)...

The loop uses USB_DEVICE_STRING_COUNT for no real reason, but worse than that, could potentially be hazardous. Following is the code from the SDK...

usb_status_t error = kStatus_USB_Success; uint8_t descriptorType = (uint8_t)((setup->wValue & 0xFF00U) >> 8U); uint8_t descriptorIndex = (uint8_t)((setup->wValue & 0x00FFU)); if (USB_REQUEST_STANDARD_GET_DESCRIPTOR != setup->bRequest) { return kStatus_USB_InvalidRequest; } switch (descriptorType) { case USB_DESCRIPTOR_TYPE_STRING: /* Get string descriptor */ if (0U == descriptorIndex) { *buffer = (uint8_t *)g_UsbDeviceLanguageList.languageString; *length = g_UsbDeviceLanguageList.stringLength; } else { uint8_t languageIndex = 0U; uint8_t stringIndex = USB_DEVICE_STRING_COUNT; for (; languageIndex < USB_DEVICE_LANGUAGE_COUNT; languageIndex++) { if (setup->wIndex == g_UsbDeviceLanguageList.languageList[languageIndex].languageId) { if (descriptorIndex < USB_DEVICE_STRING_COUNT) { stringIndex = descriptorIndex; } break; } } if (USB_DEVICE_STRING_COUNT == stringIndex) { return kStatus_USB_InvalidRequest; } *buffer = (uint8_t *)g_UsbDeviceLanguageList.languageList[languageIndex].string[stringIndex]; *length = g_UsbDeviceLanguageList.languageList[languageIndex].length[stringIndex]; } break;

The error condition is hit if the incoming language index (setup->wIndex) does not find a match with the targets' set of supported languages. This naturally occurs if the loop increments through to its max value; but instead of testing for that, the code tests for...

(USB_DEVICE_STRING_COUNT == languageIndex)

Several confusing points IMHO here:
1. Its confusing (just test for languageIndex >= USB_DEVICE_LANGUAGE_COUNT)
2. If the target supports several languages but only a few strings, then the loop could legitimately have languageIndex == USB_DEVICE_STRING_COUNT.

For anyone that agrees, here's what I use in place of the above code (I believe this fixes the error case only encountered if multiple languages are defined):

uint8_t descriptorType = (uint8_t)((setup->wValue & 0xFF00U) >> 8U); uint8_t stringIdx = (uint8_t)((setup->wValue & 0x00FFU)); usb_status_t ret = kStatus_USB_Success; if (USB_REQUEST_STANDARD_GET_DESCRIPTOR != setup->bRequest) { return kStatus_USB_InvalidRequest; } switch (descriptorType) { case USB_DESCRIPTOR_TYPE_STRING: /* Get string descriptor */ if (0U == stringIdx) { *buffer = (uint8_t *)g_UsbDeviceLanguageList.languageString; *length = g_UsbDeviceLanguageList.stringLength; } else { uint8_t languageIdx; if (stringIdx >= USB_DEVICE_STRING_COUNT) return kStatus_USB_InvalidRequest; for (languageIdx=0; languageIdx < USB_DEVICE_LANGUAGE_COUNT; languageIdx++) { if (setup->wIndex == g_UsbDeviceLanguageList.languageList[languageIdx].languageId) break; } if (languageIdx >= USB_DEVICE_LANGUAGE_COUNT) return kStatus_USB_InvalidRequest; *buffer = (uint8_t *)g_UsbDeviceLanguageList.languageList[languageIndex].string[stringIndex]; *length = g_UsbDeviceLanguageList.languageList[languageIndex].length[stringIndex]; } break;

Re: Potential bug in USB_DeviceGetDescriptor() function used in many usb_examples projects...

danielchen — Fri, 28 May 2021 08:36:40 GMT

Thank you very much for your feedback. I reported the potential issue to the software team.

Regards

Daniel

Re: Potential bug in USB_DeviceGetDescriptor() function used in many usb_examples projects...

danielchen — Mon, 31 May 2021 06:18:11 GMT

It seems your code is based on an old version. The latest version is SDK 2.9.2.

Regards

Daniel

Re: Potential bug in USB_DeviceGetDescriptor() function used in many usb_examples projects...

EdSutter — Wed, 02 Jun 2021 20:27:50 GMT

The only difference between what I show and the latest SDK (actually its 2.9.3) is a variable name change.

There is no change to the logic, so I believe the issue I mention still applies.

i.MX RT Crossover MCUs中的主题 Re: Potential bug in USB_DeviceGetDescriptor() function used in many usb_examples projects...

Potential bug in USB_DeviceGetDescriptor() function used in many usb_examples projects...

Re: Potential bug in USB_DeviceGetDescriptor() function used in many usb_examples projects...

Re: Potential bug in USB_DeviceGetDescriptor() function used in many usb_examples projects...

Re: Potential bug in USB_DeviceGetDescriptor() function used in many usb_examples projects...