Merge branch 'bugfix/backport_nvs_iterator_fix' into 'release/v4.0'

NVS: iterator corrupting entries (backport v4.0)

See merge request espressif/esp-idf!7071
This commit is contained in:
Angus Gratton 2020-01-03 13:07:20 +08:00
commit 16e102199a
4 changed files with 90 additions and 21 deletions

View file

@ -260,7 +260,8 @@ static void wifi_connect_by_bssid(uint8_t *bssid)
TEST_ESP_OK(esp_wifi_set_config(WIFI_IF_STA, &w_config));
TEST_ESP_OK(esp_wifi_connect());
bits = xEventGroupWaitBits(wifi_events, GOT_IP_EVENT, 1, 0, 5000/portTICK_RATE_MS);
ESP_LOGI(TAG, "called esp_wifi_connect()");
bits = xEventGroupWaitBits(wifi_events, GOT_IP_EVENT, 1, 0, 7000/portTICK_RATE_MS);
TEST_ASSERT(bits == GOT_IP_EVENT);
}

View file

@ -416,6 +416,9 @@ extern "C" esp_err_t nvs_set_str(nvs_handle_t handle, const char* key, const cha
if (err != ESP_OK) {
return err;
}
if (entry.mReadOnly) {
return ESP_ERR_NVS_READ_ONLY;
}
return entry.mStoragePtr->writeItem(entry.mNsIndex, nvs::ItemType::SZ, key, value, strlen(value) + 1);
}
@ -428,6 +431,9 @@ extern "C" esp_err_t nvs_set_blob(nvs_handle_t handle, const char* key, const vo
if (err != ESP_OK) {
return err;
}
if (entry.mReadOnly) {
return ESP_ERR_NVS_READ_ONLY;
}
return entry.mStoragePtr->writeItem(entry.mNsIndex, nvs::ItemType::BLOB, key, value, length);
}
@ -587,7 +593,7 @@ extern "C" esp_err_t nvs_flash_generate_keys(const esp_partition_t* partition, n
}
err = spi_flash_write(partition->address, cfg->eky, NVS_KEY_SIZE);
if(err != ESP_OK) {
if(err != ESP_OK) {
return err;
}
@ -597,12 +603,12 @@ extern "C" esp_err_t nvs_flash_generate_keys(const esp_partition_t* partition, n
}
err = esp_partition_read(partition, 0, cfg->eky, NVS_KEY_SIZE);
if(err != ESP_OK) {
if(err != ESP_OK) {
return err;
}
err = esp_partition_read(partition, NVS_KEY_SIZE, cfg->tky, NVS_KEY_SIZE);
if(err != ESP_OK) {
if(err != ESP_OK) {
return err;
}
@ -614,10 +620,10 @@ extern "C" esp_err_t nvs_flash_generate_keys(const esp_partition_t* partition, n
memcpy(crc_wr, &crc_calc, 4);
err = esp_partition_write(partition, 2 * NVS_KEY_SIZE, crc_wr, sizeof(crc_wr));
if(err != ESP_OK) {
if(err != ESP_OK) {
return err;
}
return ESP_OK;
}
@ -628,7 +634,7 @@ extern "C" esp_err_t nvs_flash_read_security_cfg(const esp_partition_t* partitio
uint32_t crc_raw, crc_read, crc_calc;
auto check_if_initialized = [](uint8_t* eky, uint8_t* tky, uint32_t crc) {
uint8_t cnt = 0;
uint8_t cnt = 0;
while(cnt < NVS_KEY_SIZE && eky[cnt] == 0xff && tky[cnt] == 0xff) cnt++;
if(cnt == NVS_KEY_SIZE && crc == 0xffffffff) {
@ -656,25 +662,25 @@ extern "C" esp_err_t nvs_flash_read_security_cfg(const esp_partition_t* partitio
/* This is an uninitialized key partition*/
return ESP_ERR_NVS_KEYS_NOT_INITIALIZED;
}
err = esp_partition_read(partition, 0, cfg->eky, NVS_KEY_SIZE);
if(err != ESP_OK) {
if(err != ESP_OK) {
return err;
}
err = esp_partition_read(partition, NVS_KEY_SIZE, cfg->tky, NVS_KEY_SIZE);
if(err != ESP_OK) {
if(err != ESP_OK) {
return err;
}
err = esp_partition_read(partition, 2 * NVS_KEY_SIZE, &crc_read, 4);
if(err != ESP_OK) {
if(err != ESP_OK) {
return err;
}
crc_calc = crc32_le(0xffffffff, cfg->eky, NVS_KEY_SIZE);
crc_calc = crc32_le(crc_calc, cfg->tky, NVS_KEY_SIZE);
@ -750,4 +756,4 @@ extern "C" void nvs_entry_info(nvs_iterator_t it, nvs_entry_info_t *out_info)
extern "C" void nvs_release_iterator(nvs_iterator_t it)
{
free(it);
}
}

View file

@ -622,9 +622,9 @@ esp_err_t Page::mLoadEntryTable()
}
}
/* Note that logic for duplicate detections works fine even
* when old-format blob is present along with new-format blob-index
* for same key on active page. Since datatype is not used in hash calculation,
/* Note that logic for duplicate detections works fine even
* when old-format blob is present along with new-format blob-index
* for same key on active page. Since datatype is not used in hash calculation,
* old-format blob will be removed.*/
if (duplicateIndex < i) {
eraseEntryAndSpan(duplicateIndex);
@ -864,6 +864,7 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si
if (key == nullptr && nsIndex == NS_ANY && chunkIdx == CHUNK_ANY) {
continue; // continue for bruteforce search on blob indices.
}
itemIndex = i;
return ESP_ERR_NVS_TYPE_MISMATCH;
}

View file

@ -549,6 +549,34 @@ TEST_CASE("can erase items", "[nvs]")
CHECK(storage.readItem(3, "key00222", val) == ESP_ERR_NVS_NOT_FOUND);
}
TEST_CASE("readonly handle fails on writing", "[nvs]")
{
SpiFlashEmulator emu(10);
const char* str = "value 0123456789abcdef0123456789abcdef";
const uint8_t blob[8] = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7};
nvs_handle_t handle_1;
const uint32_t NVS_FLASH_SECTOR = 6;
const uint32_t NVS_FLASH_SECTOR_COUNT_MIN = 3;
emu.setBounds(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN);
TEST_ESP_OK(nvs_flash_init_custom(NVS_DEFAULT_PART_NAME, NVS_FLASH_SECTOR, NVS_FLASH_SECTOR_COUNT_MIN));
// first, creating namespace...
TEST_ESP_OK(nvs_open("ro_ns", NVS_READWRITE, &handle_1));
nvs_close(handle_1);
TEST_ESP_OK(nvs_open("ro_ns", NVS_READONLY, &handle_1));
TEST_ESP_ERR(nvs_set_i32(handle_1, "key", 47), ESP_ERR_NVS_READ_ONLY);
TEST_ESP_ERR(nvs_set_str(handle_1, "key", str), ESP_ERR_NVS_READ_ONLY);
TEST_ESP_ERR(nvs_set_blob(handle_1, "key", blob, 8), ESP_ERR_NVS_READ_ONLY);
nvs_close(handle_1);
// without deinit it affects "nvs api tests"
TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME));
}
TEST_CASE("nvs api tests", "[nvs]")
{
SpiFlashEmulator emu(10);
@ -772,6 +800,39 @@ TEST_CASE("nvs iterators tests", "[nvs]")
nvs_close(handle_2);
}
TEST_CASE("Iterator with not matching type iterates correctly", "[nvs]")
{
SpiFlashEmulator emu(5);
nvs_iterator_t it;
nvs_handle_t my_handle;
const char* NAMESPACE = "test_ns_4";
const uint32_t NVS_FLASH_SECTOR = 0;
const uint32_t NVS_FLASH_SECTOR_COUNT_MIN = 5;
emu.setBounds(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN);
for (uint16_t i = NVS_FLASH_SECTOR; i < NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN; ++i) {
spi_flash_erase_sector(i);
}
TEST_ESP_OK(nvs_flash_init_custom(NVS_DEFAULT_PART_NAME, NVS_FLASH_SECTOR, NVS_FLASH_SECTOR_COUNT_MIN));
// writing string to namespace (a type which spans multiple entries)
TEST_ESP_OK(nvs_open(NAMESPACE, NVS_READWRITE, &my_handle));
TEST_ESP_OK(nvs_set_str(my_handle, "test-string", "InitString0"));
TEST_ESP_OK(nvs_commit(my_handle));
nvs_close(my_handle);
it = nvs_entry_find(NVS_DEFAULT_PART_NAME, NAMESPACE, NVS_TYPE_I32);
CHECK(it == NULL);
// re-init to trigger cleaning up of broken items -> a corrupted string will be erased
nvs_flash_deinit();
TEST_ESP_OK(nvs_flash_init_custom(NVS_DEFAULT_PART_NAME, NVS_FLASH_SECTOR, NVS_FLASH_SECTOR_COUNT_MIN));
it = nvs_entry_find(NVS_DEFAULT_PART_NAME, NAMESPACE, NVS_TYPE_STR);
CHECK(it != NULL);
nvs_release_iterator(it);
}
TEST_CASE("wifi test", "[nvs]")
{
@ -1955,7 +2016,7 @@ TEST_CASE("Multi-page blob erased using nvs_erase_key should not be found when p
TEST_ESP_OK(nvs_open("Test", NVS_READWRITE, &handle));
TEST_ESP_OK(nvs_set_blob(handle, "abc", blob, blob_size));
TEST_ESP_OK(nvs_erase_key(handle, "abc"));
TEST_ESP_ERR(nvs_get_blob(handle, "abc", NULL, &read_size), ESP_ERR_NVS_NOT_FOUND);
TEST_ESP_ERR(nvs_get_blob(handle, "abc", NULL, &read_size), ESP_ERR_NVS_NOT_FOUND);
TEST_ESP_OK(nvs_commit(handle));
nvs_close(handle);
}
@ -2054,7 +2115,7 @@ TEST_CASE("Check that NVS supports old blob format without blob index", "[nvs]")
nvs_handle_t handle;
TEST_ESP_OK( nvs_flash_init_custom("test", 0, 2) );
TEST_ESP_OK( nvs_open_from_partition("test", "dummyNamespace", NVS_READONLY, &handle));
TEST_ESP_OK( nvs_open_from_partition("test", "dummyNamespace", NVS_READWRITE, &handle));
char buf[64] = {0};
size_t buflen = 64;