After the rewrite of persistent storage in SDK For Mesh v4.0.0 the code no longer does any basic sanity checks on the data read back from the flash.
The code below is from v3.2.0 showing the type of checks done on each entry read back from flash before accepting the new data.
static fm_iterate_action_t restore_acquired_model(const fm_entry_t * p_entry, void * p_success)
{
access_model_state_data_t * p_model_data_entry = (access_model_state_data_t *) p_entry->data;
uint16_t index = p_entry->header.handle & FLASH_HANDLE_TO_ACCESS_HANDLE_MASK;
if (index >= ACCESS_MODEL_COUNT ||
p_model_data_entry->element_index >= ACCESS_ELEMENT_COUNT ||
(m_model_pool[index].model_info.subscription_pool_index < ACCESS_SUBSCRIPTION_LIST_COUNT &&
(m_model_pool[index].model_info.subscription_pool_index != p_model_data_entry->subscription_pool_index ||
!ACCESS_INTERNAL_STATE_IS_ALLOCATED(m_subscription_list_pool[p_model_data_entry->subscription_pool_index].internal_state))))
{
The "same" code in v4.0.0. It does a basic check to ensure the id is within range, but that is all that is done.
static uint32_t models_setter(mesh_config_entry_id_t id, const void * p_entry)
{
if (!IS_IN_RANGE(id.record, MESH_OPT_ACCESS_MODELS_RECORD,
MESH_OPT_ACCESS_MODELS_RECORD + ACCESS_MODEL_COUNT - 1))
{
return NRF_ERROR_NOT_FOUND;
}
uint16_t idx = id.record - MESH_OPT_ACCESS_MODELS_RECORD;
if (!m_status.is_restoring_ended)
{
access_model_state_data_t * p_data = (access_model_state_data_t *)p_entry;
memcpy(&m_model_pool[idx].model_info, p_data, sizeof(access_model_state_data_t));
E.g. if the user would move a model from one element to another the code wouldn't detect this and thus come up with an invalid state.
The following patch improves on this by doing some basic sanity checks to ensure that the models added by the models_init_cb actually matches what is loaded from flash, otherwise it shall clean up and start an empty state.
diff --git a/mesh/access/src/access.c b/mesh/access/src/access.c
index 332e579..23c2a2b 100644
--- a/mesh/access/src/access.c
+++ b/mesh/access/src/access.c
@@ -693,6 +693,9 @@ static uint32_t elements_setter(mesh_config_entry_id_t id, const void * p_entry)
if (!m_status.is_restoring_ended)
{
uint16_t * p_data = (uint16_t *)p_entry;
+ if (*p_data != m_element_pool[idx].location) {
+ return NRF_ERROR_INVALID_DATA;
+ }
memcpy(&m_element_pool[idx].location, p_data, sizeof(uint16_t));
ACCESS_INTERNAL_STATE_INVALIDATED_CLR(m_element_pool[idx].internal_state);
@@ -725,6 +728,13 @@ static uint32_t models_setter(mesh_config_entry_id_t id, const void * p_entry)
if (!m_status.is_restoring_ended)
{
access_model_state_data_t * p_data = (access_model_state_data_t *)p_entry;
+ if (p_data->element_index != m_model_pool[idx].model_info.element_index ||
+ p_data->model_id.company_id != m_model_pool[idx].model_info.model_id.company_id ||
+ p_data->model_id.model_id != m_model_pool[idx].model_info.model_id.model_id ||
+ p_data->subscription_pool_index != m_model_pool[idx].model_info.subscription_pool_index)
+ {
+ return NRF_ERROR_INVALID_DATA;
+ }
memcpy(&m_model_pool[idx].model_info, p_data, sizeof(access_model_state_data_t));
ACCESS_INTERNAL_STATE_INVALIDATED_CLR(m_model_pool[idx].internal_state);
Thanks.
