From ee836d236f3c7f4caf3ce3d785a3aceeecea5cf7 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 11 Dec 2023 23:04:35 +0800 Subject: [PATCH 1/7] fix(nvs): prevent out of bounds write if blob data is inconsistent --- components/nvs_flash/src/nvs_storage.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index ccba2263f7..af5c3f941b 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -490,6 +490,11 @@ esp_err_t Storage::readMultiPageBlob(uint8_t nsIndex, const char* key, void* dat } return err; } + if (item.varLength.dataSize > dataSize - offset) { + /* The size of the entry in the index is inconsistent with the sum of the sizes of chunks */ + err = ESP_ERR_NVS_INVALID_LENGTH; + break; + } err = findPage->readItem(nsIndex, ItemType::BLOB_DATA, key, static_cast(data) + offset, item.varLength.dataSize, static_cast (chunkStart) + chunkNum); if (err != ESP_OK) { return err; @@ -498,11 +503,14 @@ esp_err_t Storage::readMultiPageBlob(uint8_t nsIndex, const char* key, void* dat offset += item.varLength.dataSize; } + + if (err == ESP_ERR_NVS_NOT_FOUND || err == ESP_ERR_NVS_INVALID_LENGTH) { + // cleanup if a chunk is not found or the size is inconsistent + eraseMultiPageBlob(nsIndex, key); + } + NVS_ASSERT_OR_RETURN(offset == dataSize, ESP_FAIL); - if (err == ESP_ERR_NVS_NOT_FOUND) { - eraseMultiPageBlob(nsIndex, key); // cleanup if a chunk is not found - } return err; } From 9a9ef9d843823042298b5f4ba22ec0e2c5004404 Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Tue, 23 Jan 2024 17:09:18 +0100 Subject: [PATCH 2/7] fix(nvs): added check and erase of mismatched BLOB_DATA on init --- components/nvs_flash/src/nvs_storage.cpp | 82 +++++++++++++++++++++++- components/nvs_flash/src/nvs_storage.hpp | 7 +- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index af5c3f941b..addf44305b 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -20,6 +20,9 @@ #endif #endif // !ESP_PLATFORM +#include "esp_log.h" +#define TAG "nvs_storage" + namespace nvs { @@ -53,6 +56,9 @@ esp_err_t Storage::populateBlobIndices(TBlobIndexList& blobIdxList) entry->nsIndex = item.nsIndex; entry->chunkStart = item.blobIndex.chunkStart; entry->chunkCount = item.blobIndex.chunkCount; + entry->dataSize = item.blobIndex.dataSize; + entry->observedDataSize = 0; + entry->observedChunkCount = 0; blobIdxList.push_back(entry); itemIndex += item.span; @@ -62,6 +68,76 @@ esp_err_t Storage::populateBlobIndices(TBlobIndexList& blobIdxList) return ESP_OK; } +// Check BLOB_DATA entries belonging to BLOB_INDEX entries for mismatched records. +// BLOB_INDEX record is compared with information collected from BLOB_DATA records +// matched using namespace index, key and chunk version. Mismatched summary length +// or wrong number of chunks are checked. Mismatched BLOB_INDEX data are deleted +// and removed from the blobIdxList. The BLOB_DATA are left as orphans and removed +// later by the call to eraseOrphanDataBlobs(). +void Storage::eraseMismatchedBlobIndexes(TBlobIndexList& blobIdxList) +{ + for (auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { + Page& p = *it; + size_t itemIndex = 0; + Item item; + /* Chunks with same and with chunkIndex in the following ranges + * belong to same family. + * 1) VER_0_OFFSET <= chunkIndex < VER_1_OFFSET-1 => Version0 chunks + * 2) VER_1_OFFSET <= chunkIndex < VER_ANY => Version1 chunks + */ + while (p.findItem(Page::NS_ANY, ItemType::BLOB_DATA, nullptr, itemIndex, item) == ESP_OK) { + + auto iter = std::find_if(blobIdxList.begin(), + blobIdxList.end(), + [=] (const BlobIndexNode& e) -> bool + {return (strncmp(item.key, e.key, sizeof(e.key) - 1) == 0) + && (item.nsIndex == e.nsIndex) + && (item.chunkIndex >= static_cast (e.chunkStart)) + && (item.chunkIndex < static_cast ((e.chunkStart == nvs::VerOffset::VER_0_OFFSET) ? nvs::VerOffset::VER_1_OFFSET : nvs::VerOffset::VER_ANY));}); + if (iter != std::end(blobIdxList)) { + // accumulate the size + iter->observedDataSize += item.varLength.dataSize; + iter->observedChunkCount++; + } + itemIndex += item.span; + } + } + + auto iter = blobIdxList.begin(); + while (iter != blobIdxList.end()) + { + if ( (iter->observedDataSize != iter->dataSize) || (iter->observedChunkCount != iter->chunkCount) ) + { + // Delete blob_index from flash + // This is very rare case, so we can loop over all pages + for (auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { + // skip pages in non eligible states + if (it->state() == nvs::Page::PageState::CORRUPT + || it->state() == nvs::Page::PageState::INVALID + || it->state() == nvs::Page::PageState::UNINITIALIZED){ + continue; + } + + Page& p = *it; + if(p.eraseItem(iter->nsIndex, nvs::ItemType::BLOB_IDX, iter->key, 255, iter->chunkStart) == ESP_OK){ + break; + } + } + + // Delete blob index from the blobIdxList + auto tmp = iter; + ++iter; + blobIdxList.erase(tmp); + delete (nvs::Storage::BlobIndexNode*)tmp; + } + else + { + // Blob index OK + ++iter; + } + } +} + void Storage::eraseOrphanDataBlobs(TBlobIndexList& blobIdxList) { for (auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { @@ -85,6 +161,7 @@ void Storage::eraseOrphanDataBlobs(TBlobIndexList& blobIdxList) if (iter == std::end(blobIdxList)) { p.eraseItem(item.nsIndex, item.datatype, item.key, item.chunkIndex); } + itemIndex += item.span; } } @@ -143,6 +220,9 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount) return ESP_ERR_NO_MEM; } + // remove blob indexes with mismatched blob data length or chunk count + eraseMismatchedBlobIndexes(blobIdxList); + // Remove the entries for which there is no parent multi-page index. eraseOrphanDataBlobs(blobIdxList); diff --git a/components/nvs_flash/src/nvs_storage.hpp b/components/nvs_flash/src/nvs_storage.hpp index c2df7dc470..6dbb6b1007 100644 --- a/components/nvs_flash/src/nvs_storage.hpp +++ b/components/nvs_flash/src/nvs_storage.hpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -48,6 +48,9 @@ class Storage : public intrusive_list_node, public ExceptionlessAllocat uint8_t nsIndex; uint8_t chunkCount; VerOffset chunkStart; + size_t dataSize; + size_t observedDataSize; + size_t observedChunkCount; }; typedef intrusive_list TBlobIndexList; @@ -140,6 +143,8 @@ protected: esp_err_t populateBlobIndices(TBlobIndexList&); + void eraseMismatchedBlobIndexes(TBlobIndexList&); + void eraseOrphanDataBlobs(TBlobIndexList&); void fillEntryInfo(Item &item, nvs_entry_info_t &info); From 66b2a500663ba311c7a2ac20ff2211fcc65e43d1 Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Thu, 1 Feb 2024 13:45:59 +0100 Subject: [PATCH 3/7] fix(nvs): corrected findItem to return BLOB_DATA when chunkIndex = CHUNK_ANY --- components/nvs_flash/src/nvs_page.cpp | 30 ++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index 36cde3db25..dd5f81eb6b 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -878,7 +878,10 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si end = ENTRY_COUNT; } - if (nsIndex != NS_ANY && datatype != ItemType::ANY && key != NULL) { + // For BLOB_DATA, we may need to search for all chunk indexes, so the hash list won't help + // mHashIndex caluclates hash from nsIndex, key, chunkIdx + // We may not use mHashList if datatype is BLOB_DATA and chunkIdx is CHUNK_ANY as CHUNK_ANY is used by BLOB_INDEX + if (nsIndex != NS_ANY && key != NULL && (datatype == ItemType::BLOB_DATA && chunkIdx != CHUNK_ANY)) { size_t cachedIndex = mHashList.find(start, Item(nsIndex, datatype, 0, key, chunkIdx)); if (cachedIndex < ENTRY_COUNT) { start = cachedIndex; @@ -933,6 +936,31 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si && item.chunkIndex != chunkIdx) { continue; } + + // We may search for any chunk of BLOB_DATA but find BLOB_INDEX or BLOB instead as it + // uses default value of chunkIdx == CHUNK_ANY, then continue searching + if (chunkIdx == CHUNK_ANY + && datatype == ItemType::BLOB_DATA + && item.datatype != ItemType::BLOB_DATA) { + continue; + } + + // We may search for BLOB but find BLOB_INDEX instead + // In this case it is expected to return ESP_ERR_NVS_TYPE_MISMATCH + if (chunkIdx == CHUNK_ANY + && datatype == ItemType::BLOB + && item.datatype == ItemType::BLOB_IDX) { + return ESP_ERR_NVS_TYPE_MISMATCH; + } + + // We may search for BLOB but find BLOB_DATA instead + // Then continue + if (chunkIdx == CHUNK_ANY + && datatype == ItemType::BLOB + && item.datatype == ItemType::BLOB_DATA) { + continue; + } + /* Blob-index will match the with blob data. * Skip data chunks when searching for blob index*/ if (datatype == ItemType::BLOB_IDX From a1750801f6b0a53182e67f6d55828d0629656862 Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Thu, 1 Feb 2024 13:55:29 +0100 Subject: [PATCH 4/7] fix(nvs): eraseMultiPageBlob to robustly delete all related BLOB_DATA records and respect VER_ANY --- components/nvs_flash/src/nvs_page.hpp | 4 +- components/nvs_flash/src/nvs_storage.cpp | 61 ++++++++++++++++-------- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/components/nvs_flash/src/nvs_page.hpp b/components/nvs_flash/src/nvs_page.hpp index 71888a263f..4aff32ad01 100644 --- a/components/nvs_flash/src/nvs_page.hpp +++ b/components/nvs_flash/src/nvs_page.hpp @@ -98,6 +98,8 @@ public: esp_err_t findItem(uint8_t nsIndex, ItemType datatype, const char* key, size_t &itemIndex, Item& item, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY); + esp_err_t eraseEntryAndSpan(size_t index); + template esp_err_t writeItem(uint8_t nsIndex, const char* key, const T& value) { @@ -188,8 +190,6 @@ protected: esp_err_t writeEntryData(const uint8_t* data, size_t size); - esp_err_t eraseEntryAndSpan(size_t index); - esp_err_t updateFirstUsedEntry(size_t index, size_t span); static constexpr size_t getAlignmentForType(ItemType type) diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index addf44305b..327232e321 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -671,34 +671,57 @@ esp_err_t Storage::eraseMultiPageBlob(uint8_t nsIndex, const char* key, VerOffse if (err != ESP_OK) { return err; } - /* Erase the index first and make children blobs orphan*/ + // Erase the index first and make children blobs orphan err = findPage->eraseItem(nsIndex, ItemType::BLOB_IDX, key, Page::CHUNK_ANY, chunkStart); if (err != ESP_OK) { return err; } - uint8_t chunkCount = item.blobIndex.chunkCount; - - if (chunkStart == VerOffset::VER_ANY) { - chunkStart = item.blobIndex.chunkStart; - } else { - NVS_ASSERT_OR_RETURN(chunkStart == item.blobIndex.chunkStart, ESP_FAIL); + // If caller requires delete of VER_ANY + // We may face dirty NVS partition and version duplicates can be there + // Make second attempt to delete index and ignore eventual not found + if(chunkStart == VerOffset::VER_ANY) + { + err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item, Page::CHUNK_ANY, chunkStart); + if (err == ESP_OK) { + err = findPage->eraseItem(nsIndex, ItemType::BLOB_IDX, key, Page::CHUNK_ANY, chunkStart); + if (err != ESP_OK) { + return err; + } + } else if (err != ESP_ERR_NVS_NOT_FOUND) { + return err; + } } - /* Now erase corresponding chunks*/ - for (uint8_t chunkNum = 0; chunkNum < chunkCount; chunkNum++) { - err = findItem(nsIndex, ItemType::BLOB_DATA, key, findPage, item, static_cast (chunkStart) + chunkNum); + // setup limits for chunkIndex-es to be deleted + uint8_t minChunkIndex = (uint8_t) VerOffset::VER_0_OFFSET; + uint8_t maxChunkIndex = (uint8_t) VerOffset::VER_ANY; - if (err != ESP_OK && err != ESP_ERR_NVS_NOT_FOUND) { - return err; - } else if (err == ESP_ERR_NVS_NOT_FOUND) { - continue; // Keep erasing other chunks - } - err = findPage->eraseItem(nsIndex, ItemType::BLOB_DATA, key, static_cast (chunkStart) + chunkNum); - if (err != ESP_OK) { - return err; - } + if(chunkStart == VerOffset::VER_0_OFFSET) { + maxChunkIndex = (uint8_t) VerOffset::VER_1_OFFSET; + } else if (chunkStart == VerOffset::VER_1_OFFSET) { + minChunkIndex = (uint8_t) VerOffset::VER_1_OFFSET; + } + for (auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) { + size_t itemIndex = 0; + do { + err = it->findItem(nsIndex, ItemType::BLOB_DATA, key, itemIndex, item); + if (err == ESP_ERR_NVS_NOT_FOUND) { + break; + } else if (err == ESP_OK) { + // check if item.chunkIndex is within the version range indicated by chunkStart, if so, delete it + if((item.chunkIndex >= minChunkIndex) && (item.chunkIndex < maxChunkIndex)) { + err = it->eraseEntryAndSpan(itemIndex); + } + + // continue findItem until end of page + itemIndex += item.span; + } + if(err != ESP_OK) { + return err; + } + } while (err == ESP_OK && itemIndex < Page::ENTRY_COUNT); } return ESP_OK; From 0d3bbed981483015680a2c25536ac9d3d47e6a61 Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Wed, 17 Jan 2024 10:44:50 +0100 Subject: [PATCH 5/7] fix(nvs): Improved lockig mechanism for initialization phase --- components/nvs_flash/CMakeLists.txt | 5 +- components/nvs_flash/src/nvs_api.cpp | 10 +-- components/nvs_flash/src/nvs_platform.cpp | 50 +++++++++++ components/nvs_flash/src/nvs_platform.hpp | 92 ++++----------------- components/nvs_flash/src/nvs_storage.cpp | 3 +- components/nvs_flash/test_nvs_host/Makefile | 1 + tools/ci/check_copyright_ignore.txt | 1 - 7 files changed, 78 insertions(+), 84 deletions(-) create mode 100644 components/nvs_flash/src/nvs_platform.cpp diff --git a/components/nvs_flash/CMakeLists.txt b/components/nvs_flash/CMakeLists.txt index a26493ce3d..0f1dde708e 100644 --- a/components/nvs_flash/CMakeLists.txt +++ b/components/nvs_flash/CMakeLists.txt @@ -11,11 +11,12 @@ set(srcs "src/nvs_api.cpp" "src/nvs_partition.cpp" "src/nvs_partition_lookup.cpp" "src/nvs_partition_manager.cpp" - "src/nvs_types.cpp") + "src/nvs_types.cpp" + "src/nvs_platform.cpp") idf_component_register(SRCS "${srcs}" REQUIRES "esp_partition" - PRIV_REQUIRES spi_flash + PRIV_REQUIRES spi_flash newlib INCLUDE_DIRS "include" "../spi_flash/include" PRIV_INCLUDE_DIRS "private_include") diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp index 8d8d0b013d..cf8226bec3 100644 --- a/components/nvs_flash/src/nvs_api.cpp +++ b/components/nvs_flash/src/nvs_api.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -45,10 +45,6 @@ uint32_t NVSHandleEntry::s_nvs_next_handle; extern "C" void nvs_dump(const char *partName); -#ifndef LINUX_TARGET -SemaphoreHandle_t nvs::Lock::mSemaphore = nullptr; -#endif // ! LINUX_TARGET - using namespace std; using namespace nvs; @@ -272,6 +268,10 @@ static esp_err_t nvs_find_ns_handle(nvs_handle_t c_handle, NVSHandleSimple** han extern "C" esp_err_t nvs_open_from_partition(const char *part_name, const char* namespace_name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle) { + esp_err_t lock_result = Lock::init(); + if (lock_result != ESP_OK) { + return lock_result; + } Lock lock; ESP_LOGD(TAG, "%s %s %d", __func__, namespace_name, open_mode); diff --git a/components/nvs_flash/src/nvs_platform.cpp b/components/nvs_flash/src/nvs_platform.cpp new file mode 100644 index 0000000000..045996bccd --- /dev/null +++ b/components/nvs_flash/src/nvs_platform.cpp @@ -0,0 +1,50 @@ +/* + * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include "nvs_platform.hpp" + +using namespace nvs; + +#ifdef LINUX_TARGET +Lock::Lock() {} +Lock::~Lock() {} +esp_err_t nvs::Lock::init() {return ESP_OK;} +void Lock::uninit() {} +#else + +#include "sys/lock.h" + +Lock::Lock() +{ + // Newlib implementation ensures that even if mSemaphore was 0, it gets initialized. + // Locks mSemaphore + _lock_acquire(&mSemaphore); +} + +Lock::~Lock() +{ + // Unlocks mSemaphore + _lock_release(&mSemaphore); +} + +esp_err_t Lock::init() +{ + // Let postpone initialization to the Lock::Lock. + // It is designed to lazy initialize the semaphore in a properly guarded critical section + return ESP_OK; +} + +void Lock::uninit() +{ + // Uninitializes mSemaphore. Please be aware that uninitialization of semaphore shared and held by another thread + // can cause undefined behavior + if (mSemaphore) { + _lock_close(&mSemaphore); + } +} + +_lock_t Lock::mSemaphore = 0; + +#endif diff --git a/components/nvs_flash/src/nvs_platform.hpp b/components/nvs_flash/src/nvs_platform.hpp index c47fa40020..3c9275aa74 100644 --- a/components/nvs_flash/src/nvs_platform.hpp +++ b/components/nvs_flash/src/nvs_platform.hpp @@ -1,82 +1,24 @@ -// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at - -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #pragma once -#ifdef LINUX_TARGET -namespace nvs -{ -class Lock -{ -public: - Lock() { } - ~Lock() { } - static esp_err_t init() - { - return ESP_OK; - } - - static void uninit() {} -}; -} // namespace nvs - -#else // LINUX_TARGET - -#include "freertos/FreeRTOS.h" -#include "freertos/semphr.h" +#include "esp_err.h" namespace nvs { - -class Lock -{ -public: - Lock() + class Lock { - if (mSemaphore) { - xSemaphoreTake(mSemaphore, portMAX_DELAY); - } - } - - ~Lock() - { - if (mSemaphore) { - xSemaphoreGive(mSemaphore); - } - } - - static esp_err_t init() - { - if (mSemaphore) { - return ESP_OK; - } - mSemaphore = xSemaphoreCreateMutex(); - if (!mSemaphore) { - return ESP_ERR_NO_MEM; - } - return ESP_OK; - } - - static void uninit() - { - if (mSemaphore) { - vSemaphoreDelete(mSemaphore); - } - mSemaphore = nullptr; - } - - static SemaphoreHandle_t mSemaphore; -}; + public: + Lock(); + ~Lock(); + static esp_err_t init(); + static void uninit(); +#ifndef LINUX_TARGET + private: + static _lock_t mSemaphore; +#endif + }; } // namespace nvs - -#endif // LINUX_TARGET diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index 327232e321..4fd501094c 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -210,7 +210,6 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount) if (mNamespaceUsage.set(255, true) != ESP_OK) { return ESP_FAIL; } - mState = StorageState::ACTIVE; // Populate list of multi-page index entries. TBlobIndexList blobIdxList; @@ -229,6 +228,8 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount) // Purge the blob index list blobIdxList.clearAndFreeNodes(); + mState = StorageState::ACTIVE; + #ifdef DEBUG_STORAGE debugCheck(); #endif diff --git a/components/nvs_flash/test_nvs_host/Makefile b/components/nvs_flash/test_nvs_host/Makefile index be4e5c46b3..320723d4ad 100644 --- a/components/nvs_flash/test_nvs_host/Makefile +++ b/components/nvs_flash/test_nvs_host/Makefile @@ -15,6 +15,7 @@ SOURCE_FILES = \ nvs_partition.cpp \ nvs_encrypted_partition.cpp \ nvs_cxx_api.cpp \ + nvs_platform.cpp \ ) \ spi_flash_emulation.cpp \ test_compressed_enum_table.cpp \ diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index e69cb8cbee..2fe8ad1e6c 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -718,7 +718,6 @@ components/nvs_flash/src/nvs_pagemanager.hpp components/nvs_flash/src/nvs_partition.cpp components/nvs_flash/src/nvs_partition_lookup.cpp components/nvs_flash/src/nvs_partition_lookup.hpp -components/nvs_flash/src/nvs_platform.hpp components/nvs_flash/src/nvs_test_api.h components/nvs_flash/src/nvs_types.cpp components/nvs_flash/src/partition.hpp From 2acac6b848f2c8b000f05733fca1c2aaf5ee5f0b Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Sat, 24 Feb 2024 01:13:35 +0100 Subject: [PATCH 6/7] fix(nvs): Adopted CMakeLists for host test if IDF v5.1 --- components/nvs_flash/CMakeLists.txt | 11 ++++++++--- components/spi_flash/CMakeLists.txt | 2 ++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/components/nvs_flash/CMakeLists.txt b/components/nvs_flash/CMakeLists.txt index 0f1dde708e..df4b312fd8 100644 --- a/components/nvs_flash/CMakeLists.txt +++ b/components/nvs_flash/CMakeLists.txt @@ -1,5 +1,11 @@ idf_build_get_property(target IDF_TARGET) +if(${target} STREQUAL "linux") +# omit newlib for linux +else() + list(APPEND priv_requires "newlib") +endif() + set(srcs "src/nvs_api.cpp" "src/nvs_cxx_api.cpp" "src/nvs_item_hash_list.cpp" @@ -15,10 +21,9 @@ set(srcs "src/nvs_api.cpp" "src/nvs_platform.cpp") idf_component_register(SRCS "${srcs}" - REQUIRES "esp_partition" - PRIV_REQUIRES spi_flash newlib + REQUIRES "esp_partition" "spi_flash" + PRIV_REQUIRES "${priv_requires}" INCLUDE_DIRS "include" - "../spi_flash/include" PRIV_INCLUDE_DIRS "private_include") # If we use the linux target, we need to redirect the crc functions to the linux diff --git a/components/spi_flash/CMakeLists.txt b/components/spi_flash/CMakeLists.txt index e6242bd07f..0baab36924 100644 --- a/components/spi_flash/CMakeLists.txt +++ b/components/spi_flash/CMakeLists.txt @@ -1,5 +1,7 @@ idf_build_get_property(target IDF_TARGET) if(${target} STREQUAL "linux") + idf_component_register(INCLUDE_DIRS include + PRIV_INCLUDE_DIRS include/spi_flash) return() endif() From c3f335817b7b7602dd6a4812823b2c7dc12e03fe Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Thu, 7 Mar 2024 11:30:35 +0100 Subject: [PATCH 7/7] fix(nvs): Fixed Page::findItem performance degradation caused by wrong condition before hash map use The condition enabling use of hash map when page is searched for Item was modified to correct the bug introduced by commit addressing delete of any BLOB_INDEX Items. This correction returns the performance of findItem to the state before previous change. --- components/nvs_flash/src/nvs_page.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index dd5f81eb6b..23086c30f2 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -881,7 +881,7 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si // For BLOB_DATA, we may need to search for all chunk indexes, so the hash list won't help // mHashIndex caluclates hash from nsIndex, key, chunkIdx // We may not use mHashList if datatype is BLOB_DATA and chunkIdx is CHUNK_ANY as CHUNK_ANY is used by BLOB_INDEX - if (nsIndex != NS_ANY && key != NULL && (datatype == ItemType::BLOB_DATA && chunkIdx != CHUNK_ANY)) { + if (nsIndex != NS_ANY && key != NULL && (datatype != ItemType::BLOB_DATA || chunkIdx != CHUNK_ANY)) { size_t cachedIndex = mHashList.find(start, Item(nsIndex, datatype, 0, key, chunkIdx)); if (cachedIndex < ENTRY_COUNT) { start = cachedIndex;