espressif/esp-idf
1
0
Fork 1
mirror of https://github.com/espressif/esp-idf.git synced 2025-10-02 10:00:57 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'bugfix/nvs_blob_overwrite' into 'master'

Browse Source 
Bugfix of old NVS Blobs handling when legacy compatibility option is set

See merge request espressif/esp-idf!41741
This commit is contained in:
Radek Tandler
2025-09-18 10:45:42 +02:00
parent 4680b976f4 96f4f78054
commit 553d1034b0
3 changed files with 62 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
components/nvs_flash/host_test/nvs_host_test/main/test_nvs.cpp
View File

@@ -608,7 +608,7 @@ TEST_CASE("nvs api tests", "[nvs]")
nvs_handle_t handle_2; nvs_handle_t handle_2;
TEST_ESP_OK(nvs_open("namespace2", NVS_READWRITE, &handle_2)); TEST_ESP_OK(nvs_open("namespace2", NVS_READWRITE, &handle_2));
TEST_ESP_OK(nvs_set_i32(handle_2, "foo", 0x3456789a)); TEST_ESP_OK(nvs_set_i32(handle_2, "foo", 0x3456789a));
const char *str = "value 0123456789abcdef0123456789abcdef"; char str[] = "value 0123456789abcdef0123456789abcdef";
TEST_ESP_OK(nvs_set_str(handle_2, "key", str)); TEST_ESP_OK(nvs_set_str(handle_2, "key", str));
int32_t v1; int32_t v1;
@@ -619,7 +619,7 @@ TEST_CASE("nvs api tests", "[nvs]")
TEST_ESP_OK(nvs_get_i32(handle_2, "foo", &v2)); TEST_ESP_OK(nvs_get_i32(handle_2, "foo", &v2));
CHECK(0x3456789a == v2); CHECK(0x3456789a == v2);
char buf[strlen(str) + 1]; char buf[sizeof(str)];
size_t buf_len = sizeof(buf); size_t buf_len = sizeof(buf);
size_t buf_len_needed; size_t buf_len_needed;
@@ -1270,13 +1270,15 @@ public:
} else { } else {
blobBufLen = largeBlobLen ; blobBufLen = largeBlobLen ;
} }
uint8_t buf[blobBufLen]; uint8_t* buf = new uint8_t[blobBufLen];
memset(buf, 0, blobBufLen); memset(buf, 0, blobBufLen);
size_t len = blobBufLen; size_t len = blobBufLen;
auto err = nvs_get_blob(handle, keys[index], buf, &len); auto err = nvs_get_blob(handle, keys[index], buf, &len);
auto eval_result = evaluate(delayCount, err, types[index], written[index], potentially_written[index], buf, values[index], future_values[index], blobBufLen);
delete [] buf;
REQUIRE(evaluate(delayCount, err, types[index], written[index], potentially_written[index], buf, values[index], future_values[index], blobBufLen) == true); REQUIRE(eval_result == true);
break; break;
} }
@@ -1371,7 +1373,7 @@ public:
} else { } else {
blobBufLen = largeBlobLen ; blobBufLen = largeBlobLen ;
} }
uint8_t buf[blobBufLen]; uint8_t* buf = new uint8_t[blobBufLen];
memset(buf, 0, blobBufLen); memset(buf, 0, blobBufLen);
size_t blobLen = gen() % blobBufLen; size_t blobLen = gen() % blobBufLen;
std::generate_n(buf, blobLen, [&]() -> uint8_t { std::generate_n(buf, blobLen, [&]() -> uint8_t {
@@ -1386,16 +1388,19 @@ public:
if (err == ESP_ERR_FLASH_OP_FAIL) { if (err == ESP_ERR_FLASH_OP_FAIL) {
// mark potentially written // mark potentially written
potentially_written[index] = true; potentially_written[index] = true;
delete [] buf;
return err; return err;
} }
if (err == ESP_ERR_NVS_REMOVE_FAILED) { if (err == ESP_ERR_NVS_REMOVE_FAILED) {
written[index] = true; written[index] = true;
memcpy(reinterpret_cast<uint8_t *>(values[index]), buf, blobBufLen); memcpy(reinterpret_cast<uint8_t *>(values[index]), buf, blobBufLen);
delete [] buf;
return ESP_ERR_FLASH_OP_FAIL; return ESP_ERR_FLASH_OP_FAIL;
} }
REQUIRE(err == ESP_OK); REQUIRE(err == ESP_OK);
written[index] = true; written[index] = true;
memcpy(reinterpret_cast<char *>(values[index]), buf, blobBufLen); memcpy(reinterpret_cast<char *>(values[index]), buf, blobBufLen);
delete [] buf;
break; break;
} }
@@ -2889,16 +2894,20 @@ TEST_CASE("inconsistent fields in item header with correct crc are handled for m
// initial values // initial values
uint32_t uval1 = 1; uint32_t uval1 = 1;
uint32_t uval2 = 2; uint32_t uval2 = 2;
uint8_t blob_data1[blob_key1_chunk_0_len + blob_key1_chunk_1_len]; size_t blob_data1_len = blob_key1_chunk_0_len + blob_key1_chunk_1_len;
uint8_t blob_data2[blob_key2_chunk_len]; size_t blob_data2_len = blob_key2_chunk_len;
uint8_t* blob_data1 = new uint8_t[blob_data1_len];
uint8_t* blob_data2 = new uint8_t[blob_data2_len];
// value buffers // value buffers
uint32_t read_u32_1; uint32_t read_u32_1;
uint32_t read_u32_2; uint32_t read_u32_2;
uint8_t read_blob1[sizeof(blob_data1)];
uint8_t read_blob2[sizeof(blob_data2)]; size_t read_blob1_size = blob_data1_len;
size_t read_blob1_size = sizeof(read_blob1); size_t read_blob2_size = blob_data2_len;
size_t read_blob2_size = sizeof(read_blob2); uint8_t* read_blob1 = new uint8_t[read_blob1_size];
uint8_t* read_blob2 = new uint8_t[read_blob2_size];
// Skip one page, 2 entries of page header and 3 entries of valueblob1's BLOB_DATA entries // Skip one page, 2 entries of page header and 3 entries of valueblob1's BLOB_DATA entries
const size_t blob_index_offset = 4096 + 32 * 2 + 32 * 3; const size_t blob_index_offset = 4096 + 32 * 2 + 32 * 3;
@@ -2921,10 +2930,10 @@ TEST_CASE("inconsistent fields in item header with correct crc are handled for m
// initialize buffers // initialize buffers
read_u32_1 = 0; read_u32_1 = 0;
read_u32_2 = 0; read_u32_2 = 0;
memset(blob_data1, 0x55, sizeof(blob_data1)); memset(blob_data1, 0x55, blob_data1_len);
memset(blob_data2, 0xaa, sizeof(blob_data2)); memset(blob_data2, 0xaa, blob_data2_len);
memset(read_blob1, 0, sizeof(read_blob1)); memset(read_blob1, 0, read_blob1_size);
memset(read_blob2, 0, sizeof(read_blob2)); memset(read_blob2, 0, read_blob2_size);
// Write initial data to the nvs partition // Write initial data to the nvs partition
{ {
@@ -2933,8 +2942,8 @@ TEST_CASE("inconsistent fields in item header with correct crc are handled for m
nvs_handle_t handle; nvs_handle_t handle;
TEST_ESP_OK(nvs_open("test", NVS_READWRITE, &handle)); TEST_ESP_OK(nvs_open("test", NVS_READWRITE, &handle));
TEST_ESP_OK(nvs_set_u32(handle, ukey1, uval1)); TEST_ESP_OK(nvs_set_u32(handle, ukey1, uval1));
TEST_ESP_OK(nvs_set_blob(handle, blob_key1, blob_data1, sizeof(blob_data1))); TEST_ESP_OK(nvs_set_blob(handle, blob_key1, blob_data1, blob_data1_len));
TEST_ESP_OK(nvs_set_blob(handle, blob_key2, blob_data2, sizeof(blob_data2))); TEST_ESP_OK(nvs_set_blob(handle, blob_key2, blob_data2, blob_data2_len));
TEST_ESP_OK(nvs_set_u32(handle, ukey2, uval2)); TEST_ESP_OK(nvs_set_u32(handle, ukey2, uval2));
nvs_close(handle); nvs_close(handle);
@@ -3067,8 +3076,8 @@ TEST_CASE("inconsistent fields in item header with correct crc are handled for m
nvs_handle_t handle; nvs_handle_t handle;
TEST_ESP_OK(nvs_open("test", NVS_READWRITE, &handle)); TEST_ESP_OK(nvs_open("test", NVS_READWRITE, &handle));
TEST_ESP_OK(nvs_get_u32(handle, ukey1, &read_u32_1)); TEST_ESP_OK(nvs_get_u32(handle, ukey1, &read_u32_1));
TEST_ESP_ERR(nvs_get_blob(handle, blob_key1, &read_blob1, &read_blob1_size), ESP_ERR_NVS_NOT_FOUND); TEST_ESP_ERR(nvs_get_blob(handle, blob_key1, read_blob1, &read_blob1_size), ESP_ERR_NVS_NOT_FOUND);
TEST_ESP_OK(nvs_get_blob(handle, blob_key2, &read_blob2, &read_blob2_size)); TEST_ESP_OK(nvs_get_blob(handle, blob_key2, read_blob2, &read_blob2_size));
TEST_ESP_OK(nvs_get_u32(handle, ukey2, &read_u32_2)); TEST_ESP_OK(nvs_get_u32(handle, ukey2, &read_u32_2));
nvs_close(handle); nvs_close(handle);
TEST_ESP_OK(nvs_flash_deinit_partition(f.part()->get_partition_name())); TEST_ESP_OK(nvs_flash_deinit_partition(f.part()->get_partition_name()));
@@ -3076,7 +3085,8 @@ TEST_CASE("inconsistent fields in item header with correct crc are handled for m
// validate the data // validate the data
CHECK(read_u32_1 == uval1); CHECK(read_u32_1 == uval1);
CHECK(read_u32_2 == uval2); CHECK(read_u32_2 == uval2);
CHECK(memcmp(read_blob2, blob_data2, sizeof(blob_data2)) == 0); CHECK(read_blob2_size == blob_data2_len);
CHECK(memcmp(read_blob2, blob_data2, read_blob2_size) == 0);
} }
} }
@@ -3808,6 +3818,8 @@ TEST_CASE("nvs multiple write with same key but different types", "[nvs]")
TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME)); TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME));
} }
#ifndef CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY
// Following test case is not valid for new behavior not leading to multiple active values under the same key
TEST_CASE("nvs multiple write with same key blob and string involved", "[nvs]") TEST_CASE("nvs multiple write with same key blob and string involved", "[nvs]")
{ {
PartitionEmulationFixture f(0, 10); PartitionEmulationFixture f(0, 10);
@@ -3883,6 +3895,7 @@ TEST_CASE("nvs multiple write with same key blob and string involved", "[nvs]")
TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME)); TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME));
} }
#endif // !CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY
TEST_CASE("nvs find key tests", "[nvs]") TEST_CASE("nvs find key tests", "[nvs]")
{ {

8
components/nvs_flash/host_test/nvs_host_test/pytest_nvs_host_linux.py
View File

@@ -6,6 +6,14 @@ from pytest_embedded_idf.utils import idf_parametrize
@pytest.mark.host_test @pytest.mark.host_test
@pytest.mark.parametrize(
'config',
[
'default_set_key',
'legacy_set_key',
],
indirect=True,
)
@idf_parametrize('target', ['linux'], indirect=['target']) @idf_parametrize('target', ['linux'], indirect=['target'])
def test_nvs_host_linux(dut: Dut) -> None: def test_nvs_host_linux(dut: Dut) -> None:
dut.expect_exact('All tests passed', timeout=60) dut.expect_exact('All tests passed', timeout=60)

21
components/nvs_flash/src/nvs_storage.cpp
View File

@@ -407,6 +407,27 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key
if(err == ESP_OK && findPage != nullptr) { if(err == ESP_OK && findPage != nullptr) {
matchedTypePageFound = true; matchedTypePageFound = true;
} }
#ifdef CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY
// In legacy mode, we also try to find the item as BLOB if not found as BLOB_INDEX.
// In this mode, it is possible to have multiple active values under the same (logical) key.
// For BLOBs (which may have different physical representations in V1 it is BLOB, in V2 it is BLOB_INDEX) it in turn means
// that we have to check both datatypes to find the old value.
// The general case for compatibility flag disabled is below and handles all datatypes including BLOB.
// To save some cycles, we do not compile both findItem calls in this case.
if(err == ESP_ERR_NVS_NOT_FOUND) {
// If not found as BLOB_INDEX, try to find it as BLOB (legacy support).
err = findItem(nsIndex, ItemType::BLOB, key, findPage, item, Page::CHUNK_ANY, VerOffset::VER_ANY, &itemIndex);
if(err == ESP_OK && findPage != nullptr) {
matchedTypePageFound = false; // datatype does not match, we cannot extract chunkStart from the item
// keep the sequence number of the page where the item was found for later check of relocation
err = findPage->getSeqNumber(findPageSeqNumber);
if(err != ESP_OK) {
return err;
}
}
}
#endif
} else { } else {
// Handle all other data types than BLOB // Handle all other data types than BLOB
err = findItem(nsIndex, datatype, key, findPage, item, Page::CHUNK_ANY, VerOffset::VER_ANY, &itemIndex); err = findItem(nsIndex, datatype, key, findPage, item, Page::CHUNK_ANY, VerOffset::VER_ANY, &itemIndex);