[coreboot-gerrit] Patch set updated for coreboot: vboot/tpm2: enable nvmem commits on cr50 when writing firmware secdata
Aaron Durbin (adurbin@chromium.org)
gerrit at coreboot.org
Wed Mar 8 18:38:33 CET 2017
Aaron Durbin (adurbin at chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/18681
-gerrit
commit 139bdb961bcc256ac2b84582c598d3a1743f5556
Author: Aaron Durbin <adurbin at chromium.org>
Date: Wed Mar 8 11:23:11 2017 -0600
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 <adurbin at chromium.org>
---
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 95bae77..581219e 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 dd046d2..6d4d622 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 6c0cd6e..b78b735 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 c5c6d87..ec5b674 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 4c1e128..c5d08ce 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 97dfc60..3c6237a 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
More information about the coreboot-gerrit
mailing list