Cr50: NvMem: Added write/move error state

The nvmem_write() and nvmem_move() funcitons return an error
if the write or move operation would exceed the user buffer
boundary. However, the TPM2 functions which call these functions
do not check for errors. Instead TPM2 NvMem relies on the return
value of the nv_commit() function to determine if a TPM command
which modifies NvMem succeeds or fails.

This CL adds a nvmem_write_error flag which is set in cases where
an nvmem_write/nvmem_move returns an error. This error flag
is then checked in nvmem_commit() so that the commit operation can
be abandonded and the error returned back up the TPM2 stack.

Tested in full system for two cases.

Installed TPM certificates on the Cr50, then manually erased NvMem with
flasherase 0x7b000 0x5000 and rebooted system. Then on Kevin console
entered the command <trunks_client --own>

NV_MEMORY_SIZE =  9932
NVMEM_TPM_SIZE =  7168

Case 1 -> Without internal write error state, so commit() always
executes if called. In this case, the Kevin console reports
a TRUNKS_RC_WRITE_ERROR and there is a Cr50 reboot.

Kevin Console:
localhost ~ # trunks_client --own
[INFO:tpm_utility_impl.cc(1692)] CreateStorageRootKeys: Created RSA SRK.
[INFO:tpm_utility_impl.cc(1735)] CreateStorageRootKeys: Created ECC SRK.
[  134.056217] tpm tpm0: Operation Timed out
[ERROR:tpm_utility_impl.cc(1987)] DoesPersistentKeyExist:
        querying handles: TRUNKS_RC_WRITE_ERROR
[ERROR:tpm_utility_impl.cc(269)] TakeOwnership: Error creating salting
        key: TRUNKS_RC_WRITE_ERROR
[ERROR:trunks_client.cc(98)] Error taking ownership: TRUNKS_RC_WRITE_ERROR

Cr50 Console:

> [131.501920 nv_commit()]
[142.494755 nv_wr: max off = 0x1250]
[142.496347 nv_wr: max off = 0x17b4]
[142.548296 nv_commit()]
[142.678001 nv_rd: max off = 0x1250]
[142.679350 nv_rd: max off = 0x1254]
[143.269614 Nv Wr:  overflow stop: reqst = 0x1d1c, avail = 0x1c00]
[143.271460 Nv Wr:  overflow stop: reqst = 0x1d20, avail = 0x1c00]
[143.273055 Wr Err = TRUE, Resetting error only, not returning]
[143.325073 nv_commit()]

--- UART initialized after reboot ---
[Reset cause: rtc-alarm]
[Image: RW_B, cr50_v1.1.5056-8e5dc99+ private-cr51:v0.0.69- 12:23:02]
[0.004349 Inits done]
[0.007150 Active NVram partition set to 0]
[0.008086 Debug Accessory connected]
[0.009076 USB PHY B]
Console is enabled; type HELP for help.
tpm_manufactured: manufactured
[1.155766 usb_reset]
[1.240155 usb_reset]
[1.311188 SETAD 0x6c (108)]

Case 2 -> Using internal error state to gate the commit() operation.
In this case, the attempted write overflow sets the internal error
state and the commit() following attempted overflow detection is not
exectued. It results in a different AP TPM error shown below as
Error encrypting salt. The other different behavior is that observed
is that if after failing on the RSA SRK, the ECC SRK write is still
attempted.

Kevin Console:
localhost ~ # trunks_client --own
[INFO:tpm_utility_impl.cc(1692)] CreateStorageRootKeys: Created RSA SRK.
[INFO:tpm_utility_impl.cc(1735)] CreateStorageRootKeys: Created ECC SRK.
[ERROR:session_manager_impl.cc(154)] Error fetching salting key public
        info: Handle 1: TPM_RC_HANDLE
[ERROR:session_manager_impl.cc(94)] Error encrypting salt: Handle 1:
        TPM_RC_HANDLE
[ERROR:tpm_utility_impl.cc(277)] TakeOwnership: Error initializing
        AuthorizationSession: Handle 1: TPM_RC_HANDLE
[ERROR:trunks_client.cc(98)] Error taking ownership: Handle 1:
        TPM_RC_HANDLE

Cr50 Console:
> [107.867473 nv_commit()]
[133.743522 nv_wr: max off = 0x123f]
[133.744908 nv_wr: max off = 0x1250]
[133.746159 nv_wr: max off = 0x17b4]
[133.798498 nv_commit()]
[133.900131 nv_rd: max off = 0x1250]
[133.901496 nv_rd: max off = 0x1254]
[134.507033 Nv Wr:  overflow stop: reqst = 0x1d1c, avail = 0x1c00]
[134.508852 Nv Wr:  overflow stop: reqst = 0x1d20, avail = 0x1c00]
[134.510440 Wr Err = TRUE, Aborting Commit!]
[144.856751 Nv Wr:  overflow stop: reqst = 0x1d1c, avail = 0x1c00]
[144.858611 Nv Wr:  overflow stop: reqst = 0x1d20, avail = 0x1c00]
[144.860198 Wr Err = TRUE, Aborting Commit!]

BRANCH=none
BUG=chrome-os-partner:55910
TEST=manual Test in system as described above and
ran NVMEM unit tests and verified that when a write would overrun the
user buffer, the write fails and sets the error state. Then,
verified that the nv_commit() call returns an error and clears
the internal error state.

Change-Id: I376e17b273003ff3d75459b4e68ed69d42dc7415
Signed-off-by: Scott <scollyer@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/366757
Commit-Ready: Scott Collyer <scollyer@chromium.org>
Tested-by: Scott Collyer <scollyer@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

2 files changed, 29 insertions, 15 deletions

diff --git a/common/nvmem.c b/common/nvmem.c
index 36784b4ec5..c293e26ce3 100644
--- a/common/nvmem.c
+++ b/common/nvmem.c
@@ -36,6 +36,8 @@ struct nvmem_cache cache;
 
 /* NvMem error state */
 static int nvmem_error_state;
+/* Flag to track if an Nv write/move is not completed */
+static int nvmem_write_error;
 
 static int nvmem_verify_partition_sha(int index)
 {
@@ -342,6 +344,7 @@ int nvmem_init(void)
 	}
 	/* Initialize error state, assume everything is good */
 	nvmem_error_state = EC_SUCCESS;
+	nvmem_write_error = 0;
 	/* Default state for cache base_ptr and task number */
 	cache.base_ptr = NULL;
 	cache.task = TASK_ID_COUNT;
@@ -436,14 +439,17 @@ int nvmem_write(uint32_t offset, uint32_t size,
 
 	/* Make sure that the cache buffer is active */
 	ret = nvmem_lock_cache();
-	if (ret)
-		/* TODO: What to do when can't access cache buffer? */
+	if (ret) {
+		nvmem_write_error = 1;
 		return ret;
+	}
 
 	/* Compute partition offset for this write operation */
 	ret = nvmem_get_partition_off(user, offset, size, &dest_offset);
-	if (ret != EC_SUCCESS)
+	if (ret != EC_SUCCESS) {
+		nvmem_write_error = 1;
 		return ret;
+	}
 
 	/* Advance to correct offset within data buffer */
 	dest_addr = (uintptr_t)cache.base_ptr;
@@ -465,19 +471,24 @@ int nvmem_move(uint32_t src_offset, uint32_t dest_offset, uint32_t size,
 
 	/* Make sure that the cache buffer is active */
 	ret = nvmem_lock_cache();
-	if (ret)
-		/* TODO: What to do when can't access cache buffer? */
+	if (ret) {
+		nvmem_write_error = 1;
 		return ret;
+	}
 
 	/* Compute partition offset for source */
 	ret = nvmem_get_partition_off(user, src_offset, size, &s_buff_offset);
-	if (ret != EC_SUCCESS)
+	if (ret != EC_SUCCESS) {
+		nvmem_write_error = 1;
 		return ret;
+	}
 
 	/* Compute partition offset for destination */
 	ret = nvmem_get_partition_off(user, dest_offset, size, &d_buff_offset);
-	if (ret != EC_SUCCESS)
+	if (ret != EC_SUCCESS) {
+		nvmem_write_error = 1;
 		return ret;
+	}
 
 	base_addr = (uintptr_t)cache.base_ptr;
 	/* Create pointer to src location within partition */
@@ -497,6 +508,14 @@ int nvmem_commit(void)
 	uint16_t version;
 	struct nvmem_partition *p_part;
 
+	/* Ensure that all writes/moves prior to commit call succeeded */
+	if (nvmem_write_error) {
+		CPRINTS("NvMem: Write Error, commit abandoned");
+		/* Clear error state */
+		nvmem_write_error = 0;
+		nvmem_release_cache();
+		return EC_ERROR_UNKNOWN;
+	}
 	/*
 	 * All scratch buffer blocks must be written to physical flash
 	 * memory. In addition, the scratch block buffer index table
diff --git a/test/nvmem.c b/test/nvmem.c
index b9dd76c8cf..bbd580589d 100644
--- a/test/nvmem.c
+++ b/test/nvmem.c
@@ -86,11 +86,6 @@ static int test_write_read(uint32_t offset, uint32_t num_bytes, int user)
 	generate_random_data(0, num_bytes);
 	/* Write source data to NvMem */
 	ret = nvmem_write(offset, num_bytes, write_buffer, user);
-	if (ret)
-		return ret;
-	nvmem_read(offset, num_bytes, read_buffer, user);
-	/* Verify memory was written into cache ram buffer */
-	TEST_ASSERT_ARRAY_EQ(write_buffer, read_buffer, num_bytes);
 	/* Write to flash */
 	ret = nvmem_commit();
 	if (ret != EC_SUCCESS)
@@ -117,9 +112,7 @@ static int write_full_buffer(uint32_t size, int user)
 		/* Generate data for tx buffer */
 		generate_random_data(offset, len);
 		/* Write data to Nvmem cache memory */
-		ret = nvmem_write(offset, len, &write_buffer[offset], user);
-		if (ret != EC_SUCCESS)
-			return ret;
+		nvmem_write(offset, len, &write_buffer[offset], user);
 		/* Write to flash */
 		ret = nvmem_commit();
 		if (ret != EC_SUCCESS)
@@ -427,6 +420,8 @@ static int test_move(void)
 			 len, user);
 	if (!ret)
 		return EC_ERROR_UNKNOWN;
+	/* nvmem_move returned an error, need to clear internal error state */
+	nvmem_commit();
 
 	return EC_SUCCESS;
 }