diff options
author | Scott <scollyer@chromium.org> | 2016-08-10 12:34:52 -0700 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2016-08-11 14:46:42 -0700 |
commit | e6afb2ef97fbe09d65fe54e9ffcb0e6a9f26c95f (patch) | |
tree | 85d70a941e4c47a24958b87da09e1434be520ea7 /test | |
parent | 25c7cc0e4f62633660c8204f527fe19933c6b1ea (diff) | |
download | chrome-ec-e6afb2ef97fbe09d65fe54e9ffcb0e6a9f26c95f.tar.gz |
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>
Diffstat (limited to 'test')
-rw-r--r-- | test/nvmem.c | 11 |
1 files changed, 3 insertions, 8 deletions
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; } |