From eeb77379e03d868a52d42cd4f0dfba8d11c3d337 Mon Sep 17 00:00:00 2001 From: Aaron Durbin Date: Wed, 8 Mar 2017 11:23:11 -0600 Subject: [PATCH] vboot/tpm2: enable nvmem commits on cr50 when writing firmware secdata cr50 by default delays nvmem commits internally from the point of reset to accumulate change state. However, the factory process can put a board into dev mode through the recovery screen. This state is stored in the TPM's nvmem space. When the factory process is complete a disable_dev_request and battery_cutoff_request is performed. This leads to disabling the dev mode in TPM, but the battery is subsequently cut off so the nvmem contents never stick. Therefore, whenever antirollback_write_space_firmware() is called we know there was a change in secdata so request cr50 to immediately enable nvmem commits going forward. This allows state changes to happen immediately. The fallout from this is that when secdata is changed that current boot will take longer because every transaction that writes to TPM nvmem space will perform a write synchronously. All subsequent boots do not have that effect. It should also be noted that this approach to the implementation is a pretty severe layering violation. However, the current TPM APIs don't lend themselves well to extending commands or re-using code outside of the current routines which inherently assume all knowledge of every command (in conflict with vendor commands since those are vendor-specific by definition). BUG=b:35775104 BRANCH=reef TEST=Confirmed disablement of dev mode sticks in the presence of: crossystem disable_dev_request=1; crossystem battery_cutoff_request=1; reboot; Change-Id: I3395db9cbdfea45da1f5cb994c6570978593b944 Signed-off-by: Aaron Durbin Reviewed-on: https://review.coreboot.org/18681 Reviewed-by: Duncan Laurie Tested-by: build bot (Jenkins) --- src/include/tpm_lite/tlcl.h | 6 +++++ src/lib/tpm2_marshaling.c | 37 ++++++++++++++++++++++++++ src/lib/tpm2_tlcl.c | 20 ++++++++++++++ src/lib/tpm2_tlcl_structures.h | 9 +++++++ src/vboot/secdata_tpm.c | 2 ++ src/vendorcode/google/chromeos/Kconfig | 4 +++ 6 files changed, 78 insertions(+) diff --git a/src/include/tpm_lite/tlcl.h b/src/include/tpm_lite/tlcl.h index 9c277a9293..c8e68d28e9 100644 --- a/src/include/tpm_lite/tlcl.h +++ b/src/include/tpm_lite/tlcl.h @@ -156,4 +156,10 @@ uint32_t tlcl_get_permanent_flags(TPM_PERMANENT_FLAGS *pflags); */ uint32_t tlcl_disable_platform_hierarchy(void); +/** + * CR50 specific tpm command to enable nvmem commits before internal timeout + * expires. + */ +uint32_t tlcl_cr50_enable_nvcommits(void); + #endif /* TPM_LITE_TLCL_H_ */ diff --git a/src/lib/tpm2_marshaling.c b/src/lib/tpm2_marshaling.c index dd046d27a1..6d4d622d91 100644 --- a/src/lib/tpm2_marshaling.c +++ b/src/lib/tpm2_marshaling.c @@ -390,6 +390,33 @@ static void marshal_hierarchy_control(void **buffer, marshal_u8(buffer, command_body->state, buffer_space); } +static void marshal_cr50_vendor_command(void **buffer, void *command_body, + size_t *buffer_space) +{ + uint16_t *sub_command; + + /* Ensure at least the sub command can fit in the body and there's + valid pointer. Could be reading past the buffer... */ + if (command_body == NULL || *buffer_space < sizeof(uint16_t)) { + *buffer_space = 0; + return; + } + + sub_command = command_body; + + switch (*sub_command) { + case TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS: + marshal_u16(buffer, *sub_command, buffer_space); + break; + default: + /* Unsupported subcommand. */ + printk(BIOS_WARNING, "Unsupported cr50 subcommand: 0x%04x\n", + *sub_command); + *buffer_space = 0; + break; + } +} + int tpm_marshal_command(TPM_CC command, void *tpm_command_body, void *buffer, size_t buffer_size) { @@ -444,6 +471,11 @@ int tpm_marshal_command(TPM_CC command, void *tpm_command_body, marshal_pcr_extend(&cmd_body, tpm_command_body, &body_size); break; + case TPM2_CR50_VENDOR_COMMAND: + marshal_cr50_vendor_command(&cmd_body, tpm_command_body, + &body_size); + break; + default: body_size = 0; printk(BIOS_INFO, "%s:%d:Request to marshal unsupported command %#x\n", @@ -615,6 +647,11 @@ struct tpm2_response *tpm_unmarshal_response(TPM_CC command, cr_size = 0; break; + case TPM2_CR50_VENDOR_COMMAND: + /* Assume no other data returned for the time being. */ + cr_size = 0; + break; + default: { int i; diff --git a/src/lib/tpm2_tlcl.c b/src/lib/tpm2_tlcl.c index 3d65a95e97..86cc74d425 100644 --- a/src/lib/tpm2_tlcl.c +++ b/src/lib/tpm2_tlcl.c @@ -385,3 +385,23 @@ uint32_t tlcl_disable_platform_hierarchy(void) return TPM_SUCCESS; } + +uint32_t tlcl_cr50_enable_nvcommits(void) +{ + uint16_t sub_command = TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS; + struct tpm2_response *response; + + printk(BIOS_INFO, "Enabling cr50 nvmem commmits\n"); + + response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, &sub_command); + + if (response == NULL || (response && response->hdr.tpm_code)) { + if (response) + printk(BIOS_INFO, "%s: failed %x\n", __func__, + response->hdr.tpm_code); + else + printk(BIOS_INFO, "%s: failed\n", __func__); + return TPM_E_IOERROR; + } + return TPM_SUCCESS; +} diff --git a/src/lib/tpm2_tlcl_structures.h b/src/lib/tpm2_tlcl_structures.h index c5c6d87985..ec5b674701 100644 --- a/src/lib/tpm2_tlcl_structures.h +++ b/src/lib/tpm2_tlcl_structures.h @@ -69,6 +69,15 @@ struct tpm_header { #define TPM2_NV_Read ((TPM_CC)0x0000014E) #define TPM2_GetCapability ((TPM_CC)0x0000017A) #define TPM2_PCR_Extend ((TPM_CC)0x00000182) +/* TPM2 specifies vendor commands need to have this bit set. Vendor command + space is defined by the lower 16 bits. */ +#define TPM_CC_VENDOR_BIT_MASK 0x20000000 +/* FIXME: below is not enough to differentiate between vendors commands + of numerous devices. However, the current tpm2 APIs aren't very amenable + to extending generically because the marshaling code is assuming all + knowledge of all commands. */ +#define TPM2_CR50_VENDOR_COMMAND ((TPM_CC)(TPM_CC_VENDOR_BIT_MASK | 0)) +#define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS (21) /* Startup values. */ #define TPM_SU_CLEAR 0 diff --git a/src/vboot/secdata_tpm.c b/src/vboot/secdata_tpm.c index 4c1e128797..c5d08ce27f 100644 --- a/src/vboot/secdata_tpm.c +++ b/src/vboot/secdata_tpm.c @@ -515,6 +515,8 @@ uint32_t antirollback_read_space_firmware(struct vb2_context *ctx) uint32_t antirollback_write_space_firmware(struct vb2_context *ctx) { + if (IS_ENABLED(CONFIG_CR50_IMMEDIATELY_COMMIT_FW_SECDATA)) + tlcl_cr50_enable_nvcommits(); return write_secdata(FIRMWARE_NV_INDEX, ctx->secdata, VB2_SECDATA_SIZE); } diff --git a/src/vendorcode/google/chromeos/Kconfig b/src/vendorcode/google/chromeos/Kconfig index 97dfc60c22..3c6237abcf 100644 --- a/src/vendorcode/google/chromeos/Kconfig +++ b/src/vendorcode/google/chromeos/Kconfig @@ -31,6 +31,10 @@ config CHROMEOS if CHROMEOS +config CR50_IMMEDIATELY_COMMIT_FW_SECDATA + bool + default y if MAINBOARD_HAS_I2C_TPM_CR50 || MAINBOARD_HAS_SPI_TPM_CR50 + config CHROMEOS_RAMOOPS bool "Reserve space for Chrome OS ramoops" default y