Race condition in UART_RxISR_8BIT (HAL)

[Blacbird]

The error occurred while reading a UART interface (heavy load @ 115200 baud) byte by byte (with HAL_UART_Receive_IT(&huart1, &com_uartRxSignBuffer, 1) ).

MCU: STM32F302VB ST firmware version : STM32Cube_FW_F3_V1.11.2

Before the RXNE interrupt is disabled when 1 byte is received, the interrupt can be re-triggered by another received byte.

If this happens when huart->RxXferCount is already counted down to 0, huart->RxXferCount is counted down again.

An underflow occurs, obviously the RXNE interrupt is not disabled, and the following 65535 bytes are written to com_uartRxSignBuffer[0 ... 65535].

Almost certainly this will overwrite a pointer. By accessing it, an invalid memory address is accessed, resulting in a hard fault.

The code (I have commented the places where this underflow occurs) that triggered this error (stm32f3xx_hal_uart.c line 3800):

  /* Check that a Rx process is ongoing */
  if (huart->RxState == HAL_UART_STATE_BUSY_RX)
  {
    uhdata = (uint16_t) READ_REG(huart->Instance->RDR); //<< RXNE flag is reset -> IRQ can be triggered again
    *huart->pRxBuffPtr = (uint8_t)(uhdata & (uint8_t)uhMask);
    huart->pRxBuffPtr++;
    huart->RxXferCount--; // << first IRQ: count to 0; second IRQ: count to 0xFFFF;
    //<< second IRQ is triggert ( RXNE-interrupt is not yet disabled)
    if (huart->RxXferCount == 0U) //<< due to the secont IRQ RxXferCount is not 0 -> RXNE-interrupt gets not disabled
    {
      /* Disable the UART Parity Error Interrupt and RXNE interrupts */
      CLEAR_BIT(huart->Instance->CR1, (USART_CR1_RXNEIE | USART_CR1_PEIE)); //<<  RXNE-interrupt would be disabled here, no more Interrupts would arrive

My workaround: before counting down check for huart->RxXferCount is zero already:

 /* Check that a Rx process is ongoing */
  if (huart->RxState == HAL_UART_STATE_BUSY_RX)
  {
    uhdata = (uint16_t) READ_REG(huart->Instance->RDR);
    *huart->pRxBuffPtr = (uint8_t)(uhdata & (uint8_t)uhMask);
    if (huart->RxXferCount >0){
		huart->pRxBuffPtr++;
    	huart->RxXferCount--;
    }
    if (huart->RxXferCount == 0U)
    {
      /* Disable the UART Parity Error Interrupt and RXNE interrupts */
      CLEAR_BIT(huart->Instance->CR1, (USART_CR1_RXNEIE | USART_CR1_PEIE));

The big Problem with this, is that this is not a usercode section, therefore it gets overwritten by CubeMX on regeneration.

Is there a solution, with little overhead which gets not overwritten by CubeMX?

Best regards,

Johannes Strobel

[Piranha]

Especially when the UART has a heavy load, using HAL_UART_Receive_IT() for a single bytes is ridiculously inefficient. The whole HAL API is designed by incompetent "experts", who cannot grasp the concepts of continuous reception and asynchronous processing. For an actually usable solution look here:

https://github.com/MaJerle/stm32-usart-uart-dma-rx-tx

[Guenael Cadier · Accepted solution]

Dear @Blacbird​ 

ST has recently added new HAL UART API in order to manage reception of unknown length (which is usually the reason for using HAL_UART_Receive_IT() with expected rx length value = 1). It is based on reception going till either expected length is received OR IDLE event occurs. it i using similar principle than solution promoted in above provided link : https://github.com/MaJerle/stm32-usart-uart-dma-rx-tx and it is available either in Interrupt or DMA modes.

A usage example is provided in some recent STM32 Cube packages (but maybe not in STM32F3), illustrating use on this new API with a Circular DMA implementation (I think it could be found (for reference) in STM32 Cube package for G0/G4/L5, ... series under the name Examples\UART\UART_ReceptionToIdle_CircularDMA. Maybe it could help you.

Secondly, regarding your initial issue, I will notify it to ST team to check, and correct if needed (in HAL code, so that would remain inline with Mx generation).

Regards

Guenael

[Piranha]

While HAL_UARTEx_ReceiveToIdle...() functions solve the "problem" of unknown length data, the HAL is still incapable of continuous reception. The peripheral is still stopped after every data block received and some data can be lost. Useful API is the one where the peripheral is started once and then the processing code only waits for a callback event and reads the newly received data. No peripheral stopping!

[Amel NASRI]

Internal ticket number: 109147

[ASELM]

Hi @Blacbird​ ,

By reading your suggestion, it seems that your scenario isn't possible as the second interrupt can't preempt the first one (same HW/SW priority interruption) . The described scenario is hardware protected by the Cortex-M4 so no needs for software updates.

Actually, at the end of the first interrupt RXNEIE = 0 (disabled), State = HAL_UART_STATE_READY and RXNE = 0 or 1. So if we suppose that the second interrupt will be executed even when the RXNEIE is disabled, the counter will never be decremented as we have the following check condition on the RxState :

 if (huart->RxState ==HAL_UART_STATE_BUSY_RX)
 {
    huart->RxXferCount--; 

However, we may have missed some details.. Could you please share a project so that we can reproduce the same behavior.

With regards,

[Martin Franke]

Hello ASELM,

I've experienced a similar problem with the related HAL SPI Driver in slave mode. I've found that indeed SPI_RxISR_8BIT(...) can cause the hspi->RxXferCount to decrement from 0 to 0xFFFF (this is very similar to the UART_RxISR_8BIT(...) function described above). In my case it is occurring in STM32H7 based hardware we use to test various projects (Using HAL 1.10.0 for STM32H7) and it happened while I was testing a device with a bad CS line on SPI, hence a data underrun occurred. Our software causes it to retry several times and I find usually while this is occurring at some point it will hit this situation. I would have assumed this wasn't possible due to what you say about the lack of ability to pre-empt an interrupt, however somehow it is getting called again after it has been set to 0, even though it should have disabled the interrupt by that point. Also, HAL_SPI_TransmitReceive_IT() prevents size from being set to 0, (and I'm never setting it to 0 anyway), so I don't think the issue is on that end. I'm still investigating but thought I'd post this to suggest that between the issue above, and mine, you might look a bit closer here. I suggest creating an underrun condition and retyring several times. Attached is a screenshot where I added a catch to show an instance where this occurred.