USB Host MSC class driver fails if device does not respond to Get Max LUN request

[dkonigsberg]

描述一下这个bug / Describe the bug

This issue occurs within the USB Host class driver for MSC (mass storage) devices: https://github.com/cherry-embedded/CherryUSB/blob/master/class/msc/usbh_msc.c#L279

In the usbh_msc_connect() function, a call is made to usbh_msc_get_maxlun() which attempts to perform a "Get Max LUN" request on the USB device. If this request fails, the class driver will not load and the device will not be usable.

On some USB storage devices, this command fails with a USB_ERR_STALL, and thus those devices cannot be used with CherryUSB.

However, according to the USB Mass Storage Class Bulk-Only Transport specification: https://www.usb.org/sites/default/files/usbmassbulk_10.pdf (Page 7, section "3.2 Get Max LUN") "Devices that do not support multiple LUNs may STALL this command."

In other words, a stall error in response to this request should not cause the MSC connect function to fail.

Changing the error handling around usbh_msc_get_maxlun() as follows, fixes the issue:

    ret = usbh_msc_get_maxlun(msc_class, g_msc_buf[msc_class->sdchar - 'a']);
    if (ret == -USB_ERR_STALL) {
        USB_LOG_INFO("Multiple LUNs not supported");
        g_msc_buf[msc_class->sdchar - 'a'][0] = 0;
    } else if (ret < 0) {
        return ret;
    }

    USB_LOG_INFO("Get max LUN:%u\r\n", g_msc_buf[msc_class->sdchar - 'a'][0] + 1);

复现步骤 / To Reproduce

• Start an embedded device running CherryUSB as a host
• Insert a USB thumbdrive that exhibits this behavior
• Observe the logs

Most USB thumbdrives I tested do not have this problem. However, I found one that does. It has a VID=0x058F and PID=0x6387 and was received as a freebie at a tech conference many years ago. However, just knowing that these things exist, makes me worried that an end-user might try to use one.

设备信息 / Target Device

STM32F446RE, DWC2 port, FreeRTOS

日志 / Log

[I/usbh_hub] New full-speed device on Bus 0, Hub 2, Port 1 connected
[I/usbh_core] New device found,idVendor:058f,idProduct:6387,bcdDevice:0102
[I/usbh_core] The device has 1 bNumConfigurations
[I/usbh_core] The device has 1 interfaces
[I/usbh_core] Enumeration success, start loading class driver
[I/usbh_core] Loading msc class driver
[E/usbh_hub] Port 1 enumerate fail

配置 / Configuration

No response

USB中断 / USB Interrupt

No response

缓存 / Cache

No response

商业 / Business

• [ ] 是 / Yes

[sakumisu]

If device support only one lun also need support this command, this is the basic msc request.

[sakumisu]

https://github.com/cherry-embedded/CherryUSB/blob/a8ef0c4cac95a14d76f2c3be9c34c06dd441e7d4/class/msc/usbd_msc.c#L89

[dkonigsberg]

I did a little research to see how several other USB host stacks handle this situation. They all have slight differences in the comments or control flow, but none of them require a successful Get Max LUN request to attach an MSC device.

• TinyUSB
  • A failed Get Max LUN request means one LUN (comment mentions "STALL")
  • https://github.com/hathach/tinyusb/blob/c8ab65fbb6ed24b470df8c3b2f35c27ce0d54053/src/class/msc/msc_host.c#L435-L443
• STMicroelectronics Middleware USB Host
  • A failed Get Max LUN request means one LUN (comment not specific about the type of error)
  • https://github.com/STMicroelectronics/stm32_mw_usb_host/blob/3a524c2f185ac2f193cf3edd0b3f83ed064f3a5c/Class/MSC/Src/usbh_msc.c#L281-L290
• FreeBSD
  • A failed Get Max LUN request doesn't prevent the device from attaching:
  • https://cgit.freebsd.org/src/tree/sys/dev/usb/storage/umass.c#n1031
  • https://cgit.freebsd.org/src/tree/sys/dev/usb/storage/umass.c#n1603
  • FreeBSD has a large "quirks" table explicitly listing devices known to not support "Get Max LUN", which contains my device:
  • https://cgit.freebsd.org/src/tree/sys/dev/usb/quirk/usb_quirk.c#n217
  • USB_QUIRK(ALCOR, TRANSCEND, UQ_MSC_NO_GETMAXLUN, UQ_MSC_NO_SYNC_CACHE, UQ_MSC_NO_TEST_UNIT_READY)
• Linux
  • The request function has a lengthy comment explicitly explaining the behavior of some devices when they get this request:
  • https://github.com/torvalds/linux/blob/11a299a7933e03c83818b431e6a1c53ad387423d/drivers/usb/storage/transport.c#L1075-L1113
  • "Some devices don't like GetMaxLUN. They may STALL the control pipe, they may return a zero-length result, they may do nothing at all and timeout, or they may fail in even more bizarrely creative ways. In these cases the best approach is to use the default value: only one LUN."

[sakumisu]

Thank you for such a detailed explanation，could you pr a patch for this?

[sakumisu]

Done