STMicroelectronics / stm32l0xx-hal-driver

Provides the STM32Cube MCU Component "hal_driver" of the STM32L0 series.
BSD 3-Clause "New" or "Revised" License
10 stars 2 forks source link

I2C Slave mode problem while receiving data - buffer overrun #5

Closed tdjastrzebski closed 1 year ago

tdjastrzebski commented 3 years ago

While implementing I2C slave mode solution presented in I2C_TwoBoards_RestartAdvComIT official example and the one described here and here I noticed that calling HAL_I2C_Slave_Seq_Receive_IT() may cause buffer overrun leading to unpredictable behavior including program crash. In brief, received byte value is stored twice at two consecutive addresses.
Here is the detailed call sequence when receiving the last frame (pseudocode):

-> HAL_I2C_EV_IRQHandler();
   -> I2C_Slave_ISR_IT();
      -> I2C_ITSlaveCplt();
         // problem 1: I2C_FLAG_RXNE flag is cleared but on local/temp flags copy only
         tmpITFlags &= ~I2C_FLAG_RXNE;
         *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->RXDR;
         pBuffPtr++;
         -> I2C_ITSlaveSeqCplt();
            -> HAL_I2C_SlaveRxCpltCallback();
               // problem 2: HAL_I2C_Slave_Seq_Receive_IT is called when it is already known that there is no more data 
               -> HAL_I2C_Slave_Seq_Receive_IT(Size=1)
                  hi2c->XferCount = Size; // here XferCount is set to 1 again while receiving of the current frame is not finished
        <- HAL_I2C_SlaveRxCpltCallback();
         <- I2C_ITSlaveSeqCplt();
      <- I2C_ITSlaveCplt();
   <- I2C_Slave_ISR_IT();
      // problem 3: data from RXDR register is stored again in buffer at next position (buffer pointer previously increased) without checking that I2C_FLAG_RXNE flag is still set
      if (hi2c->XferCount > 0U) *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->RXDR;
<- HAL_I2C_EV_IRQHandler();

2C_NEXT_FRAME and I2C_FIRST_FRAME flags appear to be equivalent - I found no difference in HAL code, both values are checked everywhere.

Package: STM32Cube_FW_L0_V1.12.1

ASELSTM commented 2 years ago

Hi @tdjastrzebski,

Sorry for this delayed answer. Please find our development team comments.

1.  -> I2C_ITSlaveCplt();
         // problem 1: I2C_FLAG_RXNE flag is cleared but on local/temp flags copy only
         tmpITFlags &= ~I2C_FLAG_RXNE;

Here, the I2C_ITSlaveCplt() is called because STOP condition is detected, no more data can arrive on the bus. So if any, the last data in the RXNE is flushed. If there is a data on RXNE, it must be at least one size in the buffer if the buffer is well sized.

2. // problem 2: HAL_I2C_Slave_Seq_Receive_IT is called when it is already known that there is no more data 
               -> HAL_I2C_Slave_Seq_Receive_IT(Size=1)

The slave calls this function to prepare a next transaction if it comes. There is no specific activity on the slave on this step excepting enabling the interrupt.

3. hi2c->XferCount = Size; // here XferCount is set to 1 again while receiving of the current frame is not finished

This is actually not an issue. The counter and its associated buffer pointer are prepared just in case.

4. // problem 3: data from RXDR register is stored again in buffer at next position (buffer pointer previously increased) without checking that I2C_FLAG_RXNE flag is still set
      if (hi2c->XferCount > 0U) *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->RXDR;

The pointer has already been reset at call of the HAL_I2C_Slave_Seq_Receive_IT()

With regards,

ASELSTM commented 1 year ago

Hi @tdjastrzebski,

Please allow me to close this thread as no activity. You may reopen it at anytime if you have any details to share with us in order to help you to solve the issue. Thank you for your comprehension.

With regards,