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

@@ -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.
  *

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<HandleEntry*>(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);
+    return close_handles_and_deinit(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<HandleEntry*>(it);
-        }
-        it = next;
-    }
-
-    /* Finally delete the storage itself */
-    s_nvs_storage_list.erase(storage);
-    delete storage;
-
-    return ESP_OK;
 }
 
 extern "C" esp_err_t nvs_flash_deinit(void)

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_ERROR_CHECK(nvs_flash_erase());
-                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) );
         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]")

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