Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USB MTP driver. Multiple bugs #180

Open
Kun-Kun opened this issue Aug 27, 2024 · 4 comments
Open

USB MTP driver. Multiple bugs #180

Kun-Kun opened this issue Aug 27, 2024 · 4 comments
Assignees
Labels
bug Something isn't working internal bug tracker Issue confirmed and logged into the internal bug tracking system mw Middleware-related issue or pull-request. usb Universal Serial Bus

Comments

@Kun-Kun
Copy link

Kun-Kun commented Aug 27, 2024

Certainly a bug:

uint32_t length = MIN(hmtp->GenericContainer.length, len);

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:

(void)((USBD_MTP_ItfTypeDef *)pdev->pUserData[pdev->classId])->ReadData(hmtp->OperationsContainer.Param1,

It invokes the function that writes into buffer and sets it size.
But at line:
(void)USBD_memcpy((uint8_t *)data_buff, (uint8_t *)&hmtp->GenericContainer, MTP_CONT_HEADER_SIZE);

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

@TOUNSTM TOUNSTM added mw Middleware-related issue or pull-request. usb Universal Serial Bus labels Aug 28, 2024
@ALABSTM
Copy link
Contributor

ALABSTM commented Sep 24, 2024

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,

@ALABSTM ALABSTM added the bug Something isn't working label Sep 24, 2024
@ALABSTM ALABSTM moved this from To do to Analyzed in stm32cube-mcu-fw-dashboard Sep 24, 2024
@ALABSTM ALABSTM added the needs clarification Needs clarification or inputs from the user label Sep 24, 2024
@Kun-Kun
Copy link
Author

Kun-Kun commented Oct 27, 2024

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 ALABSTM added internal bug tracker Issue confirmed and logged into the internal bug tracking system and removed needs clarification Needs clarification or inputs from the user labels Nov 6, 2024
@ALABSTM
Copy link
Contributor

ALABSTM commented Nov 6, 2024

Hi @Kun-Kun,

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

With regards,

@ALABSTM
Copy link
Contributor

ALABSTM commented Nov 6, 2024

ST Internal Reference: 195825

@ALABSTM ALABSTM moved this from Analyzed to In progress in stm32cube-mcu-fw-dashboard Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working internal bug tracker Issue confirmed and logged into the internal bug tracking system mw Middleware-related issue or pull-request. usb Universal Serial Bus
Projects
Status: In progress
Development

No branches or pull requests

3 participants