STMicroelectronics / STM32CubeH7

STM32Cube MCU Full Package for the STM32H7 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))
https://www.st.com/en/embedded-software/stm32cubeh7.html
Other
490 stars 302 forks source link

ethernetif.c issues #231

Closed ra1u closed 4 months ago

ra1u commented 2 years ago

We have detected several issues in ethernetif.c For refernce we used this on NUCLEO-H743ZI, but bug likley applies also on on related boards as most of this code is shared.

  1. Race condition in thread responsible for ifup and ifdown . Lwip requres serialised acess when calling netif_set_up and netif_set_link_up

  2. Race condition in memory allocation. As function HAL_ETH_RxAllocateCallback runs in interrupt, it is not synchronized with related LWIP_MEMPOOL_FREE . We observed, that LWIP_MEMPOOL_ALLOC can return memory that was not yet freed, but there might be also other effects of undefined behavior.

  3. There is not clear how synchronization between code running from threads and interrupts is synchronized. It can happen that code in ethernetif_input thread, is interrupted by other thread that sends data followed by interrupt from ISR. There is at least race condition over eth->gState, because such requirement upon function entry can not be satisfied trough function execution.

All this issues results in undefined behavior and unreliable operation.

Looking forward to move this issue forward and find reliable solutions.

Kind regards, Luka

ASELSTM commented 1 year ago

ST Internal Reference: 157151

ASEHSTM commented 5 months ago

Hello @ra1u,

Thank you for your report.

  1. In regard to the first point, you are correct, we need to ensure thread-safe access when calling the netif_xxx() functions. It will be fixed soon.

  2. For the second point, we want to clarify that HAL_ETH_RxAllocateCallback() is not actually called from an interrupt handler. Instead, the interrupt handler calls RX_Callback() which releases a semaphore to inform the thread that a packet is ready for processing. The function is then called to allocate memory for the packet. When using FreeRTOS (NO_SYS = 0), calls for memory allocation are protected using a mutex. The LWIP architecture also handles serialization of Read/Write requests when running in OS context, and the Sequential-style APIs use RTOS facilities to exchange messages with the TCPIP thread.

  3. Finally, in regard to the third point, we want to clarify that HAL_ETH_Read() and HAL_ETH_Transmit() do not change the gState field. They only query the field to check that the ETH peripheral has started. The serialization of Read/Write requests is handled by the LWIP architecture when running in OS context.

We hope this explanation helps clarify the issues you reported. With Regards,

ASEHSTM commented 5 months ago

Hello @ra1u,

Could you please check if the first issue has been resolved in this version?

With Regards,

ASEHSTM commented 4 months ago

Hello @ra1u,

Please allow me to close this issue. You can reopen it at any time if you have any details to share with us.

With Regards,