Merge pull request #11918 from ARMmbed/IOTSTOR-978

IOTSTOR-978: Bugfixes to TDBStore and SecureStore

Author: 0xc0170

features/storage/TESTS/kvstore/general_tests_phase_1/main.cpp

@@ -68,13 +68,18 @@ static int kv_setup = TDBStoreSet;
 
 static const int heap_alloc_threshold_size = 4096;
 
+static inline uint32_t align_up(uint32_t val, uint32_t size)
+{
+    return (((val - 1) / size) + 1) * size;
+}
+
 /*----------------initialization------------------*/
 
 //init the blockdevice
 static void kvstore_init()
 {
     int res;
-    size_t erase_size, ul_bd_size, rbp_bd_size;
+    size_t program_size, erase_size, ul_bd_size, rbp_bd_size;
     BlockDevice *sec_bd;
 
     res = bd->init();
@@ -109,10 +114,17 @@ static void kvstore_init()
             flash_bd = new FlashSimBlockDevice(bd);
             sec_bd = flash_bd;
         }
+        res = sec_bd->init();
+        TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
+
+        program_size  = sec_bd->get_program_size();
+        erase_size = sec_bd->get_erase_size();
+        // We must be able to hold at least 10 small keys (20 program sectors) and master record + internal data
+        ul_bd_size  = align_up(program_size * 40, erase_size);
+        rbp_bd_size = align_up(program_size * 40, erase_size);
 
-        erase_size  = sec_bd->get_erase_size();
-        ul_bd_size  = erase_size * 4;
-        rbp_bd_size = erase_size * 2;
+        res = sec_bd->deinit();
+        TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
 
         ul_bd = new SlicingBlockDevice(sec_bd, 0, ul_bd_size);
         rbp_bd = new SlicingBlockDevice(sec_bd, ul_bd_size, ul_bd_size + rbp_bd_size);
@@ -407,14 +419,14 @@ static void set_several_key_value_sizes()
 
     name[6] = 0;
 
-    for (i = 0; i < 26; i++) {
+    for (i = 0; i < 5; i++) {
         c = i + 'a';
         name[5] = c;
         res = kvstore->set(name, name, sizeof(name), 0);
         TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
     }
 
-    for (i = 0; i < 26; i++) {
+    for (i = 0; i < 5; i++) {
         c = i + 'a';
         name[5] = c;
         res = kvstore->get(name, buffer, sizeof(buffer), &actual_size, 0);

features/storage/TESTS/kvstore/static_tests/main.cpp

@@ -34,15 +34,17 @@ static const size_t data_size = 5;
 static size_t       actual_size = 0;
 static const size_t buffer_size = 20;
 static const int    num_of_threads = 3;
-static const char   num_of_keys = 3;
+static const char num_of_keys = 11;
 #if defined(MBED_CONF_RTOS_PRESENT)
 /* Forked 3 threads plus misc, so minimum (4 * OS_STACK_SIZE) heap are required. */
 static const int heap_alloc_threshold_size = 4 * OS_STACK_SIZE;
 #else
 /* Bare metal does not require memory for threads, so use just minimum for test */
 static const int heap_alloc_threshold_size = MBED_CONF_TARGET_BOOT_STACK_SIZE;
 #endif
-static const char *keys[] = {"key1", "key2", "key3"};
+static const char *keys[num_of_keys] = { "key1", "key2", "key3", "key4", "key5", "key6", "key7", "key8", "key9",
+                                         "key10", "key11"
+                                       };
 
 static int init_res = MBED_ERROR_NOT_READY;
 
@@ -298,14 +300,14 @@ static void set_several_key_value_sizes()
 
     name[6] = 0;
 
-    for (i = 0; i < 26; i++) {
+    for (i = 0; i < num_of_keys; i++) {
         c = i + 'a';
         name[5] = c;
         res = kv_set(name, name, sizeof(name), 0);
         TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
     }
 
-    for (i = 0; i < 26; i++) {
+    for (i = 0; i < num_of_keys; i++) {
         c = i + 'a';
         name[5] = c;
         res = kv_get(name, buffer, sizeof(buffer), &actual_size);
@@ -328,7 +330,7 @@ static void set_several_unvalid_key_names()
 
     name[6] = 0;
 
-    for (i = 0; i < 11; i++) {
+    for (i = 0; i < num_of_keys; i++) {
         name[5] = unvalid[i];
         res = kv_set(name, name, sizeof(name), 0);
         if (unvalid[i] != '/') {
@@ -817,7 +819,7 @@ static void iterator_next_full_list()
     res = kv_iterator_close(kvstore_it);
     TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
 
-    for (i = 0; i < num_of_threads; i++) {
+    for (i = 0; i < num_of_keys; i++) {
         res = kv_remove(keys[i]);
         TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
     }
@@ -855,7 +857,7 @@ static void iterator_next_remove_while_iterating()
 
     int i = 0, res = 0;
 
-    for (i = 0; i < num_of_keys; i++) {
+    for (i = 0; i < 3; i++) {
         int res = kv_set(keys[i], data, data_size, 0);
         TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
     }

features/storage/kvstore/tdbstore/TDBStore.cpp

@@ -148,6 +148,10 @@ TDBStore::~TDBStore()
 
 int TDBStore::read_area(uint8_t area, uint32_t offset, uint32_t size, void *buf)
 {
+    //Check that we are not crossing area boundary
+    if (offset + size > _size) {
+        return MBED_ERROR_READ_FAILED;
+    }
     int os_ret = _buff_bd->read(buf, _area_params[area].address + offset, size);
 
     if (os_ret) {
@@ -649,6 +653,15 @@ int TDBStore::set_finalize(set_handle_t handle)
 
     _free_space_offset = align_up(ih->bd_curr_offset, _prog_size);
 
+    // Safety check: If there seems to be valid keys on the free space
+    // we should erase one sector more, just to ensure that in case of power failure
+    // next init() would not extend the scan phase to that section as well.
+    os_ret = read_record(_active_area, _free_space_offset, 0, 0, 0, actual_data_size, 0,
+                         false, false, false, false, hash, flags, next_offset);
+    if (os_ret == MBED_SUCCESS) {
+        check_erase_before_write(_active_area, _free_space_offset, sizeof(record_header_t));
+    }
+
 end:
     if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) {
         garbage_collection();
@@ -972,6 +985,7 @@ int TDBStore::increment_max_keys(void **ram_table)
     // Reallocate ram table with new size
     ram_table_entry_t *old_ram_table = (ram_table_entry_t *) _ram_table;
     ram_table_entry_t *new_ram_table = new ram_table_entry_t[_max_keys + 1];
+    memset(new_ram_table, 0, sizeof(ram_table_entry_t) * (_max_keys + 1));
 
     // Copy old content to new table
     memcpy(new_ram_table, old_ram_table, sizeof(ram_table_entry_t) * _max_keys);
@@ -1018,6 +1032,7 @@ int TDBStore::init()
     _max_keys = initial_max_keys;
 
     ram_table = new ram_table_entry_t[_max_keys];
+    memset(ram_table, 0, sizeof(ram_table_entry_t) * _max_keys);
     _ram_table = ram_table;
     _num_keys = 0;
 
@@ -1211,7 +1226,7 @@ int TDBStore::reset()
     _num_keys = 0;
     _free_space_offset = _master_record_offset;
     _active_area_version = 1;
-
+    memset(_ram_table, 0, sizeof(ram_table_entry_t) * _max_keys);
     // Write an initial master record on active area
     ret = write_master_record(_active_area, _active_area_version, _free_space_offset);
 

features/storage/kvstore/tdbstore/TDBStore.h

@@ -365,8 +365,8 @@ class TDBStore : public KVStore {
      *
      * @param[in]  area                   Area.
      * @param[in]  offset                 Offset of record in area.
-     * @param[in]  key                    Key - must not include '*' '/' '?' ':' ';' '\' '"' '|' ' ' '<' '>' '\'.
-     * @param[in]  data_buf               Data buffer.
+     * @param[out] key                    Key - must not include '*' '/' '?' ':' ';' '\' '"' '|' ' ' '<' '>' '\'.
+     * @param[out] data_buf               Data buffer.
      * @param[in]  data_buf_size          Data buffer size.
      * @param[out] actual_data_size       Actual data size.
      * @param[in]  data_offset            Offset in data.