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?

[Nemui Trinomius]

Posted on May 31, 2017 at 04:20

Enable hardware flow control also.

[Nils Zottmann]

Posted on May 31, 2017 at 08:48

Enabling hardware flow control didn't change anything, the problem still exists.

[Roberto Peruzzini]

Posted on June 08, 2017 at 21:32

I have the same problem and i solved it with your solution. Thanks

[confalonieri]

Posted on June 09, 2017 at 11:28

I have the same problem and the proposed solution doesn't work.

Premising that with library 1.6.0 my code works perfectly, after adapting it to the new library 1.8.0 I start getting problems writing to SD card.

Read is ok, but Write is not working at all.

the HAL_SD_Writeblocks return HAL_ERROR, hsd state 1, hsd errorcode 4 ?

[Tesla DeLorean]

Posted on June 22, 2018 at 03:10

>>Read is ok, but Write is not working at all.  the HAL_SD_Writeblocks return HAL_ERROR, hsd state 1, hsd errorcode 4 ?

This is indicative of an underrun error because the data output rate isn't sustaining the write. Consider using DMA

[Simon Pearson]

Posted on April 02, 2018 at 21:57

04/02/2018 - Processor: STM32L476RGTx,  STM32CubeMX: 4.24.0

Same problem, fix worked.  Thanks!!!!!

[A L]

Posted on June 22, 2018 at 02:46

Zottmann.Nils

‌

Thanks for asking this question, and providing a possible solution!

I also had the same problem.

MCU:

https://www.st.com/en/microcontrollers-microprocessors/stm32f746zg.html

STM32CubeMX: 4.0

In order to make the temporary fix work in my case, I had to modify stm32f7xx_hal_sd.c slightly differently.

But this is certainly not a proper solution...

/* Write data to SDMMC Tx FIFO */

for(count = 0U; count < 8U; count++)

{

if( ((tempbuff + count) - (uint32_t *)pData) >=

(NumberOfBlocks * BLOCKSIZE / sizeof(uint32_t)) )

break;

SDMMC_WriteFIFO(hsd->Instance, (tempbuff + count));

}

In my test program, I'm writing a test pattern into external SDRAM (16 MB), write its entire contents onto the SD card (in 1 MB chunks). After that the external SDRAM gets cleared, and the file from the SD card gets loaded back into it. Then I compare the checksums, from before and after storing the memory. - I've encountered 2 problems:

•  
•  
•  
• Hard faults because of reading from an invalid RAM address in SDMMC_WriteFIFO() (stm32f7xx_ll_sdmmc.c).
• (And also occasional CRC checksum mismatches (probably because of clock speed issues, the basic read/write test works now (I'm trying to stop the core and make a 'hibernate' stop feature))).

(I've spent quite a lot of time tracking the bug down, since I cannot really debug my MCU (STM32F746,

https://community.st.com/s/question/0D50X00009XkgAOSAZ/problem-with-single-step-stmf7

)).

Since I've only used HAL functions and FatFS to interact with the MMC/SD card so far, I do not really understand what's going on yet.

Even though the fix prevents the HAL (HAL_SD_WriteBlocks()) from reading from invalid memory locations, it still requires the input buffer to be (at least) 4-byte aligned I think. SDMMC_WriteFIFO() writes 32-bit words into the FIFO, which can also lead to problems, as far as I understand it...

[Tesla DeLorean]

Posted on June 22, 2018 at 03:36

Avoid using the polling and SDMMC_WriteFIFO methods, this is prone to underruns, and sensitive to interrupt loading.

With the F7 and DMA you are going to need to concern yourself about the write buffering and the caching.

Consider using the DTCM RAM for the buffering, it avoids issues with the caching.

Yes, you want the buffers to be 32-bit aligned, what are you doing that would preclude that happening? Writing small and odd size buffers with f_write() ? If you manage how you do the writing you can avoid this, and also write a lot faster.

Single stepping and viewing the SDMMC peripheral registers in the debugger is problematic. Mainly because it is time critical, and reading the FIFO is invasive. It is more effective to instrument and observe.

[A L]

Posted on June 22, 2018 at 04:26

Thanks for your help, Clive!

Avoid using the polling and SDMMC_WriteFIFO methods, this is prone to underruns, and sensitive to interrupt loading.

So far, I've only used f_write() which used SDMMC_WriteFIFO() through the default implementation of the FatFs driver generated by STM32CubeMx in bsp_driver_sd.c.

With the F7 and DMA you are going to need to concern yourself about the write buffering and the caching.

Yes, I should find out how to make the FatFs driver use DMA.

(That's a thing for later though... Currently I'm having problems reading large files back from the SD card to external SDRAM... ('hibernate' the system, and putting it to sleep in STOP mode). - Reading from MMC to the external RAM directly (no temporary buffer in internal SRAM, and no DMA) seems to work, I've disabled most peripherals, but I'm still getting those occasional CRC mismatches (data corruptions). - Working on it while editing this post, and after >1 day working on it, it seems to work now! Interrupts really need to be disabled altogether, to make this work... But this is off-topic!).

Consider using the DTCM RAM for the buffering, it avoids issues with the caching.

Thanks for the hint. Yes, more in-depth DTCM RAM testing is on the list. - I'm still doing very basic tests with my F7 project, to find out all hardware issues for the next PCB revision and to learn more about all the STM32 peripherals...

Yes, you want the buffers to be 32-bit aligned, what are you doing that would preclude that happening? Writing small and odd size buffers with f_write() ? If you manage how you do the writing you can avoid this, and also write a lot faster.

I was wondering what would happen if the user would use such oddly sized buffers. Byte-wise file access with FatFS is obviously possible, but the SDMMC seems to transfer the data differently on a hardware level... but I've not investigated this at all yet.

Single stepping and viewing the SDMMC peripheral registers in the debugger is problematic. Mainly because it is time critical, and reading the FIFO is invasive. It is more effective to instrument and observe.

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.