STMicroelectronics / stm32h7xx-hal-driver

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

BusFault when calculating a HASH #38

Closed wouter-palmsens closed 10 months ago

wouter-palmsens commented 1 year ago

Description of the set-up

Description of the bug

The (static) function HASH_WriteData() has an uint8_t * buffer argument, but it reads the data 4 bytes at a time, as can be seen in the code:

static HAL_StatusTypeDef HASH_WriteData(HASH_HandleTypeDef *hhash, uint8_t *pInBuffer, uint32_t Size)
{
  uint32_t buffercounter;
  __IO uint32_t inputaddr = (uint32_t) pInBuffer;

  for (buffercounter = 0U; buffercounter < Size; buffercounter += 4U)
  {
    /* Write input data 4 bytes at a time */
    HASH->DIN = *(uint32_t *)inputaddr;
    inputaddr += 4U;
    // ...

Note that the pointer cast violates the strict aliasing rule and thus invokes Undefind Behavior, but that's not the real problem here. The actual problem is that more data is read then required in case Size is not a multiple of 4. When pInBuffer is at the end of a memory region and not 32-bit aligned, this can cause a BusFault. For example, when the buffer is located at the end of the AXI SRAM on the STM32H753, reading one byte too much will cause the MCU to read from Reserved memory and thus cause a BusFault.

The following example code should demonstrate this:

uint8_t pOutBuffer[32];

uint8_t *pInBuffer = (uint8_t *)0x2407FFD1; // 47 bytes before the end of AXI SRAM
uint32_t Size = 45; // 45 bytes (so the input buffer is still completely inside AXI SRAM)

HASH_Start(&hhash, pInBuffer, Size, pOutBuffer, HAL_MAX_DELAY, HASH_ALGOSELECTION_SHA256); // will crash

A solution would be to handle the last 1-3 bytes (in case Size is not a multiple of 4) separately, thereby avoiding reading out-of-bounds.

Screenshots

image

TOUNSTM commented 1 year ago

Hello @wouter-palmsens,

Thank you for this report. We will get back to you as soon as we analyze it further. This may take some time. Thank you for your comprehension.

With regards,

HBOSTM commented 1 year ago

ST Internal Reference: 154639

HBOSTM commented 1 year ago

Hello @wouter-palmsens ,

Thank you for this contribution, this point has been reported to our development teams. I will get back to you as soon as I have any updates.

Best Regards,

wouter-palmsens commented 1 year ago

Note that my remark about violating the strict aliasing rule might not have been entirely correct, but the pointer cast can definitely cause unaligned memory access, which could result in a UsageFault for memory regions that do not support unaligned access (such as the SDRAM).

TOUNSTM commented 10 months ago

Fixed in 330a298d637ecfd7f992c14cf154596199501255