[coreboot-gerrit] New patch to review for coreboot: vboot: TPM2, recover from non-existing kernel space condition

Vadim Bendebury (vbendeb@chromium.org) gerrit at coreboot.org
Sat Nov 12 03:36:12 CET 2016


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 08530094b41105e322373e2963497b9749f8ede0
Author: Vadim Bendebury <vbendeb at chromium.org>
Date:   Fri Nov 11 14:15:31 2016 -0800

    vboot: TPM2, recover from non-existing kernel space condition
    
    The need to run tpm factory initialization is triggered by the absence
    of the FIRMWARE_NV_INDEX TPM NVRAM space. This indeed is a valid
    indication of an uninitialized TPM.
    
    This is not the only condition when TPM NVRAM spaces initialization is
    required though. The kernel space could be non-existent for a number
    of reasons: an earlier TPM factory initialization could fail, the
    kernel space was deleted accidentally or maliciously (as it is not
    created with attributes preventing its destruction).
    
    This patch enhances the logic determining when TPM factory
    initialization may be required: it is if either firmware or kernel
    space are absent.
    
    The code is enhanced not to fail if the TPM NVRAM being created
    already exists.
    
    This enhancement covers TPM2 case only, TPM1 path will have to be
    modified separately.
    
    BRANCH=none
    BUG=chrome-os-partner:59654
    TEST=modified the code not to create the kernel space, wiped out the
         TPM NVRAM and booted the device. Observed it create necessary
         spaces, 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/include/tpm_lite/tss_constants.h |  1 +
 src/lib/tpm2_tlcl.c                  | 10 ++++++--
 src/vboot/secdata_tpm.c              | 50 +++++++++++++++++++++++++++---------
 3 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/src/include/tpm_lite/tss_constants.h b/src/include/tpm_lite/tss_constants.h
index 883a5ad..6163b4b 100644
--- a/src/include/tpm_lite/tss_constants.h
+++ b/src/include/tpm_lite/tss_constants.h
@@ -40,6 +40,7 @@
 #define TPM_E_WRITE_FAILURE          ((uint32_t)0x00005008)  /* vboot local */
 #define TPM_E_READ_EMPTY             ((uint32_t)0x00005009)  /* vboot local */
 #define TPM_E_READ_FAILURE           ((uint32_t)0x0000500a)  /* vboot local */
+#define TPM_E_NV_DEFINED	     ((uint32_t)0x0000500b)  /* vboot local */
 
 #define TPM_NV_INDEX0 ((uint32_t)0x00000000)
 #define TPM_NV_INDEX_LOCK ((uint32_t)0xffffffff)
diff --git a/src/lib/tpm2_tlcl.c b/src/lib/tpm2_tlcl.c
index 457e874..08bb405 100644
--- a/src/lib/tpm2_tlcl.c
+++ b/src/lib/tpm2_tlcl.c
@@ -359,6 +359,12 @@ uint32_t tlcl_define_space(uint32_t space_index, size_t space_size)
 	if (!response)
 		return TPM_E_NO_DEVICE;
 
-	return response->hdr.tpm_code ? TPM_E_INTERNAL_INCONSISTENCY :
-		TPM_SUCCESS;
+	switch(response->hdr.tpm_code) {
+	case 0:
+		return TPM_SUCCESS;
+	case 0x14c:
+		return TPM_E_NV_DEFINED;
+	default:
+		return TPM_E_INTERNAL_INCONSISTENCY;
+	}
 }
diff --git a/src/vboot/secdata_tpm.c b/src/vboot/secdata_tpm.c
index 357d6e3..635c92f 100644
--- a/src/vboot/secdata_tpm.c
+++ b/src/vboot/secdata_tpm.c
@@ -139,9 +139,25 @@ static uint32_t safe_write(uint32_t index, const void *data, uint32_t length)
 	return tlcl_write(index, data, length);
 }
 
+/*
+ * Do not consider an error the situation when the NVRAM space to be defined
+ * alreasy exists.
+ */
+static uint32_t safe_define_space(uint32_t index, uint32_t size)
+{
+	int rv;
+
+	rv = tlcl_define_space(index, size);
+
+	if (rv == TPM_E_NV_DEFINED)
+		return TPM_SUCCESS;
+
+	return rv;
+}
+
 static uint32_t set_firmware_space(const void *firmware_blob)
 {
-	RETURN_ON_FAILURE(tlcl_define_space(FIRMWARE_NV_INDEX,
+	RETURN_ON_FAILURE(safe_define_space(FIRMWARE_NV_INDEX,
 					    VB2_SECDATA_SIZE));
 	RETURN_ON_FAILURE(safe_write(FIRMWARE_NV_INDEX, firmware_blob,
 				     VB2_SECDATA_SIZE));
@@ -150,7 +166,7 @@ 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,
+	RETURN_ON_FAILURE(safe_define_space(KERNEL_NV_INDEX,
 					    sizeof(secdata_kernel)));
 	RETURN_ON_FAILURE(safe_write(KERNEL_NV_INDEX, kernel_blob,
 				     sizeof(secdata_kernel)));
@@ -159,7 +175,7 @@ static uint32_t set_kernel_space(const void *kernel_blob)
 
 static uint32_t set_rec_hash_space(const uint8_t *data)
 {
-	RETURN_ON_FAILURE(tlcl_define_space(REC_HASH_NV_INDEX,
+	RETURN_ON_FAILURE(safe_define_space(REC_HASH_NV_INDEX,
 					    REC_HASH_NV_SIZE));
 	RETURN_ON_FAILURE(safe_write(REC_HASH_NV_INDEX, data,
 				     REC_HASH_NV_SIZE));
@@ -469,6 +485,8 @@ uint32_t setup_tpm(struct vb2_context *ctx)
 uint32_t antirollback_read_space_firmware(struct vb2_context *ctx)
 {
 	uint32_t rv;
+	uint32_t rv1;
+	uint8_t dummy;
 
 	rv = setup_tpm(ctx);
 	if (rv)
@@ -476,18 +494,26 @@ uint32_t antirollback_read_space_firmware(struct vb2_context *ctx)
 
 	/* Read the firmware space. */
 	rv = read_space_firmware(ctx);
-	if (rv == TPM_E_BADINDEX) {
-		/*
-		 * This seems the first time we've run. Initialize the TPM.
-		 */
-		VBDEBUG("TPM: Not initialized yet.\n");
-		RETURN_ON_FAILURE(factory_initialize_tpm(ctx));
-	} else if (rv != TPM_SUCCESS) {
-		VBDEBUG("TPM: Firmware space in a bad state; giving up.\n");
-		//RETURN_ON_FAILURE(factory_initialize_tpm(ctx));
+	rv1 = tlcl_read(KERNEL_NV_INDEX, &dummy, sizeof(dummy));
+
+	if ((rv == TPM_SUCCESS) && (rv1 == TPM_SUCCESS))
+		return TPM_SUCCESS;
+
+	if (((rv != TPM_E_BADINDEX) && (rv != TPM_SUCCESS)) ||
+	    ((rv1 != TPM_E_BADINDEX) && (rv1 != TPM_SUCCESS))) {
+		VBDEBUG("TPM: Firmware or kernel NVRAM space in a bad state; "
+			"giving up.\n");
 		return TPM_E_CORRUPTED_STATE;
 	}
 
+	/*
+	 * This seems the first time we've run, or kernel space could have
+	 * been deleted inadvertently or maliciously. Let's recreate default
+	 * spaces, the RW will have to make sure the kernel space is set to
+	 * the lowest acceptable index.
+	 */
+	VBDEBUG("TPM: Not (fully?) initialized yet.\n");
+	RETURN_ON_FAILURE(factory_initialize_tpm(ctx));
 	return TPM_SUCCESS;
 }
 



More information about the coreboot-gerrit mailing list