Merge branch 'bugfix/fatfs_memory_leak_with_dyn_buffer' into 'master'

fix(fatfs): fix a memory leak bug when FF_USE_DYN_BUFFER was enabled

Closes IDF-15593

See merge request espressif/esp-idf!47769

components/fatfs/host_test/main/test_fatfs.cpp

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2023-2026 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -18,7 +18,13 @@ TEST_CASE("Create volume, open file, write and read back data", "[fatfs]")
 {
     BYTE pdrv;
     FATFS fs;
+#if FF_USE_DYN_BUFFER
+    fs.win = NULL;
+#endif
     FIL file;
+#if !FF_FS_TINY && FF_USE_DYN_BUFFER
+    file.buf = NULL;
+#endif
     UINT bw;
 
     const esp_partition_t *partition = esp_partition_find_first(ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_FAT, "storage");
@@ -129,12 +135,18 @@ TEST_CASE("Test mounting 2 volumes, writing data and formatting the 2nd one, rea
     const esp_partition_t *partition0 = NULL;
     BYTE pdrv0 = UINT8_MAX;
     FATFS fs0;
+#if FF_USE_DYN_BUFFER
+    fs0.win = NULL;
+#endif
     wl_handle_t wl_handle0 = WL_INVALID_HANDLE;
 
     const char* partition_label1 = "storage2";
     const esp_partition_t *partition1 = NULL;
     BYTE pdrv1 = UINT8_MAX;
     FATFS fs1;
+#if FF_USE_DYN_BUFFER
+    fs1.win = NULL;
+#endif
     wl_handle_t wl_handle1 = WL_INVALID_HANDLE;
 
     size_t data_size = 10;
@@ -150,6 +162,9 @@ TEST_CASE("Test mounting 2 volumes, writing data and formatting the 2nd one, rea
 
     // Open file and write data
     FIL file0;
+#if !FF_FS_TINY && FF_USE_DYN_BUFFER
+    file0.buf = NULL;
+#endif
     UINT bw0;
     REQUIRE(f_open(&file0, "0:/test0.txt", FA_OPEN_ALWAYS | FA_WRITE) == FR_OK);
     // Write data
@@ -174,6 +189,9 @@ TEST_CASE("Test mounting 2 volumes, writing data and formatting the 2nd one, rea
 
     // Open file and write data
     FIL file1;
+#if !FF_FS_TINY && FF_USE_DYN_BUFFER
+    file1.buf = NULL;
+#endif
     UINT bw1;
     REQUIRE(f_open(&file1, "1:/test1.txt", FA_OPEN_ALWAYS | FA_WRITE) == FR_OK);
     // Write data

components/fatfs/src/ff.c

@@ -3513,8 +3513,10 @@ static FRESULT mount_volume (	/* FR_OK(0): successful, !=0: an error occurred */
 	if (SS(fs) > FF_MAX_SS || SS(fs) < FF_MIN_SS || (SS(fs) & (SS(fs) - 1))) return FR_DISK_ERR;
 #endif
 #if FF_USE_DYN_BUFFER
-    fs->win = ff_memalloc(SS(fs));		/* Allocate memory for sector buffer */
+    if (!fs->win) {
-    if (!fs->win) return FR_NOT_ENOUGH_CORE;
+        fs->win = ff_memalloc(SS(fs));		/* Allocate memory for sector buffer */
+        if (!fs->win) return FR_NOT_ENOUGH_CORE;
+    }
 #endif
 
 	/* Find an FAT volume on the hosting drive */
@@ -3768,8 +3770,10 @@ FRESULT f_mount (
 		ff_mutex_delete(vol);
 #endif
 #if FF_USE_DYN_BUFFER
-        if (cfs->fs_type)           /* Check if the buffer was ever allocated */
+        if (cfs->win) {           /* Check if the buffer was ever allocated */
             ff_memfree(cfs->win);   /* Deallocate buffer allocated for the filesystem object */
+            cfs->win = NULL;
+        }
 #endif
 		cfs->fs_type = 0;		/* Invalidate the filesystem object to be unregistered */
 	}
@@ -3955,18 +3959,15 @@ FRESULT f_open (
 #if !FF_FS_READONLY
 #if !FF_FS_TINY
 #if FF_USE_DYN_BUFFER
-            fp->buf = NULL;
+            if (!fp->buf) {
-            if (res == FR_OK) {
                 fp->buf = ff_memalloc(SS(fs));
                 if (!fp->buf) {
                     res = FR_NOT_ENOUGH_CORE;	/* Not enough memory */
                     goto fail;
                 }
-    			memset(fp->buf, 0, SS(fs));	/* Clear sector buffer */
             }
-#else
-            memset(fp->buf, 0, SS(fs));    /* Clear sector buffer */
 #endif
+            memset(fp->buf, 0, SS(fs));    /* Clear sector buffer */
 #endif
 			if ((mode & FA_SEEKEND) && fp->obj.objsize > 0) {	/* Seek to end of file if FA_OPEN_APPEND is specified */
 				DWORD bcs, clst;
@@ -4350,15 +4351,17 @@ FRESULT f_close (
 #else
 			fp->obj.fs = 0;	/* Invalidate file object */
 #endif
-#if !FF_FS_TINY && FF_USE_DYN_BUFFER
-            ff_memfree(fp->buf);
-            fp->buf = NULL;
-#endif
 #if FF_FS_REENTRANT
 			unlock_volume(fs, FR_OK);		/* Unlock volume */
 #endif
 		}
 	}
+#if !FF_FS_TINY && FF_USE_DYN_BUFFER
+    if (fp->buf) {
+        ff_memfree(fp->buf);
+        fp->buf = NULL;
+    }
+#endif
 	return res;
 }
 

components/fatfs/test_apps/bdl/main/test_fatfs_bdl.c

@@ -80,9 +80,15 @@ TEST_CASE("(BDL) diskio register, format, write and read on partition", "[fatfs]
     TEST_ASSERT_EQUAL(FR_OK, f_mkfs(drv, &opt, work_area, sizeof(work_area)));
 
     FATFS fs;
+#if FF_USE_DYN_BUFFER
+    fs.win = NULL;
+#endif
     TEST_ASSERT_EQUAL(FR_OK, f_mount(&fs, drv, 1));
 
     FIL file;
+#if !FF_FS_TINY && FF_USE_DYN_BUFFER
+    file.buf = NULL;
+#endif
     UINT bw;
     TEST_ASSERT_EQUAL(FR_OK, f_open(&file, "test.txt", FA_OPEN_ALWAYS | FA_READ | FA_WRITE));
 

components/fatfs/test_apps/dyn_buffers/main/CMakeLists.txt

@@ -1,3 +1,3 @@
 idf_component_register(SRCS "test_fatfs_dyn_buffers.c"
                     INCLUDE_DIRS "."
-                    REQUIRES wear_levelling fatfs vfs)
+                    REQUIRES unity wear_levelling fatfs vfs)

components/fatfs/test_apps/dyn_buffers/main/test_fatfs_dyn_buffers.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Unlicense OR CC0-1.0
  */
@@ -7,7 +7,10 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <stdint.h>
+#include "unity.h"
+#include "unity_test_utils.h"
 #include "wear_levelling.h"
+#include "diskio_wl.h"
 #include "esp_partition.h"
 #include "esp_vfs.h"
 #include "esp_vfs_fat.h"
@@ -21,10 +24,19 @@ static const char* TAG = "Test dynamic buffers";
 static volatile bool g_alloc_count_enable = false;
 static volatile int g_buffer_alloc_count = 0;
 
-static esp_vfs_fat_mount_config_t g_mount_config = {
+// Some resources are lazy allocated, the threshold is left for that case
-    .format_if_mount_failed = true,
+#define TEST_MEMORY_LEAK_THRESHOLD (1280)
-    .max_files = 5,
+
-};
+void setUp(void)
+{
+    unity_utils_record_free_mem();
+}
+
+void tearDown(void)
+{
+    esp_partition_unload_all();     //clean up some of the esp_partition's allocations
+    unity_utils_evaluate_leaks_direct(TEST_MEMORY_LEAK_THRESHOLD);
+}
 
 void esp_heap_trace_alloc_hook(void* ptr, size_t size, uint32_t caps)
 {
@@ -42,32 +54,28 @@ void esp_heap_trace_alloc_hook(void* ptr, size_t size, uint32_t caps)
     }
 }
 
-void app_main(void)
+TEST_CASE("(dyn_buffers) basic mounting and file operations", "[fatfs][dyn_buffers]")
 {
-    esp_err_t err = ESP_OK;
+    esp_vfs_fat_mount_config_t mount_config = {
+        .format_if_mount_failed = true,
+        .max_files = 5,
+    };
 
     wl_handle_t wl_handle;
-
+    esp_vfs_fat_spiflash_format_cfg_rw_wl("/spiflash", NULL, &mount_config);
-    err = esp_vfs_fat_spiflash_format_cfg_rw_wl("/spiflash", NULL, &g_mount_config);
 
     ESP_LOGI(TAG, "Mounting FATFS");
 
-    g_mount_config.format_if_mount_failed = false,
+    mount_config.format_if_mount_failed = false,
 
     g_alloc_count_enable = true;
 
-    err = esp_vfs_fat_spiflash_mount_rw_wl("/spiflash", NULL, &g_mount_config, &wl_handle);
+    TEST_ESP_OK(esp_vfs_fat_spiflash_mount_rw_wl("/spiflash", NULL, &mount_config, &wl_handle));
-    if (err != ESP_OK) {
-        ESP_LOGE(TAG, "FATFS mount failed with error: %d", err);
-        return;
-    }
 
     ESP_LOGI(TAG, "Mounted");
 
     int fd = open("/spiflash/test.txt", O_RDWR|O_CREAT);
-    if (fd < 0) {
+    TEST_ASSERT(fd >= 0);
-        ESP_LOGE(TAG, "Failed opening file");
-    }
 
     close(fd);
 
@@ -83,19 +91,67 @@ void app_main(void)
                 , g_buffer_alloc_count);
 
 #if CONFIG_FATFS_USE_DYN_BUFFERS
-
+    TEST_ASSERT_MESSAGE(g_buffer_alloc_count == 2, "FATFS buffer should have been allocated once for each context (file and fatfs)");
-    if (g_buffer_alloc_count != 2) {
-        ESP_LOGE(TAG, "FATFS buffer should have been allocated once for each context (file and fatfs)");
-        return;
-    }
 #else
-
+    TEST_ASSERT_MESSAGE(g_buffer_alloc_count == 0, "FATFS buffer should not have been allocated");
-    if (g_buffer_alloc_count != 0) {
-        ESP_LOGE(TAG, "FATFS buffer should not have been allocated");
-        return;
-    }
-
 #endif
 
     ESP_LOGI(TAG, "Done");
 }
+
+TEST_CASE("(dyn_buffers) File system data does not exist, format and remount", "[fatfs][dyn_buffers]")
+{
+    const char test_par_lab[] = "storage2";
+
+    const esp_partition_t * part  = esp_partition_find_first(ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_FAT, test_par_lab);
+    TEST_ASSERT(part != NULL);
+    esp_partition_erase_range(part, 0, part->size);
+
+
+    esp_vfs_fat_mount_config_t mount_config = {
+        .format_if_mount_failed = true,
+        .max_files = 5,
+    };
+    wl_handle_t wl_handle;
+    TEST_ESP_OK(esp_vfs_fat_spiflash_mount_rw_wl("/spiflash2", test_par_lab, &mount_config, &wl_handle));
+
+    esp_vfs_fat_spiflash_unmount_rw_wl("/spiflash2", wl_handle);
+}
+
+TEST_CASE("(dyn_buffers) Unmount and then close the file", "[fatfs][dyn_buffers]")
+{
+    const char test_par_lab[] = "storage2";
+
+    esp_vfs_fat_mount_config_t mount_config = {
+        .format_if_mount_failed = true,
+        .max_files = 5,
+    };
+    wl_handle_t wl_handle;
+    TEST_ESP_OK(esp_vfs_fat_spiflash_mount_rw_wl("/spiflash2", test_par_lab, &mount_config, &wl_handle));
+    BYTE pdrv = ff_diskio_get_pdrv_wl(wl_handle);
+    char drv[3] = {(char)('0' + pdrv), ':', 0};
+    const esp_vfs_fat_conf_t conf = {
+        .base_path = "/spiflash2",
+    };
+    FATFS *fs = NULL;
+    esp_vfs_fat_register(&conf, &fs);
+    TEST_ASSERT_NOT_NULL(fs);
+
+    int fd = open("/spiflash2/test.txt", O_RDWR|O_CREAT);
+    TEST_ASSERT(fd >= 0);
+    // Unmount fatfs only
+    f_mount(0, drv, 0);
+    close(fd);
+
+    f_mount(fs, drv, 1);
+    fd = open("/spiflash2/test.txt", O_RDWR|O_CREAT);
+    TEST_ASSERT(fd >= 0);
+    // Unmount fatfs and vfs
+    esp_vfs_fat_spiflash_unmount_rw_wl("/spiflash2", wl_handle);
+    close(fd);
+}
+
+void app_main(void)
+{
+    unity_run_menu();
+}

components/fatfs/test_apps/dyn_buffers/pytest_fatfs_dyn_buffers.py

@@ -1,4 +1,4 @@
-# SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD
+# SPDX-FileCopyrightText: 2022-2026 Espressif Systems (Shanghai) CO LTD
 # SPDX-License-Identifier: CC0-1.0
 import pytest
 from pytest_embedded import Dut
@@ -15,8 +15,4 @@ from pytest_embedded_idf.utils import idf_parametrize
 )
 @idf_parametrize('target', ['esp32'], indirect=['target'])
 def test_fatfs_flash_dyn_buffers(config: str, dut: Dut) -> None:
-    dut.expect('Mounting FATFS')
+    dut.run_all_single_board_cases(timeout=30)
-    dut.expect('Mounted')
-    dut.expect('Unmounting FATFS')
-    dut.expect('Unmounted')
-    dut.expect('Done')

components/fatfs/vfs/vfs_fat.c

@@ -245,6 +245,14 @@ esp_err_t esp_vfs_fat_unregister_path(const char* base_path)
         return err;
     }
     _lock_close(&fat_ctx->lock);
+
+#if !FF_FS_TINY && FF_USE_DYN_BUFFER
+    for (size_t i = 0; i < fat_ctx->max_files; ++i) {
+        if (fat_ctx->files[i].buf != NULL) {
+            ff_memfree(fat_ctx->files[i].buf);
+        }
+    }
+#endif
     free(fat_ctx->flags);
     free(fat_ctx);
     s_fat_ctxs[ctx] = NULL;