STMicroelectronics / STM32CubeF4

STM32Cube MCU Full Package for the STM32F4 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
Other
898 stars 424 forks source link

USB MTP driver. Multiple bugs #180

Open Kun-Kun opened 2 months ago

Kun-Kun commented 2 months ago

Certainly a bug: https://github.com/STMicroelectronics/STM32CubeF4/blob/82738194a5e85ee5331975b04905daa0a17053ec/Middlewares/ST/STM32_USB_Device_Library/Class/MTP/Src/usbd_mtp_storage.c#L463 At the file transfer such statement calculates buffer size in a wrong way. It always 12 bytes since the header is always less than Header+data. I offer to remove this line and to invoke as (void)USBD_LL_Transmit(pdev, MTPInEpAdd, buf, len); Because at file transfer there might be a situation when there is no header and the last packet of data has length 0-12 bytes.

The second bug is on the: https://github.com/STMicroelectronics/STM32CubeF4/blob/82738194a5e85ee5331975b04905daa0a17053ec/Middlewares/ST/STM32_USB_Device_Library/Class/MTP/Src/usbd_mtp_storage.c#L109 It invokes the function that writes into buffer and sets it size. But at line: https://github.com/STMicroelectronics/STM32CubeF4/blob/82738194a5e85ee5331975b04905daa0a17053ec/Middlewares/ST/STM32_USB_Device_Library/Class/MTP/Src/usbd_mtp_storage.c#L113 We overwrite this buffer with 12 byte header at the start of the file transfer thus corrupts 12 bytes of provided data in the buffer. I offer to change line 109 with: (void)((USBD_MTP_ItfTypeDef *)pdev->pUserData[pdev->classId])->ReadData(hmtp->OperationsContainer.Param1, (uint8_t *)data_buff+MTP_CONT_HEADER_SIZE, &MTP_DataLength); As a result move the pointer by 12 bytes and write buffer in the right section And it is good to notice in ReadData comments that it should return buffer with size 52 at the first invocation,should return 64 bytes at intermediate, and 0-63bytes meaning the last transfer

ALABSTM commented 1 month ago

Hi @Kun-Kun,

Thank you for all these details and proposals. Did you experience failures caused by what you reported as bugs, or are you reporting assumptions based on code inspection?

In case you faced real failures, could you please share a code snippet or a minimal project allowing to reproduce them please?

Thank you,

Kun-Kun commented 3 weeks ago

Hi @ALABSTM I did face issues during the implementation of the MTP on STM32F4DISCOVERY. I had to fix code problems above in this repository to make it work https://github.com/Kun-Kun/STM32F407-MTP-MemMap Unfortunately I have no time to make a more minimal project to prove that it doesn't work with current driver version. But you can simply rollback usbd_mtp_storage.c file in this project and verify that it won't transfer a data.

ALABSTM commented 1 week ago

Hi @Kun-Kun,

I forwarded you report to our development teams. I will keep you informed.

With regards,

ALABSTM commented 1 week ago

ST Internal Reference: 195825