USBH_MSC_ClassRequest looks buggy when using USB host MSC class

[yjsbd]

in usbh_msc.c

if(status == USBH_OK)

   {

      MSC_Handle->max_lun = (MSC_Handle->max_lun > MAX_SUPPORTED_LUN)? MAX_SUPPORTED_LUN : (uint8_t )(MSC_Handle->max_lun) + 1U;

     USBH_UsrLog ("Number of supported LUN: %lu", (int32_t)(MSC_Handle->max_lun));

     for(i = 0U; i < MSC_Handle->max_lun; i++)

     {

       MSC_Handle->unit[i].prev_ready_state = USBH_FAIL;

       MSC_Handle->unit[i].state_changed = 0U;

     }

   }

refer to https://usb.org/sites/default/files/usbmassbulk_10.pdf in page7

Data of GetMaxLun would be 1 byte.

So I think

MSC_Handle->max_lun = (MSC_Handle->max_lun > MAX_SUPPORTED_LUN)? MAX_SUPPORTED_LUN : (uint8_t )(MSC_Handle->max_lun) + 1U;

would be

MSC_Handle->max_lun = ( ((uint8_t)MSC_Handle->max_lun) > MAX_SUPPORTED_LUN)? MAX_SUPPORTED_LUN : (uint8_t )(MSC_Handle->max_lun) + 1U;

Is it right?

[Imen.D]

Hello @yjsbd​ ,

We will check this internally and come back to you with update.

Can you please specify which Cube firmware package is used ?

Kind Regards,

Imen

[yjsbd]

I'm using STM32F4 firmware V1.24

[yjsbd]

Some USB flash drive failed to get read inquiry.

After I modified processing getmaxlun, it works fine

[Imen.D]

Hi,

Absolutely the max_lun is one byte size and the casting is correect for both conditions.

There is no real issue there, as we have already taken in consideration this constrain when sending the max_lun value to host with a size of 1 byte.

We use this fonction call

status = USBH_MSC_BOT_REQ_GetMaxLUN(phost, (uint8_t *)(void *)&MSC_Handle->max_lun);

Kind Regards,

Imen

[waclawek.jan]

Hi @Imen DAHMEN​ ,

The problem here is, that while the GET_MAX_LUN request indeed requests only one byte of data from the device and that one byte arrives, the USB Rx FIFO is accessed in words, so in USB_ReadPacket() that even one byte is in fact read *and stored* as a word.

 for ( i = 0U; i < count32b; i++, dest += 4U )

 {

   *(__packed uint32_t *)dest = USBx_DFIFO(0U);

 }

So, the highmost 3 bytes of MSC_Handle->max_lun get overwritten with whatever garbage happens to currently be in the USB FIFO.

That a whole word is stored in probably the reason why in MSC_HandleTypeDef the max_lun field is uint32_t while just to hold that value it would be enough to declare it as uint8_t.

So, the solution is either to sanitize the value upon usage, as @yjsbd​  outlined above, or in MSC_HandleTypeDef define max_lun as uint8_t and add 3 dummy bytes after it.

JW

[Imen.D]

Hi Jan,

Yes, you are right, the access to the USB FIFO is in 32 bytes, that's why the max_lun is declared in the struct in uint32_t, then with the casting it takes only the first byte received from the device.

Kind Regards,

Imen

[Ede S]

Bug still present, I do not understand why nothing is done since one year to fix it : lost 1 full day looking for solution for a known problem, so sad !...

Imen, you seem to think that nothing has to be done, but if you do not cast to byte size the bolded MSC_Handle->max_lun here as suggested in the top post:

MSC_Handle->max_lun = (MSC_Handle->max_lun > MAX_SUPPORTED_LUN)? MAX_SUPPORTED_LUN : (uint8_t )(MSC_Handle->max_lun) + 1U;

The test is ran then differently, in my case my USB key has MSC_Handle->max_lun == 0, but without the cast, a wrong value is in the bolded field and returned value is then MAX_SUPPORTED_LUN ==> 2 rather than (uint8_t )(MSC_Handle->max_lun) + 1U ==> 1. That keeps my USB process stuck while inquiring for the second SCSI logical unit, for which the USBH_MSC_SCSI_Inquiry() function returns forever UBH_BUSY, and with debug level 3 on USB :

[ 0j 12h 59m 43s 10ms] USB Device Attached

[ 0j 12h 59m 43s 108ms] PID: 5572h

[ 0j 12h 59m 43s 112ms] VID: 781h

[ 0j 12h 59m 43s 112ms] Address (#1) assigned.

[ 0j 12h 59m 43s 115ms] Manufacturer : SanDisk

[ 0j 12h 59m 43s 116ms] Product : Cruzer Switch

[ 0j 12h 59m 43s 118ms] Serial Number : 4C531001450825106275

[ 0j 12h 59m 43s 123ms] Enumeration done.

[ 0j 12h 59m 43s 124ms] This device has only 1 configuration.

[ 0j 12h 59m 43s 126ms] Default configuration set.

[ 0j 12h 59m 43s 130ms] Switching to Interface (#0)

[ 0j 12h 59m 43s 132ms] Class   : 8h

[ 0j 12h 59m 43s 132ms] SubClass : 6h

[ 0j 12h 59m 43s 134ms] Protocol : 50h

[ 0j 12h 59m 43s 136ms] MSC class started.

[ 0j 12h 59m 43s 136ms] Number of supported LUN: 2

[ 0j 12h 59m 43s 140ms] LUN #0:

[ 0j 12h 59m 43s 140ms] Inquiry Vendor : SanDisk

[ 0j 12h 59m 44s 895ms] Inquiry Product : Cruzer Switch

[ 0j 12h 59m 44s 899ms] Inquiry Version : 1.00

[ 0j 12h 59m 44s 901ms] MSC Device ready

[ 0j 12h 59m 44s 904ms] MSC Device capacity : 2779774464 Bytes

[ 0j 12h 59m 44s 907ms] Block number : 30595071

[ 0j 12h 59m 44s 909ms] Block Size  : 512

[ 0j 12h 59m 44s 909ms] LUN #1:

-> stuck here.

If I process to the suggested cast, everything works fine then.

[Ede S]

In case of the cast I get :

[ 0j 13h 41m 16s 449ms] USB Device Attached

[ 0j 13h 41m 16s 546ms] PID: 5572h

[ 0j 13h 41m 16s 550ms] VID: 781h

[ 0j 13h 41m 16s 552ms] Address (#1) assigned.

[ 0j 13h 41m 16s 556ms] Manufacturer : SanDisk

[ 0j 13h 41m 16s 558ms] Product : Cruzer Switch

[ 0j 13h 41m 16s 560ms] Serial Number : 4C531001450825106275

[ 0j 13h 41m 16s 562ms] Enumeration done.

[ 0j 13h 41m 16s 564ms] This device has only 1 configuration.

[ 0j 13h 41m 16s 568ms] Default configuration set.

[ 0j 13h 41m 16s 570ms] Switching to Interface (#0)

[ 0j 13h 41m 16s 572ms] Class   : 8h

[ 0j 13h 41m 16s 574ms] SubClass : 6h

[ 0j 13h 41m 16s 574ms] Protocol : 50h

[ 0j 13h 41m 16s 576ms] MSC class started.

[ 0j 13h 41m 16s 578ms] Number of supported LUN: 1

[ 0j 13h 41m 16s 580ms] LUN #0:

[ 0j 13h 41m 16s 580ms] Inquiry Vendor : SanDisk

[ 0j 13h 41m 21s 299ms] Inquiry Product : Cruzer Switch

[ 0j 13h 41m 21s 303ms] Inquiry Version : 1.00

[ 0j 13h 41m 21s 305ms] MSC Device ready

[ 0j 13h 41m 21s 307ms] MSC Device capacity : 2779774464 Bytes

[ 0j 13h 41m 21s 309ms] Block number : 30595071

[ 0j 13h 41m 21s 311ms] Block Size  : 512

[ 0j 13h 41m 21s 317ms] Erasing flash data, please wait... <== this is my behavior in HOST_USER_CLASS_ACTIVE that was never reached before, stuck in HOST_USER_CLASS_SELECTED