From 69b0919904f51015f604e06077dc341cedf20298 Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Fri, 22 Nov 2019 12:07:56 +0800 Subject: [PATCH 1/3] NVS: BUGFIX non-matching type iterator works Closes IDFGH-2229 --- components/nvs_flash/src/nvs_page.cpp | 7 ++-- .../nvs_flash/test_nvs_host/test_nvs.cpp | 35 ++++++++++++++++++- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index e1fb4dbe0..72aee0909 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -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; } diff --git a/components/nvs_flash/test_nvs_host/test_nvs.cpp b/components/nvs_flash/test_nvs_host/test_nvs.cpp index f78440a0d..f62c2d850 100644 --- a/components/nvs_flash/test_nvs_host/test_nvs.cpp +++ b/components/nvs_flash/test_nvs_host/test_nvs.cpp @@ -772,6 +772,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 +1988,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); } From 92b10b4ba37ea93d756a0bcb0e279fc178cb3879 Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Tue, 26 Nov 2019 14:43:30 +0800 Subject: [PATCH 2/3] NVS: bugfix nvs_set_str/blob checks write mode --- components/nvs_flash/src/nvs_api.cpp | 36 +++++++++++-------- .../nvs_flash/test_nvs_host/test_nvs.cpp | 30 +++++++++++++++- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp index 19f3ba3e1..14cf367fb 100644 --- a/components/nvs_flash/src/nvs_api.cpp +++ b/components/nvs_flash/src/nvs_api.cpp @@ -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); -} \ No newline at end of file +} diff --git a/components/nvs_flash/test_nvs_host/test_nvs.cpp b/components/nvs_flash/test_nvs_host/test_nvs.cpp index f62c2d850..4b2181064 100644 --- a/components/nvs_flash/test_nvs_host/test_nvs.cpp +++ b/components/nvs_flash/test_nvs_host/test_nvs.cpp @@ -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); @@ -2087,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; From daa2178f30995a0ad40f1d569d927839d32ced66 Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Thu, 19 Dec 2019 11:53:52 +0800 Subject: [PATCH 3/3] WIFI: added log for wifi test, increased timeout --- components/esp_wifi/test/test_wifi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/esp_wifi/test/test_wifi.c b/components/esp_wifi/test/test_wifi.c index 08996d9de..24144f7fd 100644 --- a/components/esp_wifi/test/test_wifi.c +++ b/components/esp_wifi/test/test_wifi.c @@ -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); }