search

STMicroelectronics / stm32h7xx-hal-driver

Provides the STM32Cube MCU Component "hal_driver" of the STM32H7 series.
BSD 3-Clause "New" or "Revised" License
98 stars 43 forks source link

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

Closed KarstenHohmeier closed 1 month ago

KarstenHohmeier commented 2 months ago

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 gStateand 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.

ASEHSTM commented 2 months ago

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,