Buffer overflow in CubeMX SDMMC-Driver

[Nils Zottmann]

Posted on May 30, 2017 at 18:11

I'm trying to get a SD-Card working with FATFS on a NUCLEO-L476RG. Code was generated with recent STM32CubeMX 4.0 and Firmware Package 1.8.0 for L4. SD-access in polling mode, no DMA.

My application was crashing (Default_Handler, Infinite_Loop) while writing to the card. The crash was happening instm32l4xx_hal_sd.c in HAL_SD_WriteBlocks(), here:

/* Write block(s) in polling mode */
 while(!__HAL_SD_GET_FLAG(hsd, SDMMC_FLAG_TXUNDERR | SDMMC_FLAG_DCRCFAIL | SDMMC_FLAG_DTIMEOUT | SDMMC_FLAG_DATAEND))
 {
 if(__HAL_SD_GET_FLAG(hsd, SDMMC_FLAG_TXFIFOHE))
 {
 /* Write data to SDMMC Tx FIFO */
 for(count = 0U; count < 8U; count++)
 {
 SDMMC_WriteFIFO(hsd->Instance, (tempbuff + count));
 }
 tempbuff += 8U;
 }
 
 [...]
 }�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

After a crash, (tempbuff + count) has the value 0x20018000 (see attachment). This seems to be a invalid address which is not in RAM, access causes the system fault.

As far as I understand the problem, if the DATAEND flag has not been set yet but the buffer was completely written to the SDMMC fifo, tempbuff will get incremented further out of the bounds of the buffer. If the buffer is located at the end of the memory, a system fault occurs when trying to read. Accessing parts of the memory beyond the buffer should never happen in general but if the memory following the buffer is readable, this will probably not be noticed.

My fix is to stop incrementing tempbuff when the buffersize is reached, replace

tempbuff += 8U;�?

with

if( (tempbuff - (uint32_t *)pData) < (NumberOfBlocks * BLOCKSIZE / sizeof(uint32_t)) ) tempbuff += 8U;�?

This seems to work, but I'm not sure if this is best solution without any side effects. At first glance it seems to work but I haven't done detailed tests yet.

Can anyone confirm this problem? Is my assumption right that this is a bug in the HAL driver?

[Tesla DeLorean]

Posted on June 22, 2018 at 05:02

>>So far, I've only used f_write() which used SDMMC_WriteFIFO() through the default implementation in bsp_driver_sd.c.

I wouldn't postpone that too long, the polled code is pretty fragile. On the F746 and F769 DISCO boards the example code when driving the display and writing to the card readily fails.

There are several code examples under the trees demonstrating DMA

STM32Cube_FW_F7_V1.11.0\Projects\STM32F769I_EVAL\Applications\USB_Host\DynamicSwitch_Standalone\Src\sd_diskio_dma.c

>>Luckily, it was possible to see the bug in GDB with a simple backtrace, that showed that SDMMC_WriteFIFO() was called with an invalid address.

The loop there does seem dangerously unbounded, the expectation is that the FIFO is constrained by the single sector read, or config.DataLength

Understanding and replicating that condition is certainly something you want to do and get in front of people at ST.

[A L]

Posted on June 22, 2018 at 15:59

Thanks again for the help.

>>Luckily, it was possible to see the bug in GDB with a simple backtrace, that showed that SDMMC_WriteFIFO() was called with an invalid address.

The loop there does seem dangerously unbounded, the expectation is that the FIFO is constrained by the single sector read, or config.DataLength

I try to understand what's going on... but I'm a bit lost, since I don't really know how FatFs, FAT file systems, and the SDMMC peripheral really handles stuff internally. So far, things have been a black box to me. Simple file access in C...

I do trust FatFs, since it has been battle tested by many people, but what about the HAL?

Some notes:

Fixed blocksize: stm32f7xx_hal_mmc.h:243

 &sharpdefine BLOCKSIZE   ((uint32_t)512U)         /*!< Block size is 512 bytes */

stm32f7xx_hal_sd.c:746 in HAL_SD_WriteBlocks()

    /* Configure the SD DPSM (Data Path State Machine) */

    config.DataTimeOut   = SDMMC_DATATIMEOUT;    config.DataLength    = NumberOfBlocks * BLOCKSIZE;    config.DataBlockSize = SDMMC_DATABLOCK_SIZE_512B;    config.TransferDir   = SDMMC_TRANSFER_DIR_TO_CARD;    config.TransferMode  = SDMMC_TRANSFER_MODE_BLOCK;    config.DPSM          = SDMMC_DPSM_ENABLE;    SDMMC_ConfigData(hsd->Instance, &config);

Although, I've used large 1 MB chunks, which were nicely aligned to 4- / 8- / 512-byte grid, I can't say yet why HAL_SD_WriteBlocks() wants to read more bytes than needed (blocksize -> DataLength?). As far as I remember, it didn't help to make chunks smaller, only setting the chunk size to 1 byte helped.

EDIT: The last chunk was normally smaller than 1 MB, but it was still 32-bit aligned... still the error always occurred at the end of the test run, when I've approached the end of valid SDRAM address range [0xc0000000-0xc1000000) in my case... HAL_SD_WriteBlocks() wanted to read from address >= 0xc1000000.

Here an example backtrace (-O0, gcc with gdb) from my test program:

&sharp0  HAL_SD_WriteBlocks (hsd=0x200044b4 <hsd1>,

    pData=0xc0300000 ..., BlockAdd=2029318, NumberOfBlocks=8, Timeout=4294967295)    at Drivers/STM32F7xx_HAL_Driver/Src/stm32f7xx_hal_sd.c:680&sharp1  0x08014ef2 in BSP_SD_WriteBlocks (pData=0xc0300000, WriteAddr=2029318, NumOfBlocks=8, Timeout=4294967295)    at Src/bsp_driver_sd.c:147&sharp2  0x08014d78 in SD_write (lun=0 '\000',    buff=0xc0300000 ..., sector=2029318, count=8)    at Src/sd_diskio.c:202&sharp3  0x08014150 in disk_write (pdrv=0 '\000',    buff=0xc0300000 ..., sector=2029318, count=8)    at Middlewares/Third_Party/FatFs/src/diskio.c:144&sharp4  0x08013824 in f_write (fp=0x2004fcf8, buff=0xc0300000, btw=1048576, bw=0x2004fcf4)    at Middlewares/Third_Party/FatFs/src/ff.c:3666&sharp5  0x0801675c in _save_memory_to_sd_card (filename=0x8020698 'MEM.SYS',    data_start=0xc0000000 'BM6\372\005', data_end=0xc0f4ba60 ..., crc_out=0x2004ff60)    at ../bq/bq.c:252

Understanding and replicating that condition is certainly something you want to do and get in front of people at ST.

Yes, totally, but I'm not really qualified to say what's going wrong yet...

Help to track down this bug would be appreciated.

I wouldn't postpone that too long, the polled code is pretty fragile. On the F746 and F769 DISCO boards the example code when driving the display and writing to the card readily fails.

There are several code examples under the trees demonstrating DMA

STM32Cube_FW_F7_V1.11.0\Projects\STM32F769I_EVAL\Applications\USB_Host\DynamicSwitch_Standalone\Src\sd_diskio_dma.c

Thanks a lot for the hint, I wasn't aware of that. In STM32CubeMX there's also an option to generate code using the DMA template (FATFS Configuration -> IP Instances -> Use DMA Template). Have not tried that one yet. - I think DMA could really help with the SD card issues I had so far... (speed issues, probably interrupts causing issues with the polling access...).

[Tesla DeLorean]

Posted on June 22, 2018 at 17:40

FatFs manages the block IO through the diskio.c layers, basically reading/writing whole blocks (1 or multiple). If it needs to work on portions, it will read in a whole block, modify a portion, and write back a new whole block.

At the read/write level the IO code could recognized buffer alignment issues, or memory vs dma issues, and decompose into single blocks via a scratch buffer.

I suspect FatFs might pass large unaligned buffers if it is breaking down a larger transfer into block aligned pieces. If you read/write cluster aligned buffers at the FatFs level this should not be an issue.