esp_vfs_fat_spiflash_format_rw_wl formats wrong partition (IDFGH-11404)

[tonystuart]

Answers checklist.

• I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
• I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
• I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v5.3-dev-222-gb90dfe0759

Espressif SoC revision.

ESP32-S3

Operating System used.

Linux

How did you build your project?

Command line with idf.py

If you are using Windows, please specify command line type.

None

Development Kit.

Custom Board

Power Supply used.

USB

What is the expected behavior?

I expected the ESP-IDF function esp_vfs_fat_spiflash_format_rw_wl to format the partition identified by the supplied partition_label.

What is the actual behavior?

Instead, the ESP-IDF function esp_vfs_fat_spiflash_format_rw_wl formats an SPI flash filesystem that has previously been mounted from a Micro SD card.

Steps to reproduce.

1. Use esp_vfs_fat_sdspi_mount to mount an existing FAT filesystem located on an SPI flash Micro SD card (e.g. /sdcard)
2. Use esp_vfs_fat_spiflash_format_rw_wl to format a new FAT filesystem on an internal SPI flash memory partition (e.g. /local)
3. Note that esp_vfs_fat_spiflash_format_rw_wl does not return in a timely manner
4. Power off device and mount Micro SD card in another system (e.g. Linux)
5. Note that Micro SD has been reformatted. It has a new volume label and contains an empty partition.

Debug Logs.

I (8718) EXPRESS: initialize_fatfs erase and format local on factory reset
I (8719) WS_FATFS: ws_fatfs_erase label=local, address=0x7a0000, size=0x2a0000, encrypted=0
I (19055) WS_FATFS: ws_fatfs_format label=local, mount_point=/local
D (19423) vfs_fat_spiflash: using pdrv=1
W (19425) vfs_fat_spiflash: f_mount failed (13)
I (19425) vfs_fat_spiflash: Formatting FATFS partition, allocation unit size=4096
I (19868) vfs_fat_spiflash: Mounting again
I (19868) vfs_fat_spiflash: Formatting FATFS partition, allocation unit size=4096
D (19872) sdmmc_sd: sdmmc_sd_get_erase_timeout_ms: erase timeout 166 s (erasing 2684928 kB, ES=8, ET=2, EO=2, AU=4096 kB)
E (24867) task_wdt: Task watchdog got triggered. The following tasks/users did not reset the watchdog in time:
E (24867) task_wdt:  - IDLE1 (CPU 1)
E (24868) task_wdt: Tasks currently running:
E (24868) task_wdt: CPU 0: IDLE0
E (24868) task_wdt: CPU 1: main
E (24868) task_wdt: Print CPU 1 (current core) backtrace

More Information.

It looks like the problem is in the implementation of the ESP-IDF esp_vfs_fat_spiflash_format_rw_wl function in the esp-idf/components/fatfs/vfs/vfs_fat_spiflash.c module.

This module generally uses the following pattern to initialize a null terminated three byte drive designator string:

char drv[3] = {(char)('0' + pdrv), ':', 0};

This converts pdrv to a digit starting at '0' and assigns it to the first character in the string.

In the case of the failing module, instead of initializing drv using the above single statement approach, it uses a two statement approach. The first statement consists of:

char drv[3] = {0, ':', 0};

Later, the second statement consists of:

drv[1] = (char)('0' + s_ctx[id]->pdrv);

The first statement assigns a null terminator to the first character of drv (i.e. the byte at index 0), creating an empty string.

The second statement assigns a drive digit to the second character of the string (i.e. the byte at index 1). However, because the first byte is the null terminator, this has no effect and it's still an empty string.

Following the second statement, esp_vfs_fat_spiflash_format_rw_wl calls:

f_mount(0, drv, 0);

This function (f_mount) calls get_ldnumber to convert the value in drv back to a number. Function get_ldnumber validates for a NULL drive designator, but it doesn't validate for an empty drive designator, and it returns 0 as the drive instead of an error.

At this point, esp_vfs_fat_spiflash_format_rw_wl calls f_mkfs to create a file system, passing a drv value of 0 (the already mounted existing SD Card filesystem) instead of 1 (the local flash filesystem we want to flash).

This wipes out the existing SD filesystem.

The simplest fix is to change the errant line from:

drv[1] = (char)('0' + s_ctx[id]->pdrv);

to:

drv[0] = (char)('0' + s_ctx[id]->pdrv);

I tried this and it fixes the problem.

Another alternative would be to combine the drv declaration and initialization statements by removing the declaration and replacing the initialization with a single statement that both declares and initializes drv (as is found elsewhere in the module):

char drv[3] = {(char)('0' + s_ctx[id]->pdrv), ':', 0};

I'm happy to provide a pull request with this change.

However, it's worth noting there are other code quality issues in this module (e.g. get_ldnumber checks for NULL string, but not empty string; esp_vfs_fat_spiflash_format_rw_wl doesn't check f_mount return code). These should probably be added to a tech debt issue, if one exists.

[adokitkat]

Hello. Thank you for the detailed information! No need to create a pull request, we will add you as a co-author to the fix commit.