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

HAL_ETH_ReleaseTxPacket() can present the PTP TX timestamp of a different packet to the application code if preempted #73

Closed
KarstenHohmeier opened this issue Sep 8, 2024 · 1 comment
Assignees
Labels
bug Something isn't working eth Ethernet-related issue or pull-request hal HAL-LL driver-related issue or pull-request.

Comments

@KarstenHohmeier
Copy link

I described several locking and concurrency problems of HAL_ETH in issue #72 #71 #70 and #69. This is another one related to the improper or missing locking via gState and HAL_ETH_STATE_BUSY.

If HAL_ETH_ReleaseTxPacket() gets preempted and another call to itself in another threads get scheduled (the functions is racing with itself), then the PTP timestamps stored in heth->TxTimestamp may be overwritten with those of another packet before the application callback HAL_ETH_TxPtpCallback() or heth->txPtpCallback() in the original call to HAL_ETH_ReleaseTxPacket() gets executed.

https://github.com/STMicroelectronics/stm32h7xx_hal_driver/blob/ceda3ceeca2ee77a76d2de2ee6b560ad87c90a48/Src/stm32h7xx_hal_eth.c#L1437-L1467

This would cause the application code to read the overwritten PTP timestamp and not the timestamp of the packet it actually sent.

This problem could be solved by either locking properly and atomically via gState or by not storing the timestamp in heth->TxTimestamp but in a stack variable in HAL_ETH_ReleaseTxPacket() and handing that stack pointer to the application which would be unique per thread.

@ALABSTM ALABSTM added bug Something isn't working hal HAL-LL driver-related issue or pull-request. eth Ethernet-related issue or pull-request labels Sep 9, 2024
@ASEHSTM
Copy link
Contributor

ASEHSTM commented Sep 16, 2024

Hello @KarstenHohmeier,

Thank you for your report. The HAL library is often used in contexts where the end user manages the synchronization of calls to HAL functions. If your application operates in a multi-threaded environment, we recommend using external synchronization mechanisms, such as mutexes or semaphores, to protect calls to HAL_ETH_ReleaseTxPacket().

With Regards,

@ASEHSTM ASEHSTM moved this from To do to Analyzed in stm32cube-mcu-hal-dashboard Sep 16, 2024
@ASEHSTM ASEHSTM closed this as not planned Won't fix, can't repro, duplicate, stale Oct 7, 2024
@github-project-automation github-project-automation bot moved this from Analyzed to Done in stm32cube-mcu-hal-dashboard Oct 7, 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 eth Ethernet-related issue or pull-request hal HAL-LL driver-related issue or pull-request.
Projects
Development

No branches or pull requests

3 participants