From 3d85e64b21fef16fbe123f744d6bc485536eb58d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20M=C3=BAdry?= Date: Mon, 22 Jun 2026 13:59:57 +0200 Subject: [PATCH] test(vfs): Test changes regarding VFS register incorrect check fix --- .../vfs/test_apps/main/test_vfs_paths.c | 185 +++++++++++++++++- 1 file changed, 184 insertions(+), 1 deletion(-) diff --git a/components/vfs/test_apps/main/test_vfs_paths.c b/components/vfs/test_apps/main/test_vfs_paths.c index 65fc624fcdb..d9cdc0015d4 100644 --- a/components/vfs/test_apps/main/test_vfs_paths.c +++ b/components/vfs/test_apps/main/test_vfs_paths.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 */ @@ -270,3 +270,186 @@ TEST_CASE("vfs checks mount point path", "[vfs]") test_register_ok("/23456789012345"); test_register_fail("/234567890123456"); } + +/* Regression test for a slot-accounting bug in esp_vfs_register_fs_common(). + * + * The registration code used to keep an ever-increasing counter (s_vfs_count/s_vfs_upper_bound) + * which was incremented when a VFS was registered into a new top slot, but was + * never decremented on unregister. The "is there room for another VFS?" check + * compared that counter against VFS_MAX_COUNT. As a result, after the VFS table + * had been filled once, repeatedly unregistering and re-registering a VFS would + * eventually (and permanently) fail with ESP_ERR_NO_MEM even though free slots + * were available. + * + * The bug was fixed by checking for an actually-free slot (esp_get_free_index()) + * instead of relying on the counter, and by lowering the upper bound when the + * topmost entry is removed. + * + * These tests register/unregister the dummy VFS many times to ensure the slot + * accounting stays correct over time. They are expected to fail on the + * pre-fix code and pass on the fixed code. + */ + +/* Number of VFS slots available for this test app (see sdkconfig.defaults). */ +#define TEST_VFS_MAX_COUNT CONFIG_VFS_MAX_COUNT + +/* Build a short, unique mount point ("/t") for the test VFS entries. */ +static void make_test_path(char *buf, size_t buf_len, int idx) +{ + snprintf(buf, buf_len, "/t%d", idx); +} + +TEST_CASE("vfs can re-register after the table has been filled", "[vfs]") +{ + /* Separate context per registered VFS, because ESP_VFS_FLAG_CONTEXT_PTR + * stores the pointer for the lifetime of the registration. */ + static dummy_vfs_t insts[TEST_VFS_MAX_COUNT]; + char paths[TEST_VFS_MAX_COUNT][8]; + + /* Fill every free VFS slot. Some slots may already be taken by VFSes that + * the system registered at startup (e.g. /dev/null), so we keep going until + * registration reports the table is full instead of assuming a fixed count. */ + int registered = 0; + for (int i = 0; i < TEST_VFS_MAX_COUNT; ++i) { + make_test_path(paths[i], sizeof(paths[i]), i); + insts[i] = (dummy_vfs_t) { .match_path = "", .called = false }; + esp_err_t err = esp_vfs_register_fs(paths[i], &s_dummy_vfs, + ESP_VFS_FLAG_CONTEXT_PTR, &insts[i]); + if (err == ESP_ERR_NO_MEM) { + break; // table is full + } + TEST_ESP_OK(err); + registered++; + } + + /* We must have been able to register at least one entry, and the table must + * actually be full now (the next registration must fail with NO_MEM). */ + TEST_ASSERT_GREATER_THAN(0, registered); + dummy_vfs_t overflow_inst = { .match_path = "", .called = false }; + TEST_ESP_ERR(ESP_ERR_NO_MEM, + esp_vfs_register_fs("/overflow", &s_dummy_vfs, + ESP_VFS_FLAG_CONTEXT_PTR, &overflow_inst)); + + /* Free the topmost entry we registered, then try to register again. + * On the buggy code the stale counter would still equal VFS_MAX_COUNT and + * this registration would incorrectly fail with ESP_ERR_NO_MEM. */ + int top = registered - 1; + TEST_ESP_OK(esp_vfs_unregister(paths[top])); + + insts[top] = (dummy_vfs_t) { .match_path = "", .called = false }; + TEST_ESP_OK(esp_vfs_register_fs(paths[top], &s_dummy_vfs, + ESP_VFS_FLAG_CONTEXT_PTR, &insts[top])); + + /* Clean up everything we registered so the leak check in tearDown passes. */ + for (int i = 0; i < registered; ++i) { + TEST_ESP_OK(esp_vfs_unregister(paths[i])); + } +} + +TEST_CASE("vfs can re-register into a hole in the middle of a full table", "[vfs]") +{ + /* This targets the case where an entry that is NOT the topmost one is + * unregistered while the table is full. That leaves a NULL "hole" in the + * middle of the s_vfs table and, crucially, does NOT lower s_vfs_upper_bound + * (the hole is below the upper bound). Registering again must reuse that + * hole. + * + * On the buggy code the stale counter stayed at VFS_MAX_COUNT, so this + * re-registration failed with ESP_ERR_NO_MEM even though the freed middle + * slot was available. */ + static dummy_vfs_t insts[TEST_VFS_MAX_COUNT]; + char paths[TEST_VFS_MAX_COUNT][8]; + + /* Fill the table completely. */ + int registered = 0; + for (int i = 0; i < TEST_VFS_MAX_COUNT; ++i) { + make_test_path(paths[i], sizeof(paths[i]), i); + insts[i] = (dummy_vfs_t) { .match_path = "", .called = false }; + esp_err_t err = esp_vfs_register_fs(paths[i], &s_dummy_vfs, + ESP_VFS_FLAG_CONTEXT_PTR, &insts[i]); + if (err == ESP_ERR_NO_MEM) { + break; // table is full + } + TEST_ESP_OK(err); + registered++; + } + + /* We need at least 3 entries so that there is a genuine middle entry that is + * neither the first nor the topmost slot. */ + TEST_ASSERT_GREATER_OR_EQUAL_INT(3, registered); + + /* The table must be full now. */ + dummy_vfs_t overflow_inst = { .match_path = "", .called = false }; + TEST_ESP_ERR(ESP_ERR_NO_MEM, + esp_vfs_register_fs("/overflow", &s_dummy_vfs, + ESP_VFS_FLAG_CONTEXT_PTR, &overflow_inst)); + + /* Unregister an entry in the middle (not the first, not the topmost). This + * creates a NULL hole below the upper bound. */ + int middle = registered / 2; + TEST_ASSERT_NOT_EQUAL(0, middle); + TEST_ASSERT_NOT_EQUAL(registered - 1, middle); + TEST_ESP_OK(esp_vfs_unregister(paths[middle])); + + /* Register again: with only the middle slot free, esp_get_free_index() must + * return exactly that slot and the registration must succeed. */ + insts[middle] = (dummy_vfs_t) { .match_path = "", .called = false }; + TEST_ESP_OK(esp_vfs_register_fs(paths[middle], &s_dummy_vfs, + ESP_VFS_FLAG_CONTEXT_PTR, &insts[middle])); + + /* The table must be full again - the hole was reused, not appended. */ + TEST_ESP_ERR(ESP_ERR_NO_MEM, + esp_vfs_register_fs("/overflow", &s_dummy_vfs, + ESP_VFS_FLAG_CONTEXT_PTR, &overflow_inst)); + + /* Clean up everything. */ + for (int i = 0; i < registered; ++i) { + TEST_ESP_OK(esp_vfs_unregister(paths[i])); + } +} + +TEST_CASE("vfs survives repeated register/unregister cycles", "[vfs]") +{ + /* Fill the whole VFS table and then drain it again, several times over. + * + * On the fixed code every round must be able to register exactly the same + * number of entries, because unregistering lowers the upper bound again. + * On the buggy code the stale counter never came back down, so the second + * and later rounds would be able to register fewer entries (and eventually + * none at all), which this test detects. */ + static dummy_vfs_t insts[TEST_VFS_MAX_COUNT]; + char paths[TEST_VFS_MAX_COUNT][8]; + for (int i = 0; i < TEST_VFS_MAX_COUNT; ++i) { + make_test_path(paths[i], sizeof(paths[i]), i); + } + + const int rounds = 5; + int first_round_count = -1; + for (int r = 0; r < rounds; ++r) { + /* Register until the table is full. */ + int count = 0; + for (int i = 0; i < TEST_VFS_MAX_COUNT; ++i) { + insts[i] = (dummy_vfs_t) { .match_path = "", .called = false }; + esp_err_t err = esp_vfs_register_fs(paths[i], &s_dummy_vfs, + ESP_VFS_FLAG_CONTEXT_PTR, &insts[i]); + if (err == ESP_ERR_NO_MEM) { + break; + } + TEST_ESP_OK(err); + count++; + } + + if (first_round_count < 0) { + first_round_count = count; + TEST_ASSERT_GREATER_THAN(0, first_round_count); + } else { + /* The capacity must not shrink between rounds. */ + TEST_ASSERT_EQUAL_INT(first_round_count, count); + } + + /* Drain the table again. */ + for (int i = 0; i < count; ++i) { + TEST_ESP_OK(esp_vfs_unregister(paths[i])); + } + } +}