nvmem: protect flash accesses with a mutex

Multiple tasks could be trying to modify NVMEM concurrently. To avoid
data corruption add a mutex which guarantees that only one thread of
execution has access to the flash storing NVMEM objects.

Various paths accessing flash contents are now protected by the same
mutex. Mutex control functions are put in wrappers, which makes it
easier to add debugging code when needed.

BRANCH=cr50, cr50-mp
BUG=b:69907320, b:130828517
TEST=attempts to take a device through RMA open do not fail any more.

Change-Id: I6424477dced20d00f6165006cd3b3968433be6d0
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1584586
Reviewed-by: Andrey Pronin <apronin@chromium.org>

1 files changed, 85 insertions, 5 deletions

diff --git a/common/new_nvmem.c b/common/new_nvmem.c
index 9a9973bd4b..de26e0cf90 100644
--- a/common/new_nvmem.c
+++ b/common/new_nvmem.c
@@ -19,6 +19,7 @@
 #include "nvmem_vars.h"
 #include "shared_mem.h"
 #include "system.h"
+#include "task.h"
 #include "timer.h"
 
 /*
@@ -315,6 +316,42 @@ static uint8_t page_list[NEW_NVMEM_TOTAL_PAGES];
 static uint32_t next_evict_obj_base;
 
 /*
+ * Mutex to protect flash space containing NVMEM objects. All operations
+ * modifying the flash contents or relying on its consistency (like searching
+ * in the flash) should acquire this mutex before proceeding.
+ *
+ * The interfaces grabbing this mutex are
+ *
+ *  new_nvmem_migrate()
+ *  new_nvmem_init()
+ *  new_nvmem_save()
+ *  getvar()
+ *  setvar()
+ *  nvmem_erase_tpm_data()
+ *
+ * The only static function using the mutex is browse_flash_contents() which
+ * can be invoked from the CLI and while it never modifies the flash contents,
+ * it still has to be protected to be able to properly iterate over the entire
+ * flash contents.
+ */
+static struct mutex flash_mtx;
+
+/*
+ * Wrappers around locking/unlocking mutex make it easier to debug issues by
+ * adding with minimal code changes like printouts of line numbers where the
+ * functions are invoked from.
+ */
+static void lock_mutex(int line_num)
+{
+	mutex_lock(&flash_mtx);
+}
+
+static void unlock_mutex(int line_num)
+{
+	mutex_unlock(&flash_mtx);
+}
+
+/*
  * Total space taken by key, value pairs in flash. It is limited to give TPM
  * objects priority.
  */
@@ -654,8 +691,9 @@ static uint32_t aligned_container_size(const struct nn_container *ch)
  *  EC_ERROR_MEMORY_ALLOCATION  if 'erased' object reached (not an error).
  *  EC_ERROR_INVAL	        if verification failed or read is out of sync.
  */
-enum ec_error_list get_next_object(struct access_tracker *at,
-				   struct nn_container *ch, int include_deleted)
+test_export_static enum ec_error_list get_next_object(struct access_tracker *at,
+						      struct nn_container *ch,
+						      int include_deleted)
 {
 	uint32_t salt[4];
 	uint8_t ctype;
@@ -1437,6 +1475,8 @@ enum ec_error_list new_nvmem_migrate(unsigned int act_partition)
 
 	ch = get_scratch_buffer(CONFIG_FLASH_BANK_SIZE);
 
+	lock_mutex(__LINE__);
+
 	/* Populate half of page_list with available page offsets. */
 	for (i = 0; i < ARRAY_SIZE(page_list) / 2; i++)
 		page_list[i] = flash_base / CONFIG_FLASH_BANK_SIZE + i;
@@ -1453,6 +1493,8 @@ enum ec_error_list new_nvmem_migrate(unsigned int act_partition)
 
 	add_final_delimiter();
 
+	unlock_mutex(__LINE__);
+
 	if (browse_flash_contents(0) != EC_SUCCESS)
 		/* Never returns. */
 		report_no_payload_failure(NVMEMF_MIGRATION_FAILURE);
@@ -2188,6 +2230,8 @@ enum ec_error_list new_nvmem_init(void)
 	/* Initialize NVMEM indices. */
 	NvEarlyStageFindHandle(0);
 
+	lock_mutex(__LINE__);
+
 	init_page_list();
 
 	start = get_time();
@@ -2196,6 +2240,8 @@ enum ec_error_list new_nvmem_init(void)
 
 	init = get_time();
 
+	unlock_mutex(__LINE__);
+
 	ccprintf("init took %d\n", (uint32_t)(init.val - start.val));
 
 	return rv;
@@ -2488,7 +2534,7 @@ static enum ec_error_list save_new_object(uint16_t obj_base, void *buf)
 	return save_container(ch);
 }
 
-enum ec_error_list new_nvmem_save(void)
+static enum ec_error_list new_nvmem_save_(void)
 {
 	const void *fence_ph;
 	size_t i;
@@ -2599,6 +2645,17 @@ enum ec_error_list new_nvmem_save(void)
 	return EC_SUCCESS;
 }
 
+enum ec_error_list new_nvmem_save(void)
+{
+	enum ec_error_list rv;
+
+	lock_mutex(__LINE__);
+	rv = new_nvmem_save_();
+	unlock_mutex(__LINE__);
+
+	return rv;
+}
+
 /* Caller must free memory allocated by this function! */
 static struct max_var_container *find_var(const uint8_t *key, size_t key_len,
 					  struct access_tracker *at)
@@ -2644,7 +2701,9 @@ const struct tuple *getvar(const uint8_t *key, uint8_t key_len)
 	if (!key || !key_len)
 		return NULL;
 
+	lock_mutex(__LINE__);
 	vc = find_var(key, key_len, &at);
+	unlock_mutex(__LINE__);
 
 	if (vc)
 		return &vc->t_header;
@@ -2687,8 +2746,8 @@ static enum ec_error_list save_container(struct nn_container *nc)
 	return save_object(nc);
 }
 
-int setvar(const uint8_t *key, uint8_t key_len, const uint8_t *val,
-	   uint8_t val_len)
+static int setvar_(const uint8_t *key, uint8_t key_len, const uint8_t *val,
+		   uint8_t val_len)
 {
 	enum ec_error_list rv;
 	int erase_request;
@@ -2790,6 +2849,18 @@ int setvar(const uint8_t *key, uint8_t key_len, const uint8_t *val,
 	return rv;
 }
 
+int setvar(const uint8_t *key, uint8_t key_len, const uint8_t *val,
+	   uint8_t val_len)
+{
+	int rv;
+
+	lock_mutex(__LINE__);
+	rv = setvar_(key, key_len, val, val_len);
+	unlock_mutex(__LINE__);
+
+	return rv;
+}
+
 static void dump_contents(const struct nn_container *ch)
 {
 	const uint8_t *buf = (const void *)ch;
@@ -2824,6 +2895,8 @@ int nvmem_erase_tpm_data(void)
 
 	ch = get_scratch_buffer(CONFIG_FLASH_BANK_SIZE);
 
+	lock_mutex(__LINE__);
+
 	while (get_next_object(&at, ch, 0) == EC_SUCCESS) {
 
 		if ((ch->container_type != NN_OBJ_TPM_RESERVED) &&
@@ -2833,6 +2906,8 @@ int nvmem_erase_tpm_data(void)
 		delete_object(&at, ch);
 	}
 
+	unlock_mutex(__LINE__);
+
 	shared_mem_release(ch);
 
 	/*
@@ -2891,7 +2966,9 @@ int nvmem_erase_tpm_data(void)
 
 	} while (master_at.list_index != (saved_list_index + 1));
 
+	lock_mutex(__LINE__);
 	rv = compact_nvmem();
+	unlock_mutex(__LINE__);
 
 	if (rv == EC_SUCCESS)
 		rv = new_nvmem_init();
@@ -2914,6 +2991,7 @@ test_export_static enum ec_error_list browse_flash_contents(int print)
 	struct access_tracker at = {};
 
 	ch = get_scratch_buffer(CONFIG_FLASH_BANK_SIZE);
+	lock_mutex(__LINE__);
 
 	while ((rv = get_next_object(&at, ch, 1)) == EC_SUCCESS) {
 		uint8_t ctype = ch->container_type;
@@ -2981,6 +3059,8 @@ test_export_static enum ec_error_list browse_flash_contents(int print)
 		}
 	}
 
+	unlock_mutex(__LINE__);
+
 	shared_mem_release(ch);
 
 	if (rv == EC_ERROR_MEMORY_ALLOCATION) {