From 6cdb8f0f49b8f9e1afe685a4b94915316a076204 Mon Sep 17 00:00:00 2001 From: Marcin Tomczyk Date: Mon, 22 Jul 2019 14:07:21 +0200 Subject: [PATCH] NVStore.cpp (and KVStore) - run-time failure handling missing --- .../kvstore/securestore/SecureStore.cpp | 3 + features/storage/nvstore/source/nvstore.cpp | 60 ++++++++++++++----- 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/features/storage/kvstore/securestore/SecureStore.cpp b/features/storage/kvstore/securestore/SecureStore.cpp index 17207481a7..58764dacef 100644 --- a/features/storage/kvstore/securestore/SecureStore.cpp +++ b/features/storage/kvstore/securestore/SecureStore.cpp @@ -736,6 +736,9 @@ int SecureStore::init() int ret = MBED_SUCCESS; MBED_ASSERT(!(scratch_buf_size % enc_block_size)); + if (scratch_buf_size % enc_block_size) { + return MBED_SYSTEM_ERROR_BASE; + } _mutex.lock(); #if defined(MBEDTLS_PLATFORM_C) diff --git a/features/storage/nvstore/source/nvstore.cpp b/features/storage/nvstore/source/nvstore.cpp index 5dc20fbb0b..42202eaa66 100644 --- a/features/storage/nvstore/source/nvstore.cpp +++ b/features/storage/nvstore/source/nvstore.cpp @@ -177,9 +177,7 @@ void NVStore::set_max_keys(uint16_t num_keys) { uint16_t key = 0, old_max_keys = 0; - MBED_ASSERT(num_keys < get_max_possible_keys()); - - if (num_keys < NVSTORE_NUM_PREDEFINED_KEYS) { + if (num_keys < NVSTORE_NUM_PREDEFINED_KEYS || num_keys >= get_max_possible_keys()) { return; } @@ -196,6 +194,7 @@ void NVStore::set_max_keys(uint16_t num_keys) if (num_keys < _max_keys) { for (key = num_keys; key < _max_keys; key++) { if (_offset_by_key[key] != 0) { + _mutex->unlock(); return; } } @@ -212,7 +211,12 @@ void NVStore::set_max_keys(uint16_t num_keys) // Reallocate _offset_by_key with new size uint32_t *old_offset_by_key = (uint32_t *) _offset_by_key; uint32_t *new_offset_by_key = new uint32_t[_max_keys]; + MBED_ASSERT(new_offset_by_key); + if (!new_offset_by_key) { + _mutex->unlock(); + return; + } // Copy old content to new table memset(new_offset_by_key, 0, sizeof(uint32_t) * _max_keys); @@ -249,6 +253,10 @@ void NVStore::calc_validate_area_params() size_t flash_addr; size_t sector_size; + if (flash_size == 0) { + return; + } + int area = 0; size_t left_size = flash_size; @@ -293,7 +301,6 @@ void NVStore::calc_validate_area_params() _flash_area_params[0].size = 0; _flash_area_params[1].size = 0; while (area >= 0) { - MBED_ASSERT(flash_addr > flash_start_addr); sector_size = _flash->get_sector_size(flash_addr - 1); flash_addr -= sector_size; _flash_area_params[area].size += sector_size; @@ -827,8 +834,9 @@ int NVStore::init() //Check if we are on internal memory && try to set the internal memory for TDBStore use. ret = avoid_conflict_nvstore_tdbstore(NVSTORE); //NVstore in internal memory can not be initialize when TDBStore is in use - MBED_ASSERT(ret != MBED_ERROR_ALREADY_INITIALIZED); - + if (ret == MBED_ERROR_ALREADY_INITIALIZED) { + return ret; + } // This handles the case that init function is called by more than one thread concurrently. // Only the one who gets the value of 1 in _init_attempts_val will proceed, while others will @@ -842,17 +850,23 @@ int NVStore::init() } _mutex = new PlatformMutex; - MBED_ASSERT(_mutex); + if (!_mutex) { + return NVSTORE_OS_ERROR; + } _size = (uint32_t) -1; _flash = new mbed::FlashIAP; - MBED_ASSERT(_flash); + if (!_flash) { + return NVSTORE_OS_ERROR; + } _flash->init(); _min_prog_size = std::max(_flash->get_page_size(), (uint32_t)sizeof(nvstore_record_header_t)); if (_min_prog_size > sizeof(nvstore_record_header_t)) { _page_buf = new uint8_t[_min_prog_size]; - MBED_ASSERT(_page_buf); + if (!_page_buf) { + return NVSTORE_OS_ERROR; + } } calc_validate_area_params(); @@ -869,7 +883,9 @@ int NVStore::init() // Find start of empty space at the end of the area. This serves for both // knowing whether the area is empty and for the record traversal at the end. ret = calc_empty_space(area, free_space_offset_of_area[area]); - MBED_ASSERT(!ret); + if (ret) { + return ret; + } if (!free_space_offset_of_area[area]) { area_state[area] = NVSTORE_AREA_STATE_EMPTY; @@ -881,7 +897,9 @@ int NVStore::init() ret = read_record(area, 0, sizeof(master_rec), &master_rec, actual_size, 0, valid, key, flags, owner, next_offset); - MBED_ASSERT((ret == NVSTORE_SUCCESS) || (ret == NVSTORE_BUFF_TOO_SMALL)); + if ((ret != NVSTORE_SUCCESS) && (ret != NVSTORE_BUFF_TOO_SMALL)) { + return ret; + } if (ret == NVSTORE_BUFF_TOO_SMALL) { // Buf too small error means that we have a corrupt master record - // treat it as such @@ -891,7 +909,9 @@ int NVStore::init() // We have a non valid master record, in a non-empty area. Just erase the area. if ((!valid) || (key != master_record_key)) { ret = flash_erase_area(area); - MBED_ASSERT(!ret); + if (ret) { + return ret; + } area_state[area] = NVSTORE_AREA_STATE_EMPTY; continue; } @@ -914,7 +934,9 @@ int NVStore::init() } _offset_by_key = new uint32_t[_max_keys]; - MBED_ASSERT(_offset_by_key); + if (!_offset_by_key) { + return NVSTORE_OS_ERROR; + } for (key = 0; key < _max_keys; key++) { _offset_by_key[key] = 0; @@ -924,7 +946,9 @@ int NVStore::init() if ((area_state[0] == NVSTORE_AREA_STATE_EMPTY) && (area_state[1] == NVSTORE_AREA_STATE_EMPTY)) { _active_area = 0; ret = write_master_record(_active_area, 1, _free_space_offset); - MBED_ASSERT(ret == NVSTORE_SUCCESS); + if (ret != NVSTORE_SUCCESS) { + return ret; + } _init_done = 1; return NVSTORE_SUCCESS; } @@ -939,7 +963,9 @@ int NVStore::init() } _active_area_version = versions[_active_area]; ret = flash_erase_area(1 - _active_area); - MBED_ASSERT(!ret); + if (ret) { + return ret; + } } // Traverse area until reaching the empty space at the end or until reaching a faulty record @@ -947,7 +973,9 @@ int NVStore::init() ret = read_record(_active_area, _free_space_offset, 0, NULL, actual_size, 1, valid, key, flags, owner, next_offset); - MBED_ASSERT(ret == NVSTORE_SUCCESS); + if (ret != NVSTORE_SUCCESS) { + return ret; + } // In case we have a faulty record, this probably means that the system crashed when written. // Perform a garbage collection, to make the other area valid.