From d52a21de5cf75bf33fad5ea60d21f131d1491d69 Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Mon, 8 Mar 2021 10:44:15 +0800 Subject: [PATCH] Random NVS fixes * Checking Lock::init() now * fixed typos in nvs_flash.h * Added missing parameter checks in nvs encryption function * Closes IDF-1462 * Closes IDF-2900 --- components/nvs_flash/include/nvs_flash.h | 10 +++-- components/nvs_flash/src/nvs_api.cpp | 38 ++++++++++++++++--- components/nvs_flash/src/nvs_cxx_api.cpp | 8 ++++ .../nvs_flash/src/nvs_handle_locked.cpp | 4 +- .../nvs_flash/src/nvs_handle_locked.hpp | 5 +++ components/nvs_flash/src/nvs_platform.hpp | 6 ++- 6 files changed, 57 insertions(+), 14 deletions(-) diff --git a/components/nvs_flash/include/nvs_flash.h b/components/nvs_flash/include/nvs_flash.h index a5ad9ac289..7e3d6a7985 100644 --- a/components/nvs_flash/include/nvs_flash.h +++ b/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. diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp index 3a2a3495b6..dd49845c94 100644 --- a/components/nvs_flash/src/nvs_api.cpp +++ b/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; diff --git a/components/nvs_flash/src/nvs_cxx_api.cpp b/components/nvs_flash/src/nvs_cxx_api.cpp index 65fb1128b2..b315fecdf1 100644 --- a/components/nvs_flash/src/nvs_cxx_api.cpp +++ b/components/nvs_flash/src/nvs_cxx_api.cpp @@ -31,6 +31,14 @@ std::unique_ptr 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; diff --git a/components/nvs_flash/src/nvs_handle_locked.cpp b/components/nvs_flash/src/nvs_handle_locked.cpp index 89e5cbbca0..3921887448 100644 --- a/components/nvs_flash/src/nvs_handle_locked.cpp +++ b/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; diff --git a/components/nvs_flash/src/nvs_handle_locked.hpp b/components/nvs_flash/src/nvs_handle_locked.hpp index 39d514c7a6..b9b0cb5620 100644 --- a/components/nvs_flash/src/nvs_handle_locked.hpp +++ b/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(); diff --git a/components/nvs_flash/src/nvs_platform.hpp b/components/nvs_flash/src/nvs_platform.hpp index 5c6b5b8b23..c47fa40020 100644 --- a/components/nvs_flash/src/nvs_platform.hpp +++ b/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