Merge branch 'bugfix/nvs_missing_API_arg_checks' into 'master'

Random NVS fixes

Closes IDF-1462 and IDF-2900

See merge request espressif/esp-idf!12649

components/nvs_flash/include/nvs_flash.h

@@ -181,7 +181,7 @@ esp_err_t nvs_flash_erase_partition_ptr(const esp_partition_t *partition);
  *                              If cfg is NULL, no encryption is used.
  *
  * @return
- *      - ESP_OK if storage was successfully initialized.
+ *      - ESP_OK if storage has been initialized successfully.
  *      - ESP_ERR_NVS_NO_FREE_PAGES if the NVS storage contains no empty pages
  *        (which may happen if NVS partition was truncated)
  *      - ESP_ERR_NOT_FOUND if no partition with label "nvs" is found in the partition table
@@ -193,13 +193,13 @@ esp_err_t nvs_flash_secure_init(nvs_sec_cfg_t* cfg);
 /**
  * @brief Initialize NVS flash storage for the specified partition.
  *
- * @param[in]  partition_label   Label of the partition. Note that internally a reference to
+ * @param[in]  partition_label   Label of the partition. Note that internally, a reference to
  *                               passed value is kept and it should be accessible for future operations
  *
  * @param[in]  cfg Security configuration (keys) to be used for NVS encryption/decryption.
  *                              If cfg is null, no encryption/decryption is used.
  * @return
- *      - ESP_OK if storage was successfully initialized.
+ *      - ESP_OK if storage has been initialized successfully.
  *      - ESP_ERR_NVS_NO_FREE_PAGES if the NVS storage contains no empty pages
  *        (which may happen if NVS partition was truncated)
  *      - ESP_ERR_NOT_FOUND if specified partition is not found in the partition table
@@ -221,6 +221,7 @@ esp_err_t nvs_flash_secure_init_partition(const char *partition_label, nvs_sec_c
  *
  * @return
  *      -ESP_OK, if cfg was read successfully;
+ *      -ESP_INVALID_ARG, if partition or cfg;
  *      -or error codes from esp_partition_write/erase APIs.
  */
 
@@ -236,10 +237,11 @@ esp_err_t nvs_flash_generate_keys(const esp_partition_t* partition, nvs_sec_cfg_
  * @param[out] cfg       Pointer to nvs security configuration structure.
  *                       Pointer must be non-NULL.
  *
- * @note  Provided parition is assumed to be marked 'encrypted'.
+ * @note  Provided partition is assumed to be marked 'encrypted'.
  *
  * @return
  *      -ESP_OK, if cfg was read successfully;
+ *      -ESP_INVALID_ARG, if partition or cfg;
  *      -ESP_ERR_NVS_KEYS_NOT_INITIALIZED, if the partition is not yet written with keys.
  *      -ESP_ERR_NVS_CORRUPT_KEY_PART, if the partition containing keys is found to be corrupt
  *      -or error codes from esp_partition_read API.

components/nvs_flash/src/nvs_api.cpp

@@ -103,7 +103,10 @@ static esp_err_t close_handles_and_deinit(const char* part_name)
 
 extern "C" esp_err_t nvs_flash_init_partition_ptr(const esp_partition_t *partition)
 {
-    Lock::init();
+    esp_err_t lock_result = Lock::init();
+    if (lock_result != ESP_OK) {
+        return lock_result;
+    }
     Lock lock;
 
     if (partition == nullptr) {
@@ -129,7 +132,10 @@ extern "C" esp_err_t nvs_flash_init_partition_ptr(const esp_partition_t *partiti
 #ifndef LINUX_TARGET
 extern "C" esp_err_t nvs_flash_init_partition(const char *part_name)
 {
-    Lock::init();
+    esp_err_t lock_result = Lock::init();
+    if (lock_result != ESP_OK) {
+        return lock_result;
+    }
     Lock lock;
 
     return NVSPartitionManager::get_instance()->init_partition(part_name);
@@ -175,7 +181,10 @@ extern "C" esp_err_t nvs_flash_init(void)
 #ifdef CONFIG_NVS_ENCRYPTION
 extern "C" esp_err_t nvs_flash_secure_init_partition(const char *part_name, nvs_sec_cfg_t* cfg)
 {
-    Lock::init();
+    esp_err_t lock_result = Lock::init();
+    if (lock_result != ESP_OK) {
+        return lock_result;
+    }
     Lock lock;
 
     return NVSPartitionManager::get_instance()->secure_init_partition(part_name, cfg);
@@ -189,7 +198,10 @@ 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();
+    esp_err_t lock_result = Lock::init();
+    if (lock_result != ESP_OK) {
+        return lock_result;
+    }
     Lock lock;
 
     // if the partition is initialized, uninitialize it first
@@ -213,7 +225,10 @@ extern "C" esp_err_t nvs_flash_erase_partition(const char *part_name)
 
 extern "C" esp_err_t nvs_flash_erase_partition_ptr(const esp_partition_t *partition)
 {
-    Lock::init();
+    esp_err_t lock_result = Lock::init();
+    if (lock_result != ESP_OK) {
+        return lock_result;
+    }
     Lock lock;
 
     if (partition == nullptr) {
@@ -241,7 +256,10 @@ extern "C" esp_err_t nvs_flash_erase(void)
 
 extern "C" esp_err_t nvs_flash_deinit_partition(const char* partition_name)
 {
-    Lock::init();
+    esp_err_t lock_result = Lock::init();
+    if (lock_result != ESP_OK) {
+        return lock_result;
+    }
     Lock lock;
 
     return close_handles_and_deinit(partition_name);
@@ -565,6 +583,10 @@ extern "C" esp_err_t nvs_get_used_entry_count(nvs_handle_t c_handle, size_t* use
 
 extern "C" esp_err_t nvs_flash_generate_keys(const esp_partition_t* partition, nvs_sec_cfg_t* cfg)
 {
+    if (cfg == nullptr || partition == nullptr) {
+        return ESP_ERR_INVALID_ARG;
+    }
+
     auto err = esp_partition_erase_range(partition, 0, partition->size);
     if(err != ESP_OK) {
         return err;
@@ -625,6 +647,10 @@ extern "C" esp_err_t nvs_flash_generate_keys(const esp_partition_t* partition, n
 
 extern "C" esp_err_t nvs_flash_read_security_cfg(const esp_partition_t* partition, nvs_sec_cfg_t* cfg)
 {
+    if (cfg == nullptr || partition == nullptr) {
+        return ESP_ERR_INVALID_ARG;
+    }
+
     uint8_t eky_raw[NVS_KEY_SIZE], tky_raw[NVS_KEY_SIZE];
     uint32_t crc_raw, crc_read, crc_calc;
 

components/nvs_flash/src/nvs_cxx_api.cpp

@@ -31,6 +31,14 @@ std::unique_ptr<NVSHandle> open_nvs_handle_from_partition(const char *partition_
         return nullptr;
     }
 
+    esp_err_t lock_result = Lock::init();
+    if (lock_result != ESP_OK) {
+        if (err != nullptr) {
+            *err = lock_result;
+        }
+        return nullptr;
+    }
+
     Lock lock;
 
     NVSHandleSimple *handle_simple;

components/nvs_flash/src/nvs_handle_locked.cpp

@@ -15,9 +15,7 @@
 
 namespace nvs {
 
-NVSHandleLocked::NVSHandleLocked(NVSHandleSimple *handle) : handle(handle) {
-    Lock::init();
-}
+NVSHandleLocked::NVSHandleLocked(NVSHandleSimple *handle) : handle(handle) { }
 
 NVSHandleLocked::~NVSHandleLocked() {
     Lock lock;

components/nvs_flash/src/nvs_handle_locked.hpp

@@ -30,6 +30,11 @@ namespace nvs {
  */
 class NVSHandleLocked : public NVSHandle {
 public:
+    /**
+     * @param handle The NVSHandleSimple instance which this class "decorates" (see Decorator design pattern).
+     *
+     * @note Lock::init() MUST be called BEFORE an instance of this class is used.
+     */
     NVSHandleLocked(NVSHandleSimple *handle);
 
     virtual ~NVSHandleLocked();

components/nvs_flash/src/nvs_platform.hpp

@@ -21,7 +21,11 @@ class Lock
 public:
     Lock() { }
     ~Lock() { }
-    static void init() {}
+    static esp_err_t init()
+    {
+        return ESP_OK;
+    }
+
     static void uninit() {}
 };
 } // namespace nvs