From 0242c79881d425b1e8641573f2c24e33ed3b29e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20M=C3=BAdry?= Date: Thu, 11 Jun 2026 12:50:50 +0200 Subject: [PATCH 1/2] fix(fatfs): Fix esp_vfs_fat_unregister_path leak if esp_vfs_unregister fails --- components/fatfs/vfs/vfs_fat.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/fatfs/vfs/vfs_fat.c b/components/fatfs/vfs/vfs_fat.c index 9dcd532aa2b..08b8fc36ab9 100644 --- a/components/fatfs/vfs/vfs_fat.c +++ b/components/fatfs/vfs/vfs_fat.c @@ -242,7 +242,7 @@ esp_err_t esp_vfs_fat_unregister_path(const char* base_path) vfs_fat_ctx_t* fat_ctx = s_fat_ctxs[ctx]; esp_err_t err = esp_vfs_unregister(fat_ctx->base_path); if (err != ESP_OK) { - return err; + ESP_LOGW(TAG, "esp_vfs_unregister failed (0x%x), cleaning up anyway", err); // should not happen, but if it does, we don't want to leak memory and keep the VFS in a broken state } _lock_close(&fat_ctx->lock); @@ -256,7 +256,7 @@ esp_err_t esp_vfs_fat_unregister_path(const char* base_path) free(fat_ctx->flags); free(fat_ctx); s_fat_ctxs[ctx] = NULL; - return ESP_OK; + return err; } esp_err_t esp_vfs_fat_info(const char* base_path, From 067c6f89893854251b2f90d7e63c4f17f69eed06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20M=C3=BAdry?= Date: Thu, 11 Jun 2026 15:34:27 +0200 Subject: [PATCH 2/2] fix(vfs): Fix incorrect check in esp_vfs_register_fs_common and optimize memory --- components/vfs/vfs.c | 53 ++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/components/vfs/vfs.c b/components/vfs/vfs.c index 1a0518f3cfb..ee063b231b7 100644 --- a/components/vfs/vfs.c +++ b/components/vfs/vfs.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -72,7 +72,7 @@ typedef struct { } fds_triple_t; static vfs_entry_t* s_vfs[VFS_MAX_COUNT] = { 0 }; -static size_t s_vfs_count = 0; +static size_t s_vfs_upper_bound = 0; // upper bound of indices in s_vfs which can be occupied by VFS entries; always equal to the index of the last non-NULL entry + 1 static fd_table_t s_fd_table[MAX_FDS] = { [0 ... MAX_FDS-1] = FD_TABLE_ENTRY_UNUSED }; static _lock_t s_fd_table_lock; @@ -410,8 +410,9 @@ static esp_err_t esp_vfs_register_fs_common( void *ctx, int *vfs_index) { - if (s_vfs_count >= VFS_MAX_COUNT) { - return ESP_ERR_NO_MEM; + ssize_t index = esp_get_free_index(); + if (index < 0) { // Check for free slot before doing any other work + return ESP_ERR_NO_MEM; } if (vfs == NULL) { @@ -432,17 +433,12 @@ static esp_err_t esp_vfs_register_fs_common( } } - ssize_t index = esp_get_free_index(); - if (index < 0) { - return ESP_ERR_NO_MEM; - } - if (s_vfs[index] != NULL) { return ESP_ERR_INVALID_STATE; } - if (index == s_vfs_count) { - s_vfs_count++; + if (index == s_vfs_upper_bound) { + s_vfs_upper_bound++; } vfs_entry_t *entry = heap_caps_malloc(sizeof(vfs_entry_t) + base_path_len + 1, VFS_MALLOC_FLAGS); @@ -620,6 +616,10 @@ esp_err_t esp_vfs_unregister_with_id(esp_vfs_id_t vfs_id) } _lock_release(&s_fd_table_lock); + while (s_vfs_upper_bound > 0 && s_vfs[s_vfs_upper_bound - 1] == NULL) { // Move the upper bound down if we just removed the last entry + s_vfs_upper_bound--; + } + return ESP_OK; } @@ -629,7 +629,7 @@ esp_err_t esp_vfs_unregister_fs_with_id(esp_vfs_id_t vfs_id) __attribute__((alia esp_err_t esp_vfs_unregister(const char* base_path) { const size_t base_path_len = strlen(base_path); - for (size_t i = 0; i < s_vfs_count; ++i) { + for (size_t i = 0; i < s_vfs_upper_bound; ++i) { vfs_entry_t* vfs = s_vfs[i]; if (vfs == NULL) { continue; @@ -651,7 +651,7 @@ esp_err_t esp_vfs_register_fd(esp_vfs_id_t vfs_id, int *fd) esp_err_t esp_vfs_register_fd_with_local_fd(esp_vfs_id_t vfs_id, int local_fd, bool permanent, int *fd) { - if (vfs_id < 0 || vfs_id >= s_vfs_count || fd == NULL) { + if (vfs_id < 0 || vfs_id >= s_vfs_upper_bound || fd == NULL) { ESP_LOGD(TAG, "Invalid arguments for esp_vfs_register_fd_with_local_fd(%d, %d, %d, 0x%p)", vfs_id, local_fd, permanent, fd); return ESP_ERR_INVALID_ARG; @@ -685,7 +685,7 @@ esp_err_t esp_vfs_unregister_fd(esp_vfs_id_t vfs_id, int fd) { esp_err_t ret = ESP_ERR_INVALID_ARG; - if (vfs_id < 0 || vfs_id >= s_vfs_count || fd < 0 || fd >= MAX_FDS) { + if (vfs_id < 0 || vfs_id >= s_vfs_upper_bound || fd < 0 || fd >= MAX_FDS) { ESP_LOGD(TAG, "Invalid arguments for esp_vfs_unregister_fd(%d, %d)", vfs_id, fd); return ret; } @@ -748,7 +748,7 @@ void esp_vfs_dump_registered_paths(FILE *fp) esp_err_t esp_vfs_set_readonly_flag(const char* base_path) { const size_t base_path_len = strlen(base_path); - for (size_t i = 0; i < s_vfs_count; ++i) { + for (size_t i = 0; i < s_vfs_upper_bound; ++i) { vfs_entry_t* vfs = s_vfs[i]; if (vfs == NULL) { continue; @@ -764,11 +764,10 @@ esp_err_t esp_vfs_set_readonly_flag(const char* base_path) const vfs_entry_t *get_vfs_for_index(int index) { - if (index < 0 || index >= s_vfs_count) { + if (index < 0 || index >= VFS_MAX_COUNT) { return NULL; - } else { - return s_vfs[index]; } + return s_vfs[index]; } static inline bool fd_valid(int fd) @@ -812,7 +811,7 @@ const vfs_entry_t* get_vfs_for_path(const char* path) const vfs_entry_t* best_match = NULL; ssize_t best_match_prefix_len = -1; size_t len = strlen(path); - for (size_t i = 0; i < s_vfs_count; ++i) { + for (size_t i = 0; i < s_vfs_upper_bound; ++i) { const vfs_entry_t* vfs = s_vfs[i]; if (vfs == NULL || vfs->path_prefix_len == LEN_PATH_PREFIX_IGNORED) { continue; @@ -836,7 +835,7 @@ const vfs_entry_t* get_vfs_for_path(const char* path) // Out of all matching path prefixes, select the longest one; // i.e. if "/dev" and "/dev/uart" both match, for "/dev/uart/1" path, // choose "/dev/uart", - // This causes all s_vfs_count VFS entries to be scanned when opening + // This causes all s_vfs_upper_bound VFS entries to be scanned when opening // a file by name. This can be optimized by introducing a table for // FS search order, sorted so that longer prefixes are checked first. if (best_match_prefix_len < (ssize_t) vfs->path_prefix_len) { @@ -1482,10 +1481,10 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds return -1; } - // Capture s_vfs_count to a local variable in case a new driver is registered or removed during this actual select() - // call. s_vfs_count cannot be protected with a mutex during a select() call (which can be one without a timeout) + // Capture s_vfs_upper_bound to a local variable in case a new driver is registered or removed during this actual select() + // call. s_vfs_upper_bound cannot be protected with a mutex during a select() call (which can be one without a timeout) // because that could block the registration of new driver. - const size_t vfs_count = s_vfs_count; + const size_t vfs_count = s_vfs_upper_bound; fds_triple_t *vfs_fds_triple; if ((vfs_fds_triple = heap_caps_calloc(vfs_count, sizeof(fds_triple_t), VFS_MALLOC_FLAGS)) == NULL) { __errno_r(r) = ENOMEM; @@ -1691,8 +1690,8 @@ void esp_vfs_select_triggered(esp_vfs_select_sem_t sem) // Another way would be to go through s_fd_table and find the VFS // which has a permanent FD. But in order to avoid to lock // s_fd_table_lock we go through the VFS table. - for (int i = 0; i < s_vfs_count; ++i) { - // Note: s_vfs_count could have changed since the start of vfs_select() call. However, that change doesn't + for (int i = 0; i < s_vfs_upper_bound; ++i) { + // Note: s_vfs_upper_bound could have changed since the start of vfs_select() call. However, that change doesn't // matter here stop_socket_select() will be called for only valid VFS drivers. const vfs_entry_t *vfs = s_vfs[i]; if (vfs != NULL @@ -1714,8 +1713,8 @@ void esp_vfs_select_triggered_isr(esp_vfs_select_sem_t sem, BaseType_t *woken) // Another way would be to go through s_fd_table and find the VFS // which has a permanent FD. But in order to avoid to lock // s_fd_table_lock we go through the VFS table. - for (int i = 0; i < s_vfs_count; ++i) { - // Note: s_vfs_count could have changed since the start of vfs_select() call. However, that change doesn't + for (int i = 0; i < s_vfs_upper_bound; ++i) { + // Note: s_vfs_upper_bound could have changed since the start of vfs_select() call. However, that change doesn't // matter here stop_socket_select() will be called for only valid VFS drivers. const vfs_entry_t *vfs = s_vfs[i]; if (vfs != NULL