alex/esp-idf
alex/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2024-09-19 14:26:01 -04:00
Code Issues Actions 3 Packages Projects Releases Wiki Activity

[nvs_flash]: Entries with state == 1 don't crash

Browse Source
This commit is contained in:
Jakob Hasse 2021-07-27 17:58:52 +08:00
parent 36cd038526
commit de6b54de66
8 changed files with 100 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
components/nvs_flash/host_test/fixtures/test_fixtures.hpp
View File

@ -122,6 +122,9 @@ struct PartitionMockFixture {
const char *partition_name = NVS_DEFAULT_PART_NAME)
: part_mock(start_sector * SPI_FLASH_SEC_SIZE, sector_size * SPI_FLASH_SEC_SIZE) {
std::fill_n(raw_header, sizeof(raw_header)/sizeof(raw_header[0]), UINT8_MAX);
// This resets the mocks and prevents meeting accidental expectations from previous tests.
Mockesp_partition_Init();
}
~PartitionMockFixture() { }
@ -151,7 +154,7 @@ struct NVSPageFixture : public PartitionMockFixture {
nvs::Page page;
};
struct NVSValidPageFixture : public PartitionMockFixture {
struct NVSValidPageFlashFixture : public PartitionMockFixture {
const static uint8_t NS_INDEX = 1;
// valid header
@ -164,7 +167,7 @@ struct NVSValidPageFixture : public PartitionMockFixture {
uint8_t value_entry [32];
NVSValidPageFixture(uint32_t start_sector = 0,
NVSValidPageFlashFixture(uint32_t start_sector = 0,
uint32_t sector_size = 1,
const char *partition_name = NVS_DEFAULT_PART_NAME)
: PartitionMockFixture(start_sector, sector_size, partition_name),
@ -173,8 +176,7 @@ struct NVSValidPageFixture : public PartitionMockFixture {
ns_entry {0x00, 0x01, 0x01, 0xff, 0x68, 0xc5, 0x3f, 0x0b, 't', 'e', 's', 't', '_', 'n', 's', '\0',
'\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', 1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
value_entry {0x01, 0x01, 0x01, 0xff, 0x3d, 0xf3, 0x99, 0xe5, 't', 'e', 's', 't', '_', 'v', 'a', 'l',
'u', 'e', '\0', '\0', '\0', '\0', '\0', '\0', 47, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
page()
'u', 'e', '\0', '\0', '\0', '\0', '\0', '\0', 47, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}
{
std::fill_n(raw_entry_table, sizeof(raw_entry_table)/sizeof(raw_entry_table[0]), 0);
raw_entry_table[0] = 0xfa;
@ -202,7 +204,15 @@ struct NVSValidPageFixture : public PartitionMockFixture {
// read normal entry second time during duplicated entry check
esp_partition_read_ExpectAnyArgsAndReturn(ESP_OK);
esp_partition_read_ReturnArrayThruPtr_dst(value_entry, 32);
}
};
struct NVSValidPageFixture : public NVSValidPageFlashFixture {
NVSValidPageFixture(uint32_t start_sector = 0,
uint32_t sector_size = 1,
const char *partition_name = NVS_DEFAULT_PART_NAME)
: NVSValidPageFlashFixture(start_sector, sector_size, partition_name), page()
{
if (page.load(&part_mock, start_sector) != ESP_OK) throw FixtureException("couldn't setup page");
}
@ -392,9 +402,6 @@ struct NVSFullPageFixture : public PartitionMockFixture {
'u', 'e', '\0', '\0', '\0', '\0', '\0', '\0', 47, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
page()
{
std::fill_n(raw_entry_table, sizeof(raw_entry_table)/sizeof(raw_entry_table[0]), 0);
raw_entry_table[0] = 0xfa;
// entry_table with all elements deleted except the namespace entry written and the last entry free
std::fill_n(raw_entry_table, sizeof(raw_entry_table)/sizeof(raw_entry_table[0]), 0);
raw_entry_table[0] = 0x0a;

61
components/nvs_flash/host_test/nvs_page_test/main/nvs_page_test.cpp
View File

@ -30,6 +30,8 @@ void test_Page_load_reading_header_fails()
TEST_ASSERT_EQUAL(Page::PageState::INVALID, page.state());
TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, page.load(&mock, 0));
Mockesp_partition_Verify();
}
void test_Page_load_reading_data_fails()
@ -44,6 +46,8 @@ void test_Page_load_reading_data_fails()
TEST_ASSERT_EQUAL(Page::PageState::INVALID, page.state());
TEST_ASSERT_EQUAL(ESP_FAIL, page.load(&mock, 0));
Mockesp_partition_Verify();
}
void test_Page_load__uninitialized_page_has_0xfe()
@ -62,6 +66,8 @@ void test_Page_load__uninitialized_page_has_0xfe()
TEST_ASSERT_EQUAL(ESP_OK, page.load(&fix.part_mock, 0));
TEST_ASSERT_EQUAL(Page::PageState::CORRUPT, page.state());
Mockesp_partition_Verify();
}
void test_Page_load__initialized_corrupt_header()
@ -79,6 +85,60 @@ void test_Page_load__initialized_corrupt_header()
TEST_ASSERT_EQUAL(ESP_OK, page.load(&fix.part_mock, 0));
TEST_ASSERT_EQUAL(Page::PageState::CORRUPT, page.state());
Mockesp_partition_Verify();
}
void test_Page_load__corrupt_entry_table()
{
PartitionMockFixture fix;
// valid header
uint8_t raw_header_valid [32] = {0xfe, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc2, 0x16, 0xdd, 0xdc};
// entry table with one entry
uint8_t raw_entry_table [32];
uint8_t ns_entry [32] = {0x00, 0x01, 0x01, 0xff, 0x68, 0xc5, 0x3f, 0x0b, 't', 'e', 's', 't', '_', 'n', 's', '\0',
'\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', 1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
uint8_t raw_header[4] = {0xff, 0xff, 0xff, 0xff};
std::fill_n(raw_entry_table, sizeof(raw_entry_table)/sizeof(raw_entry_table[0]), 0);
raw_entry_table[0] = 0xfa;
// read page header
esp_partition_read_raw_ExpectAnyArgsAndReturn(ESP_OK);
esp_partition_read_raw_ReturnArrayThruPtr_dst(raw_header_valid, 32);
// read entry table
esp_partition_read_raw_ExpectAnyArgsAndReturn(ESP_OK);
esp_partition_read_raw_ReturnArrayThruPtr_dst(raw_entry_table, 32);
// read next free entry's header
esp_partition_read_raw_ExpectAnyArgsAndReturn(ESP_OK);
esp_partition_read_raw_ReturnArrayThruPtr_dst(raw_header, 4);
// read namespace entry
esp_partition_read_ExpectAnyArgsAndReturn(ESP_OK);
esp_partition_read_ReturnArrayThruPtr_dst(ns_entry, 32);
// we expect a raw word write from the partition in order to change the entry bits to erased (0)
esp_partition_write_raw_ExpectAndReturn(&fix.part_mock.partition, 32, nullptr, 4, ESP_OK);
esp_partition_write_raw_IgnoreArg_src();
// corrupt entry table as well as crc of corresponding item
raw_entry_table[0] = 0xf6;
Page page;
// Page::load() should return ESP_OK, but state has to be corrupt
TEST_ASSERT_EQUAL(ESP_OK, page.load(&fix.part_mock, 0));
TEST_ASSERT_EQUAL(Page::PageState::ACTIVE, page.state());
TEST_ASSERT_EQUAL(1, page.getUsedEntryCount());
Mockesp_partition_Verify();
}
void test_Page_load_success()
@ -886,6 +946,7 @@ int main(int argc, char **argv)
RUN_TEST(test_Page_load_reading_data_fails);
RUN_TEST(test_Page_load__uninitialized_page_has_0xfe);
RUN_TEST(test_Page_load__initialized_corrupt_header);
RUN_TEST(test_Page_load__corrupt_entry_table);
RUN_TEST(test_Page_load_success);
RUN_TEST(test_Page_load_full_page);
RUN_TEST(test_Page_load__seq_number_0);

12
components/nvs_flash/src/nvs_item_hash_list.cpp
View File

@ -65,7 +65,7 @@ esp_err_t HashList::insert(const Item& item, size_t index)
return ESP_OK;
}
void HashList::erase(size_t index, bool itemShouldExist)
bool HashList::erase(size_t index)
{
for (auto it = mBlockList.begin(); it != mBlockList.end();) {
bool haveEntries = false;
@ -81,7 +81,7 @@ void HashList::erase(size_t index, bool itemShouldExist)
}
if (haveEntries && foundIndex) {
/* item was found, and HashListBlock still has some items */
return;
return true;
}
}
/* no items left in HashListBlock, can remove */
@ -95,12 +95,12 @@ void HashList::erase(size_t index, bool itemShouldExist)
}
if (foundIndex) {
/* item was found and empty HashListBlock was removed */
return;
return true;
}
}
if (itemShouldExist) {
assert(false && "item should have been present in cache");
}
// item hasn't been present in cache");
return false;
}
size_t HashList::find(size_t start, const Item& item)

2
components/nvs_flash/src/nvs_item_hash_list.hpp
View File

@ -29,7 +29,7 @@ public:
~HashList();
esp_err_t insert(const Item& item, size_t index);
void erase(const size_t index, bool itemShouldExist=true);
bool erase(const size_t index);
size_t find(size_t start, const Item& item);
void clear();

15
components/nvs_flash/src/nvs_page.cpp
View File

@ -393,8 +393,9 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, ui
esp_err_t Page::eraseEntryAndSpan(size_t index)
{
uint32_t seq_num;
getSeqNumber(seq_num);
auto state = mEntryTable.get(index);
assert(state == EntryState::WRITTEN || state == EntryState::EMPTY);
size_t span = 1;
if (state == EntryState::WRITTEN) {
@ -404,7 +405,7 @@ esp_err_t Page::eraseEntryAndSpan(size_t index)
return rc;
}
if (item.calculateCrc32() != item.crc32) {
mHashList.erase(index, false);
mHashList.erase(index);
rc = alterEntryState(index, EntryState::ERASED);
--mUsedEntryCount;
++mErasedEntryCount;
@ -601,6 +602,16 @@ esp_err_t Page::mLoadEntryTable()
continue;
}
if (mEntryTable.get(i) == EntryState::ILLEGAL) {
lastItemIndex = INVALID_ENTRY;
auto err = eraseEntryAndSpan(i);
if (err != ESP_OK) {
mState = PageState::INVALID;
return err;
}
continue;
}
lastItemIndex = i;
auto err = readEntry(i, item);

1
components/nvs_flash/src/nvs_page.hpp
View File

@ -175,6 +175,7 @@ protected:
EMPTY = 0x3, // 0b11, default state after flash erase
WRITTEN = EMPTY & ~ESB_WRITTEN, // entry was written
ERASED = WRITTEN & ~ESB_ERASED, // entry was written and then erased
ILLEGAL = 0x1, // only possible if flash is inconsistent
INVALID = 0x4 // entry is in inconsistent state (write started but ESB_WRITTEN has not been set yet)
};

5
components/nvs_flash/test_nvs_host/test_nvs.cpp
View File

@ -313,7 +313,8 @@ TEST_CASE("HashList is cleaned up as soon as items are erased", "[nvs]")
INFO("Added " << count << " items, " << hashlist.getBlockCount() << " blocks");
// Remove them in reverse order
for (size_t i = count; i > 0; --i) {
hashlist.erase(i - 1, true);
// Make sure that the element existed before it's erased
CHECK(hashlist.erase(i - 1) == true);
}
CHECK(hashlist.getBlockCount() == 0);
// Add again
@ -326,7 +327,7 @@ TEST_CASE("HashList is cleaned up as soon as items are erased", "[nvs]")
INFO("Added " << count << " items, " << hashlist.getBlockCount() << " blocks");
// Remove them in the same order
for (size_t i = 0; i < count; ++i) {
hashlist.erase(i, true);
CHECK(hashlist.erase(i) == true);
}
CHECK(hashlist.getBlockCount() == 0);
}

1
components/spi_flash/mock/mock_config.yaml
View File

@ -5,3 +5,4 @@
- return_thru_ptr
- array
- callback
- ignore_arg