From 4746554954fef059eb5240166feca4f9870d38e3 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 11 Dec 2023 23:04:35 +0800 Subject: [PATCH 1/6] 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 194bfe2b0c..4c6ee0816b 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -512,6 +512,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; @@ -520,11 +525,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 2fc1fabceea6d9ce761084f8ce69058995e7d96f Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Tue, 23 Jan 2024 17:09:18 +0100 Subject: [PATCH 2/6] 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 4c6ee0816b..83d715a81b 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 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 7c19d6fb9a..3ea642e773 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; @@ -144,6 +147,8 @@ protected: esp_err_t populateBlobIndices(TBlobIndexList&); + void eraseMismatchedBlobIndexes(TBlobIndexList&); + void eraseOrphanDataBlobs(TBlobIndexList&); void fillEntryInfo(Item &item, nvs_entry_info_t &info); From 7938bbf3c070aad8bb7d90eef672edc016b164f7 Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Thu, 1 Feb 2024 13:45:59 +0100 Subject: [PATCH 3/6] 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 5fcf39c965..f6ae0ed2f4 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -879,7 +879,10 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si end = ENTRY_COUNT; } - if (nsIndex != NS_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; @@ -934,6 +937,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 e51277fbc714aa1c879509af643e9f3ab41a2684 Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Thu, 1 Feb 2024 13:55:29 +0100 Subject: [PATCH 4/6] 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 83d715a81b..3ce7e1be60 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -693,34 +693,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 c4eaf865161706bdd5fa62a98ec72688f65797fe Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Wed, 17 Jan 2024 10:44:50 +0100 Subject: [PATCH 5/6] 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 d54703ce9b..270f37ce49 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 d349aea466..effed329dd 100644 --- a/components/nvs_flash/src/nvs_api.cpp +++ b/components/nvs_flash/src/nvs_api.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -51,10 +51,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; @@ -268,6 +264,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 3ce7e1be60..c1fcc3c9c4 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 0ac5df21fc..044a39e72a 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 c01b3b48f7..8cabaeb70f 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -583,7 +583,6 @@ components/nvs_flash/src/nvs_item_hash_list.cpp components/nvs_flash/src/nvs_pagemanager.hpp 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/test_nvs_host/main.cpp From 36092067a02aefb618c05867cb5daec19b1c1205 Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Thu, 7 Mar 2024 11:30:35 +0100 Subject: [PATCH 6/6] 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 f6ae0ed2f4..c06ea202df 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -882,7 +882,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;