[coreboot-gerrit] Patch set updated for coreboot: vboot: make TPM factory init sequence more robust.

[Vadim Bendebury (vbendeb@chromium.org)]

Vadim Bendebury (vbendeb at chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17398

-gerrit

commit ac6994bb8182416bde5c1e1b4e17c275e19446ec
Author: Vadim Bendebury <vbendeb at chromium.org>
Date:   Mon Nov 14 16:36:26 2016 -0800

    vboot: make TPM factory init sequence more robust.
    
    Currently the code considers the absence of the NVRAM firmware
    rollback space a a trigger for invoking the TPM factory initialization
    sequence.
    
    Note that the kernel rollback and MRC cache hash spaces are created
    after the firmware rollback space. This opens an ever so narrow window
    of opportunity for bricking the device, in case a startup is
    interrupted after firmware space has been created, but before kernel
    and MRC hash spaces are created.
    
    The suggested solution is to create the firmware space last, and to
    allow for kernel and MRC cache spaces to exist during TPM factory
    initialization.
    
    BRANCH=none
    BUG=chrome-os-partner:59654
    TEST=odified the code not to create the firmware space, wiped out the
         TPM NVRAM and booted the device. Observed it create kernel and
         MRC cache spaces on the first run, and then reporting return code
         0x14c for already existing spaces on the following restarts.
    
         Verified that the device boots fine in normal and recovery modes
         and TPM NVRAM spaces are writeable in recovery mode.
    
    Change-Id: Id0e772448d6af1340e800ec3b78ec67913aa6289
    Signed-off-by: Vadim Bendebury <vbendeb at chromium.org>
---
 src/vboot/secdata_tpm.c | 44 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/src/vboot/secdata_tpm.c b/src/vboot/secdata_tpm.c
index 357d6e3..4c1e128 100644
--- a/src/vboot/secdata_tpm.c
+++ b/src/vboot/secdata_tpm.c
@@ -150,31 +150,53 @@ static uint32_t set_firmware_space(const void *firmware_blob)
 
 static uint32_t set_kernel_space(const void *kernel_blob)
 {
-	RETURN_ON_FAILURE(tlcl_define_space(KERNEL_NV_INDEX,
-					    sizeof(secdata_kernel)));
-	RETURN_ON_FAILURE(safe_write(KERNEL_NV_INDEX, kernel_blob,
-				     sizeof(secdata_kernel)));
-	return TPM_SUCCESS;
+	uint32_t rv;
+
+	rv = tlcl_define_space(KERNEL_NV_INDEX, sizeof(secdata_kernel));
+	if (rv == TPM_E_NV_DEFINED) {
+		VBDEBUG("%s: kernel space already exists\n", __func__);
+		return TPM_SUCCESS;
+	}
+
+	if (rv != TPM_SUCCESS)
+		return rv;
+
+	return safe_write(KERNEL_NV_INDEX, kernel_blob, sizeof(secdata_kernel));
 }
 
 static uint32_t set_rec_hash_space(const uint8_t *data)
 {
-	RETURN_ON_FAILURE(tlcl_define_space(REC_HASH_NV_INDEX,
-					    REC_HASH_NV_SIZE));
-	RETURN_ON_FAILURE(safe_write(REC_HASH_NV_INDEX, data,
-				     REC_HASH_NV_SIZE));
-	return TPM_SUCCESS;
+	uint32_t rv;
+
+	rv = tlcl_define_space(REC_HASH_NV_INDEX, REC_HASH_NV_SIZE);
+	if (rv == TPM_E_NV_DEFINED) {
+		VBDEBUG("%s: MRC Hash space already exists\n", __func__);
+		return TPM_SUCCESS;
+	}
+
+	if (rv != TPM_SUCCESS)
+		return rv;
+
+	return safe_write(REC_HASH_NV_INDEX, data, REC_HASH_NV_SIZE);
 }
 
 static uint32_t _factory_initialize_tpm(struct vb2_context *ctx)
 {
 	RETURN_ON_FAILURE(tlcl_force_clear());
-	RETURN_ON_FAILURE(set_firmware_space(ctx->secdata));
+
+	/*
+	 * Of all NVRAM spaces defined by this function the firmware space
+	 * must be defined last, because its existence is considered an
+	 * indication that TPM factory initialization was successfully
+	 * completed.
+	 */
 	RETURN_ON_FAILURE(set_kernel_space(secdata_kernel));
 
 	if (IS_ENABLED(CONFIG_VBOOT_HAS_REC_HASH_SPACE))
 		RETURN_ON_FAILURE(set_rec_hash_space(rec_hash_data));
 
+	RETURN_ON_FAILURE(set_firmware_space(ctx->secdata));
+
 	return TPM_SUCCESS;
 }