From 90fdeb095557f309b42f194d788c0787557fa884 Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Fri, 14 Feb 2020 15:09:22 +0100 Subject: [PATCH] NVS Flash: prevent erasing initialized partition Closes https://github.com/espressif/esp-idf/issues/4755 Closes https://github.com/espressif/esp-idf/issues/2777 * nvs_flash_erase_partition() checks whether the parition in question is initialized already and will return an error if so * reflect changes in the documentation --- components/nvs_flash/include/nvs_flash.h | 15 +++-- components/nvs_flash/src/nvs_api.cpp | 79 ++++++++++++++---------- components/nvs_flash/test/test_nvs.c | 33 +++++++--- tools/ci/config/target-test.yml | 2 +- 4 files changed, 86 insertions(+), 43 deletions(-) diff --git a/components/nvs_flash/include/nvs_flash.h b/components/nvs_flash/include/nvs_flash.h index 55b218ec56..eafca2ce18 100644 --- a/components/nvs_flash/include/nvs_flash.h +++ b/components/nvs_flash/include/nvs_flash.h @@ -88,30 +88,37 @@ esp_err_t nvs_flash_deinit_partition(const char* partition_label); /** * @brief Erase the default NVS partition * - * This function erases all contents of the default NVS partition (one with label "nvs") + * Erases all contents of the default NVS partition (one with label "nvs"). + * + * @note If the partition is initialized, this function first de-initializes it. Afterwards, the partition has to + * be initialized again to be used. * * @return * - ESP_OK on success * - ESP_ERR_NOT_FOUND if there is no NVS partition labeled "nvs" in the * partition table + * - different error in case de-initialization fails (shouldn't happen) */ esp_err_t nvs_flash_erase(void); /** * @brief Erase specified NVS partition * - * This function erases all contents of specified NVS partition + * Erase all content of a specified NVS partition * - * @param[in] part_name Name (label) of the partition to be erased + * @note If the partition is initialized, this function first de-initializes it. Afterwards, the partition has to + * be initialized again to be used. + * + * @param[in] part_name Name (label) of the partition which should be erased * * @return * - ESP_OK on success * - ESP_ERR_NOT_FOUND if there is no NVS partition with the specified name * in the partition table + * - different error in case de-initialization fails (shouldn't happen) */ esp_err_t nvs_flash_erase_partition(const char *part_name); - /** * @brief Initialize the default NVS partition. * diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp index cc73d0e43a..0357494168 100644 --- a/components/nvs_flash/src/nvs_api.cpp +++ b/components/nvs_flash/src/nvs_api.cpp @@ -143,6 +143,40 @@ extern "C" esp_err_t nvs_flash_secure_init_custom(const char *partName, uint32_t } #endif +static esp_err_t close_handles_and_deinit(const char* part_name) +{ + nvs::Storage* storage = lookup_storage_from_name(part_name); + if (!storage) { + return ESP_ERR_NVS_NOT_INITIALIZED; + } + +#ifdef CONFIG_NVS_ENCRYPTION + if(EncrMgr::isEncrActive()) { + auto encrMgr = EncrMgr::getInstance(); + encrMgr->removeSecurityContext(storage->getBaseSector()); + } +#endif + + /* Clean up handles related to the storage being deinitialized */ + auto it = s_nvs_handles.begin(); + auto next = it; + while(it != s_nvs_handles.end()) { + next++; + if (it->mStoragePtr == storage) { + ESP_LOGD(TAG, "Deleting handle %d (ns=%d) related to partition \"%s\" (missing call to nvs_close?)", + it->mHandle, it->mNsIndex, part_name); + s_nvs_handles.erase(it); + delete static_cast(it); + } + it = next; + } + + /* Finally delete the storage itself */ + s_nvs_storage_list.erase(storage); + delete storage; + + return ESP_OK; +} #ifdef ESP_PLATFORM extern "C" esp_err_t nvs_flash_init_partition(const char *part_name) @@ -201,6 +235,19 @@ extern "C" esp_err_t nvs_flash_secure_init(nvs_sec_cfg_t* cfg) extern "C" esp_err_t nvs_flash_erase_partition(const char *part_name) { + Lock::init(); + Lock lock; + + // if the partition is initialized, uninitialize it first + if (lookup_storage_from_name(part_name)) { + esp_err_t err = close_handles_and_deinit(part_name); + + // only hypothetical/future case, deinit_partition() only fails if partition is uninitialized + if (err != ESP_OK) { + return err; + } + } + const esp_partition_t* partition = esp_partition_find_first( ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_NVS, part_name); if (partition == NULL) { @@ -221,37 +268,7 @@ extern "C" esp_err_t nvs_flash_deinit_partition(const char* partition_name) Lock::init(); Lock lock; - nvs::Storage* storage = lookup_storage_from_name(partition_name); - if (!storage) { - return ESP_ERR_NVS_NOT_INITIALIZED; - } - -#ifdef CONFIG_NVS_ENCRYPTION - if(EncrMgr::isEncrActive()) { - auto encrMgr = EncrMgr::getInstance(); - encrMgr->removeSecurityContext(storage->getBaseSector()); - } -#endif - - /* Clean up handles related to the storage being deinitialized */ - auto it = s_nvs_handles.begin(); - auto next = it; - while(it != s_nvs_handles.end()) { - next++; - if (it->mStoragePtr == storage) { - ESP_LOGD(TAG, "Deleting handle %d (ns=%d) related to partition \"%s\" (missing call to nvs_close?)", - it->mHandle, it->mNsIndex, partition_name); - s_nvs_handles.erase(it); - delete static_cast(it); - } - it = next; - } - - /* Finally delete the storage itself */ - s_nvs_storage_list.erase(storage); - delete storage; - - return ESP_OK; + return close_handles_and_deinit(partition_name); } extern "C" esp_err_t nvs_flash_deinit(void) diff --git a/components/nvs_flash/test/test_nvs.c b/components/nvs_flash/test/test_nvs.c index 7eaafc2951..7a395a0d94 100644 --- a/components/nvs_flash/test/test_nvs.c +++ b/components/nvs_flash/test/test_nvs.c @@ -18,16 +18,35 @@ static const char* TAG = "test_nvs"; +TEST_CASE("flash erase deinitializes initialized partition", "[nvs]") +{ + nvs_handle_t handle; + esp_err_t err = nvs_flash_init(); + if (err == ESP_ERR_NVS_NO_FREE_PAGES || err == ESP_ERR_NVS_NEW_VERSION_FOUND) { + nvs_flash_erase(); + err = nvs_flash_init(); + } + ESP_ERROR_CHECK( err ); + + TEST_ESP_OK(nvs_flash_init()); + TEST_ESP_OK(nvs_open("uninit_ns", NVS_READWRITE, &handle)); + nvs_close(handle); + TEST_ESP_OK(nvs_flash_erase()); + + // exptected: no partition is initialized since nvs_flash_erase() deinitialized the partition again + TEST_ESP_ERR(ESP_ERR_NVS_NOT_INITIALIZED, nvs_open("uninit_ns", NVS_READWRITE, &handle)); + + // just to be sure it's deinitialized in case of error and not affecting other tests + nvs_flash_deinit(); +} + TEST_CASE("various nvs tests", "[nvs]") { nvs_handle_t handle_1; esp_err_t err = nvs_flash_init(); if (err == ESP_ERR_NVS_NO_FREE_PAGES || err == ESP_ERR_NVS_NEW_VERSION_FOUND) { ESP_LOGW(TAG, "nvs_flash_init failed (0x%x), erasing partition and retrying", err); - const esp_partition_t* nvs_partition = esp_partition_find_first( - ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_NVS, NULL); - assert(nvs_partition && "partition table must have an NVS partition"); - ESP_ERROR_CHECK( esp_partition_erase_range(nvs_partition, 0, nvs_partition->size) ); + ESP_ERROR_CHECK(nvs_flash_erase()); err = nvs_flash_init(); } ESP_ERROR_CHECK( err ); @@ -94,7 +113,7 @@ TEST_CASE("calculate used and free space", "[nvs]") const esp_partition_t* nvs_partition = esp_partition_find_first( ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_NVS, NULL); assert(nvs_partition && "partition table must have an NVS partition"); - ESP_ERROR_CHECK( esp_partition_erase_range(nvs_partition, 0, nvs_partition->size) ); + ESP_ERROR_CHECK(nvs_flash_erase()); err = nvs_flash_init(); } ESP_ERROR_CHECK( err ); @@ -102,8 +121,8 @@ TEST_CASE("calculate used and free space", "[nvs]") // erase if have any namespace TEST_ESP_OK(nvs_get_stats(NULL, &stat1)); if(stat1.namespace_count != 0) { - TEST_ESP_OK(nvs_flash_erase()); TEST_ESP_OK(nvs_flash_deinit()); + TEST_ESP_OK(nvs_flash_erase()); TEST_ESP_OK(nvs_flash_init()); } @@ -216,8 +235,8 @@ TEST_CASE("calculate used and free space", "[nvs]") nvs_close(handle_3); - TEST_ESP_OK(nvs_flash_erase()); TEST_ESP_OK(nvs_flash_deinit()); + TEST_ESP_OK(nvs_flash_erase()); } TEST_CASE("check for memory leaks in nvs_set_blob", "[nvs]") diff --git a/tools/ci/config/target-test.yml b/tools/ci/config/target-test.yml index 58e0eddd99..76dc4a87d8 100644 --- a/tools/ci/config/target-test.yml +++ b/tools/ci/config/target-test.yml @@ -223,7 +223,7 @@ example_test_009: UT_001: extends: .unit_test_template - parallel: 29 + parallel: 30 tags: - ESP32_IDF - UT_T1_1