Skip to contentSkip to conversation

efi: call get_event_log before ExitBootServices

PATCH In review Thiebaud Weksteen 2 participants 7 messages Sep 6, 2017 – Sep 7, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 efi: call get_event_log before ExitBootServices

With TPM 2.0, access to the event log is only possible by using the EFI TPM2 Boot Service. Modify the EFI stub to copy the log area to a new Linux-specific EFI table so it remains accessible for future use.

---
 arch/x86/boot/compressed/eboot.c      |  1 +
 drivers/char/tpm/tpm.h                | 29 ++++++++++---
 drivers/char/tpm/tpm_eventlog.h       | 26 +++---------
 drivers/firmware/efi/libstub/Makefile |  3 +-
 drivers/firmware/efi/libstub/tpm.c    | 80 +++++++++++++++++++++++++++++++++++
 include/linux/efi.h                   | 35 +++++++++++++++
 6 files changed, 146 insertions(+), 28 deletions(-)
arch/x86/boot/compressed/eboot.c +1 -0
index a1686f3dc295..97bec8df36ff 100644
@@ -999,6 +999,7 @@ struct boot_params *efi_main(struct efi_config *c,
 
 	/* Ask the firmware to clear memory on unclean shutdown */
 	efi_enable_reset_attack_mitigation(sys_table);
+	efi_retrieve_tpm2_eventlog(boot_params, sys_table);
 
 	setup_graphics(boot_params);
 
drivers/char/tpm/tpm.h +24 -5
index 04fbff2edbf3..efede7dc9340 100644
@@ -40,6 +40,8 @@
 #include <asm/intel-family.h>
 #endif
 
+#include "tpm_eventlog.h"
+
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 4096,
@@ -397,11 +399,6 @@ struct tpm_cmd_t {
 	tpm_cmd_params	params;
 } __packed;
 
-struct tpm2_digest {
-	u16 alg_id;
-	u8 digest[SHA512_DIGEST_SIZE];
-} __packed;
-
 /* A string buffer type for constructing TPM commands. This is based on the
  * ideas of string buffer code in security/keys/trusted.h but is heap based
  * in order to keep the stack usage minimal.
@@ -581,4 +578,26 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
 		       u8 *cmd);
 int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
 		      u32 cc, u8 *buf, size_t *bufsiz);
+
+extern const struct seq_operations tpm2_binary_b_measurements_seqops;
+
+#if defined(CONFIG_ACPI)
+int tpm_read_log_acpi(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_acpi(struct tpm_chip *chip)
+{
+	return -ENODEV;
+}
+#endif
+#if defined(CONFIG_OF)
+int tpm_read_log_of(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_of(struct tpm_chip *chip)
+{
+	return -ENODEV;
+}
+#endif
+
+int tpm_bios_log_setup(struct tpm_chip *chip);
+void tpm_bios_log_teardown(struct tpm_chip *chip);
 #endif
drivers/char/tpm/tpm_eventlog.h +5 -21
index b4b549559203..1009a2503985 100644
@@ -104,6 +104,11 @@ struct tcg_event_field {
 	u8 event[0];
 } __packed;
 
+struct tpm2_digest {
+	u16 alg_id;
+	u8 digest[SHA512_DIGEST_SIZE];
+} __packed;
+
 struct tcg_pcr_event2 {
 	u32 pcr_idx;
 	u32 event_type;
@@ -112,26 +117,5 @@ struct tcg_pcr_event2 {
 	struct tcg_event_field event;
 } __packed;
 
-extern const struct seq_operations tpm2_binary_b_measurements_seqops;
-
-#if defined(CONFIG_ACPI)
-int tpm_read_log_acpi(struct tpm_chip *chip);
-#else
-static inline int tpm_read_log_acpi(struct tpm_chip *chip)
-{
-	return -ENODEV;
-}
-#endif
-#if defined(CONFIG_OF)
-int tpm_read_log_of(struct tpm_chip *chip);
-#else
-static inline int tpm_read_log_of(struct tpm_chip *chip)
-{
-	return -ENODEV;
-}
-#endif
-
-int tpm_bios_log_setup(struct tpm_chip *chip);
-void tpm_bios_log_teardown(struct tpm_chip *chip);
 
 #endif
drivers/firmware/efi/libstub/Makefile +1 -2
index dedf9bde44db..2abe6d22dc5f 100644
@@ -29,8 +29,7 @@ OBJECT_FILES_NON_STANDARD	:= y
 # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
 KCOV_INSTRUMENT			:= n
 
-lib-y				:= efi-stub-helper.o gop.o secureboot.o
-lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
+lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
drivers/firmware/efi/libstub/tpm.c +80 -0

96 lines not on this page · raw message

include/linux/efi.h +35 -0
index 8dc3d94a3e3c..5eaaa68ac58a 100644
@@ -472,6 +472,26 @@ typedef struct {
 	u64 get_all;
 } apple_properties_protocol_64_t;
 
+typedef struct {
+	u32 get_capability;
+	u32 get_event_log;
+	u32 hash_log_extend_event;
+	u32 submit_command;
+	u32 get_active_pcr_banks;
+	u32 set_active_pcr_banks;
+	u32 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_32_t;
+
+typedef struct {
+	u64 get_capability;
+	u64 get_event_log;
+	u64 hash_log_extend_event;
+	u64 submit_command;
+	u64 get_active_pcr_banks;
+	u64 set_active_pcr_banks;
+	u64 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_64_t;
+
 /*
  * Types and defines for EFI ResetSystem
  */
@@ -622,6 +642,7 @@ void efi_native_runtime_setup(void);
 #define EFI_MEMORY_ATTRIBUTES_TABLE_GUID	EFI_GUID(0xdcfa911d, 0x26eb, 0x469f,  0xa2, 0x20, 0x38, 0xb7, 0xdc, 0x46, 0x12, 0x20)
 #define EFI_CONSOLE_OUT_DEVICE_GUID		EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4,  0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 #define APPLE_PROPERTIES_PROTOCOL_GUID		EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb,  0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)
+#define EFI_TCG2_PROTOCOL_GUID			EFI_GUID(0x607f766c, 0x7455, 0x42be,  0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
 
 #define EFI_IMAGE_SECURITY_DATABASE_GUID	EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596,  0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
 #define EFI_SHIM_LOCK_GUID			EFI_GUID(0x605dab50, 0xe046, 0x4300,  0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
@@ -634,6 +655,7 @@ void efi_native_runtime_setup(void);
 #define LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID	EFI_GUID(0xe03fc20a, 0x85dc, 0x406e,  0xb9, 0x0e, 0x4a, 0xb5, 0x02, 0x37, 0x1d, 0x95)
 #define LINUX_EFI_LOADER_ENTRY_GUID		EFI_GUID(0x4a67b082, 0x0a4c, 0x41cf,  0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f)
 #define LINUX_EFI_RANDOM_SEED_TABLE_GUID	EFI_GUID(0x1ce1e5bc, 0x7ceb, 0x42f2,  0x81, 0xe5, 0x8a, 0xad, 0xf1, 0x80, 0xf5, 0x7b)
+#define LINUX_EFI_TPM_EVENT_LOG_GUID		EFI_GUID(0xb7799cb0, 0xeca2, 0x4943,  0x96, 0x67, 0x1f, 0xae, 0x07, 0xb7, 0x47, 0xfa)
 
 typedef struct {
 	efi_guid_t guid;
@@ -1504,6 +1526,9 @@ static inline void
 efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) { }
 #endif
 
+void efi_retrieve_tpm2_eventlog(struct boot_params *boot_params,
+				efi_system_table_t *sys_table);
+
 /*
  * Arch code can implement the following three template macros, avoiding
  * reptition for the void/non-void return cases of {__,}efi_call_virt():
@@ -1571,4 +1596,14 @@ struct linux_efi_random_seed {
 	u8	bits[];
 };
 
+
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 0x1
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2   0x2
+
+struct linux_efi_tpm_eventlog {
+	u32	size;
+	u8	version;
+	u8	log[];
+};
+
 #endif /* _LINUX_EFI_H */
Signature · 1 line
2.14.1.581.gf28d330327-goog
#1 lore raw

PATCH 2/2 tpm: surface TPM event log based on EFI table

---
 drivers/char/tpm/Makefile        |  2 +-
 drivers/char/tpm/tpm.h           |  8 +++++
 drivers/char/tpm/tpm1_eventlog.c | 16 +++++++---
 drivers/char/tpm/tpm_efi.c       | 66 ++++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_eventlog.h  |  1 -
 drivers/firmware/efi/efi.c       |  2 ++
 include/linux/efi.h              |  1 +
 7 files changed, 90 insertions(+), 6 deletions(-)
 create mode 100644 drivers/char/tpm/tpm_efi.c
drivers/char/tpm/Makefile +1 -1
index 23681f01f95a..74182a63eef2 100644
@@ -4,7 +4,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 	 tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
-         tpm2-space.o
+         tpm2-space.o tpm_efi.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
 tpm-$(CONFIG_OF) += tpm_of.o
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
drivers/char/tpm/tpm.h +8 -0
index efede7dc9340..8858b6467c0d 100644
@@ -597,6 +597,14 @@ static inline int tpm_read_log_of(struct tpm_chip *chip)
 	return -ENODEV;
 }
 #endif
+#if defined(CONFIG_EFI)
+int tpm_read_log_efi(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_efi(struct tpm_chip *chip)
+{
+	return -ENODEV;
+}
+#endif
 
 int tpm_bios_log_setup(struct tpm_chip *chip);
 void tpm_bios_log_teardown(struct tpm_chip *chip);
drivers/char/tpm/tpm1_eventlog.c +12 -4
index 9a8605e500b5..c7d9b10de97f 100644
@@ -21,6 +21,7 @@
  */
 
 #include <linux/seq_file.h>
+#include <linux/efi.h>
 #include <linux/fs.h>
 #include <linux/security.h>
 #include <linux/module.h>
@@ -368,6 +369,10 @@ static int tpm_read_log(struct tpm_chip *chip)
 	}
 
 	rc = tpm_read_log_acpi(chip);
+	if (rc != -ENODEV && rc != -EIO)
+		return rc;
+
+	rc = tpm_read_log_efi(chip);
 	if (rc != -ENODEV)
 		return rc;
 
@@ -388,11 +393,13 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 {
 	const char *name = dev_name(&chip->dev);
 	unsigned int cnt;
-	int rc = 0;
+	int rc = 0, log_version;
+
 
 	rc = tpm_read_log(chip);
-	if (rc)
+	if (rc < 0)
 		return rc;
+	log_version = rc;
 
 	cnt = 0;
 	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
@@ -404,14 +411,15 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 	cnt++;
 
 	chip->bin_log_seqops.chip = chip;
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+
+	if (log_version == EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 ||
+			(!log_version && (chip->flags & TPM_CHIP_FLAG_TPM2)))
 		chip->bin_log_seqops.seqops =
 			&tpm2_binary_b_measurements_seqops;
 	else
 		chip->bin_log_seqops.seqops =
 			&tpm_binary_b_measurements_seqops;
 
-
 	chip->bios_dir[cnt] =
 	    securityfs_create_file("binary_bios_measurements",
 				   0440, chip->bios_dir[0],
drivers/char/tpm/tpm_efi.c new file +66 -0

66 lines not on this page · raw message

drivers/char/tpm/tpm_eventlog.h +0 -1
index 1009a2503985..9144a8707d70 100644
@@ -117,5 +117,4 @@ struct tcg_pcr_event2 {
 	struct tcg_event_field event;
 } __packed;
 
-
 #endif
drivers/firmware/efi/efi.c +2 -0
index f97f272e16ee..886f67f7519e 100644
@@ -52,6 +52,7 @@ struct efi __read_mostly efi = {
 	.properties_table	= EFI_INVALID_TABLE_ADDR,
 	.mem_attr_table		= EFI_INVALID_TABLE_ADDR,
 	.rng_seed		= EFI_INVALID_TABLE_ADDR,
+	.tpm_log		= EFI_INVALID_TABLE_ADDR
 };
 EXPORT_SYMBOL(efi);
 
@@ -444,6 +445,7 @@ static __initdata efi_config_table_type_t common_tables[] = {
 	{EFI_PROPERTIES_TABLE_GUID, "PROP", &efi.properties_table},
 	{EFI_MEMORY_ATTRIBUTES_TABLE_GUID, "MEMATTR", &efi.mem_attr_table},
 	{LINUX_EFI_RANDOM_SEED_TABLE_GUID, "RNG", &efi.rng_seed},
+	{LINUX_EFI_TPM_EVENT_LOG_GUID, "TPMEventLog", &efi.tpm_log},
 	{NULL_GUID, NULL, NULL},
 };
 
include/linux/efi.h +1 -0
index 5eaaa68ac58a..277a2347fa34 100644
@@ -930,6 +930,7 @@ extern struct efi {
 	unsigned long properties_table;	/* properties table */
 	unsigned long mem_attr_table;	/* memory attributes table */
 	unsigned long rng_seed;		/* UEFI firmware random seed */
+	unsigned long tpm_log;		/* TPM2 Event Log table */
 	efi_get_time_t *get_time;
 	efi_set_time_t *set_time;
 	efi_get_wakeup_time_t *get_wakeup_time;
Signature · 1 line
2.14.1.581.gf28d330327-goog
#2 lore raw

Re: [PATCH 2/2] tpm: surface TPM event log based on EFI table

Quoted 1 line · Thiebaud Weksteen

No empty commit logs please. If you expect people to review your code, you really need to explain what it does and why. On top of that, a cover letter that summarizes it and keeps track of the changes between submissions is highly appreciated as well.

Quoted 56 lines
---
 drivers/char/tpm/Makefile        |  2 +-
 drivers/char/tpm/tpm.h           |  8 +++++
 drivers/char/tpm/tpm1_eventlog.c | 16 +++++++---
 drivers/char/tpm/tpm_efi.c       | 66 ++++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_eventlog.h  |  1 -
 drivers/firmware/efi/efi.c       |  2 ++
 include/linux/efi.h              |  1 +
 7 files changed, 90 insertions(+), 6 deletions(-)
 create mode 100644 drivers/char/tpm/tpm_efi.c
drivers/char/tpm/Makefile +1 -1
index 23681f01f95a..74182a63eef2 100644
@@ -4,7 +4,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
         tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
-         tpm2-space.o
+         tpm2-space.o tpm_efi.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
 tpm-$(CONFIG_OF) += tpm_of.o
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
drivers/char/tpm/tpm.h +8 -0
index efede7dc9340..8858b6467c0d 100644
@@ -597,6 +597,14 @@ static inline int tpm_read_log_of(struct tpm_chip *chip)
        return -ENODEV;
 }
 #endif
+#if defined(CONFIG_EFI)
+int tpm_read_log_efi(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_efi(struct tpm_chip *chip)
+{
+       return -ENODEV;
+}
+#endif
 
 int tpm_bios_log_setup(struct tpm_chip *chip);
 void tpm_bios_log_teardown(struct tpm_chip *chip);
drivers/char/tpm/tpm1_eventlog.c +1 -0
index 9a8605e500b5..c7d9b10de97f 100644
@@ -21,6 +21,7 @@
  */
 
 #include <linux/seq_file.h>
+#include <linux/efi.h>
 #include <linux/fs.h>
 #include <linux/security.h>
 #include <linux/module.h>

drivers/char/tpm/tpm1_eventlog.c static int tpm_read_log(struct tpm_chip *chip) · line 372

Quoted 4 lines
drivers/char/tpm/tpm1_eventlog.c +1 -0
@@ -368,6 +369,10 @@ static int tpm_read_log(struct tpm_chip *chip)
        }
 
        rc = tpm_read_log_acpi(chip);
+       if (rc != -ENODEV && rc != -EIO)

What is the logic here? Why is -EIO treated differently all of a sudden?

Quoted 23 lines
drivers/char/tpm/tpm1_eventlog.c +7 -2
+               return rc;
+
+       rc = tpm_read_log_efi(chip);
        if (rc != -ENODEV)
                return rc;
 
@@ -388,11 +393,13 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 {
        const char *name = dev_name(&chip->dev);
        unsigned int cnt;
-       int rc = 0;
+       int rc = 0, log_version;
+
 
        rc = tpm_read_log(chip);
-       if (rc)
+       if (rc < 0)
                return rc;
+       log_version = rc;
 
        cnt = 0;
        chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);

drivers/char/tpm/tpm1_eventlog.c int tpm_bios_log_setup(struct tpm_chip *chip) · line 416

Quoted 7 lines
drivers/char/tpm/tpm1_eventlog.c +3 -1
@@ -404,14 +411,15 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
        cnt++;
 
        chip->bin_log_seqops.chip = chip;
-       if (chip->flags & TPM_CHIP_FLAG_TPM2)
+
+       if (log_version == EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 ||
+                       (!log_version && (chip->flags & TPM_CHIP_FLAG_TPM2)))

Indentation please

drivers/char/tpm/tpm1_eventlog.c int tpm_bios_log_setup(struct tpm_chip *chip) · line 414

Quoted 7 lines
drivers/char/tpm/tpm1_eventlog.c +0 -1
                chip->bin_log_seqops.seqops =
                        &tpm2_binary_b_measurements_seqops;
        else
                chip->bin_log_seqops.seqops =
                        &tpm_binary_b_measurements_seqops;
 
-

Spurious whitespace change.

Quoted 17 lines
drivers/char/tpm/tpm1_eventlog.c +0 -0
        chip->bios_dir[cnt] =
            securityfs_create_file("binary_bios_measurements",
                                   0440, chip->bios_dir[0],
drivers/char/tpm/tpm_efi.c new file +8 -0
new file mode 100644
index 000000000000..0ff070e5b4df
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2017 Google
+ *
+ * Authors:
+ *      Thiebaud Weksteen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License

drivers/char/tpm/tpm_efi.c line 18

Quoted 10 lines
drivers/char/tpm/tpm_efi.c new file +10 -0
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#include <linux/efi.h>
+
+#include "tpm.h"
+#include "tpm_eventlog.h"
+

In your previous patch, you include the latter into the former, so this is redundant.

Quoted 9 lines
drivers/char/tpm/tpm_efi.c new file +9 -0
+
+/* read binary bios log from EFI configuration table */
+int tpm_read_log_efi(struct tpm_chip *chip)
+{
+
+       struct linux_efi_tpm_eventlog *log_tbl;
+       struct tpm_bios_log *log;
+       u32 log_size;
+       u8 tpm_log_version;

drivers/char/tpm/tpm_efi.c line 37

Quoted 10 lines
drivers/char/tpm/tpm_efi.c new file +10 -0
+
+       if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+               return -ENODEV;
+
+       if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
+               return -ENODEV;
+
+       log = &chip->log;
+
+       log_tbl = ioremap(efi.tpm_log, sizeof(*log_tbl));

Use memremap for memory, not ioremap

drivers/char/tpm/tpm_efi.c line 46

Quoted 9 lines
drivers/char/tpm/tpm_efi.c new file +9 -0
+       if (!log_tbl) {
+               pr_err("Could not map UEFI TPM log table !\n");
+               return -ENOMEM;
+       }
+
+       log_size = log_tbl->size;
+       iounmap(log_tbl);
+
+       log_tbl = ioremap(efi.tpm_log, sizeof(*log_tbl) + log_size);

Same here

Quoted 25 lines
drivers/char/tpm/tpm_efi.c new file +20 -0
+       if (!log_tbl) {
+               pr_err("Could not map UEFI TPM log table payload!\n");
+               return -ENOMEM;
+       }
+
+       /* malloc EventLog space */
+       log->bios_event_log = kmalloc(log_size, GFP_KERNEL);
+       if (!log->bios_event_log)
+               goto err_iounmap;
+       memcpy(log->bios_event_log, log_tbl->log, log_size);
+       log->bios_event_log_end = log->bios_event_log + log_size;
+
+       tpm_log_version = log_tbl->version;
+       iounmap(log_tbl);
+       return tpm_log_version;
+
+err_iounmap:
+       iounmap(log_tbl);
+       return -ENOMEM;
+}
drivers/char/tpm/tpm_eventlog.h +0 -0
index 1009a2503985..9144a8707d70 100644

drivers/char/tpm/tpm_eventlog.h struct tcg_pcr_event2 { · line 120

Quoted 4 lines
drivers/char/tpm/tpm_eventlog.h +0 -1
@@ -117,5 +117,4 @@ struct tcg_pcr_event2 {
        struct tcg_event_field event;
 } __packed;
 
-

Spurious whitespace change

Quoted 40 lines
drivers/char/tpm/tpm_eventlog.h +0 -0
 #endif
drivers/firmware/efi/efi.c +2 -0
index f97f272e16ee..886f67f7519e 100644
@@ -52,6 +52,7 @@ struct efi __read_mostly efi = {
        .properties_table       = EFI_INVALID_TABLE_ADDR,
        .mem_attr_table         = EFI_INVALID_TABLE_ADDR,
        .rng_seed               = EFI_INVALID_TABLE_ADDR,
+       .tpm_log                = EFI_INVALID_TABLE_ADDR
 };
 EXPORT_SYMBOL(efi);
 
@@ -444,6 +445,7 @@ static __initdata efi_config_table_type_t common_tables[] = {
        {EFI_PROPERTIES_TABLE_GUID, "PROP", &efi.properties_table},
        {EFI_MEMORY_ATTRIBUTES_TABLE_GUID, "MEMATTR", &efi.mem_attr_table},
        {LINUX_EFI_RANDOM_SEED_TABLE_GUID, "RNG", &efi.rng_seed},
+       {LINUX_EFI_TPM_EVENT_LOG_GUID, "TPMEventLog", &efi.tpm_log},
        {NULL_GUID, NULL, NULL},
 };
 
include/linux/efi.h +1 -1
index 5eaaa68ac58a..277a2347fa34 100644
@@ -930,6 +930,7 @@ extern struct efi {
        unsigned long properties_table; /* properties table */
        unsigned long mem_attr_table;   /* memory attributes table */
        unsigned long rng_seed;         /* UEFI firmware random seed */
+       unsigned long tpm_log;          /* TPM2 Event Log table */
        efi_get_time_t *get_time;
        efi_set_time_t *set_time;
        efi_get_wakeup_time_t *get_wakeup_time;
--

2.14.1.581.gf28d330327-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
In reply to Thiebaud Weksteen

Hi Thiebaud,

Quoted 5 lines · Thiebaud Weksteen

With TPM 2.0, access to the event log is only possible by using the EFI TPM2 Boot Service. Modify the EFI stub to copy the log area to a new Linux-specific EFI table so it remains accessible for future use.

Quoted 9 lines
---
 arch/x86/boot/compressed/eboot.c      |  1 +
 drivers/char/tpm/tpm.h                | 29 ++++++++++---
 drivers/char/tpm/tpm_eventlog.h       | 26 +++---------
 drivers/firmware/efi/libstub/Makefile |  3 +-
 drivers/firmware/efi/libstub/tpm.c    | 80 +++++++++++++++++++++++++++++++++++
 include/linux/efi.h                   | 35 +++++++++++++++
 6 files changed, 146 insertions(+), 28 deletions(-)

Why is this x86 only?

Quoted 112 lines
arch/x86/boot/compressed/eboot.c +1 -0
index a1686f3dc295..97bec8df36ff 100644
@@ -999,6 +999,7 @@ struct boot_params *efi_main(struct efi_config *c,
 
        /* Ask the firmware to clear memory on unclean shutdown */
        efi_enable_reset_attack_mitigation(sys_table);
+       efi_retrieve_tpm2_eventlog(boot_params, sys_table);
 
        setup_graphics(boot_params);
 
drivers/char/tpm/tpm.h +24 -5
index 04fbff2edbf3..efede7dc9340 100644
@@ -40,6 +40,8 @@
 #include <asm/intel-family.h>
 #endif
 
+#include "tpm_eventlog.h"
+
 enum tpm_const {
        TPM_MINOR = 224,        /* officially assigned */
        TPM_BUFSIZE = 4096,
@@ -397,11 +399,6 @@ struct tpm_cmd_t {
        tpm_cmd_params  params;
 } __packed;
 
-struct tpm2_digest {
-       u16 alg_id;
-       u8 digest[SHA512_DIGEST_SIZE];
-} __packed;
-
 /* A string buffer type for constructing TPM commands. This is based on the
  * ideas of string buffer code in security/keys/trusted.h but is heap based
  * in order to keep the stack usage minimal.
@@ -581,4 +578,26 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
                       u8 *cmd);
 int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
                      u32 cc, u8 *buf, size_t *bufsiz);
+
+extern const struct seq_operations tpm2_binary_b_measurements_seqops;
+
+#if defined(CONFIG_ACPI)
+int tpm_read_log_acpi(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_acpi(struct tpm_chip *chip)
+{
+       return -ENODEV;
+}
+#endif
+#if defined(CONFIG_OF)
+int tpm_read_log_of(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_of(struct tpm_chip *chip)
+{
+       return -ENODEV;
+}
+#endif
+
+int tpm_bios_log_setup(struct tpm_chip *chip);
+void tpm_bios_log_teardown(struct tpm_chip *chip);
 #endif
drivers/char/tpm/tpm_eventlog.h +5 -21
index b4b549559203..1009a2503985 100644
@@ -104,6 +104,11 @@ struct tcg_event_field {
        u8 event[0];
 } __packed;
 
+struct tpm2_digest {
+       u16 alg_id;
+       u8 digest[SHA512_DIGEST_SIZE];
+} __packed;
+
 struct tcg_pcr_event2 {
        u32 pcr_idx;
        u32 event_type;
@@ -112,26 +117,5 @@ struct tcg_pcr_event2 {
        struct tcg_event_field event;
 } __packed;
 
-extern const struct seq_operations tpm2_binary_b_measurements_seqops;
-
-#if defined(CONFIG_ACPI)
-int tpm_read_log_acpi(struct tpm_chip *chip);
-#else
-static inline int tpm_read_log_acpi(struct tpm_chip *chip)
-{
-       return -ENODEV;
-}
-#endif
-#if defined(CONFIG_OF)
-int tpm_read_log_of(struct tpm_chip *chip);
-#else
-static inline int tpm_read_log_of(struct tpm_chip *chip)
-{
-       return -ENODEV;
-}
-#endif
-
-int tpm_bios_log_setup(struct tpm_chip *chip);
-void tpm_bios_log_teardown(struct tpm_chip *chip);
 
 #endif
drivers/firmware/efi/libstub/Makefile +0 -0
index dedf9bde44db..2abe6d22dc5f 100644

drivers/firmware/efi/libstub/Makefile OBJECT_FILES_NON_STANDARD := y · line 33

Quoted 7 lines
drivers/firmware/efi/libstub/Makefile +1 -2
@@ -29,8 +29,7 @@ OBJECT_FILES_NON_STANDARD     := y
 # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
 KCOV_INSTRUMENT                        := n
 
-lib-y                          := efi-stub-helper.o gop.o secureboot.o
-lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
+lib-y                          := efi-stub-helper.o gop.o secureboot.o tpm.o
 

Did you build test on ARM/arm64 as well?

Quoted 15 lines
drivers/firmware/efi/libstub/Makefile +0 -0
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
drivers/firmware/efi/libstub/tpm.c +1 -0
index 6224cdbc9669..ba2cb68833fb 100644
@@ -4,6 +4,7 @@
  * Copyright (C) 2016 CoreOS, Inc
  * Copyright (C) 2017 Google, Inc.
  *     Matthew Garrett <[email protected]>
+ *     Thiebaud Weksteen <[email protected]>
  *
  * This file is part of the Linux kernel, and is made available under the
  * terms of the GNU General Public License version 2.

drivers/firmware/efi/libstub/tpm.c line 18

Quoted 6 lines
drivers/firmware/efi/libstub/tpm.c +2 -0
@@ -12,7 +13,9 @@
 #include <asm/efi.h>
 
 #include "efistub.h"
+#include "../../../char/tpm/tpm_eventlog.h"
 
+#if IS_ENABLED(CONFIG_RESET_ATTACK_MITIGATION)

IS_ENABLED() is intended for C style if()s rather than preprocessor conditionals. Please replace with #ifdef

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 65

Quoted 11 lines
drivers/firmware/efi/libstub/tpm.c +4 -0
 static const efi_char16_t efi_MemoryOverWriteRequest_name[] = {
        'M', 'e', 'm', 'o', 'r', 'y', 'O', 'v', 'e', 'r', 'w', 'r', 'i', 't',
        'e', 'R', 'e', 'q', 'u', 'e', 's', 't', 'C', 'o', 'n', 't', 'r', 'o',
@@ -56,3 +59,80 @@ void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg)
                    EFI_VARIABLE_BOOTSERVICE_ACCESS |
                    EFI_VARIABLE_RUNTIME_ACCESS, sizeof(val), &val);
 }
+
+#endif
+
+void efi_retrieve_tpm2_eventlog_1_2(struct boot_params *boot_params,

Does this function actually use struct boot_params?

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 78

Quoted 13 lines
drivers/firmware/efi/libstub/tpm.c +13 -0
+                                   efi_system_table_t *sys_table_arg)
+{
+       efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
+       efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
+       efi_status_t status;
+       efi_physical_addr_t log_location, log_last_entry;
+       struct linux_efi_tpm_eventlog *log_tbl;
+       size_t log_size, last_entry_size;
+       efi_bool_t truncated;
+       void *tcg2_protocol;
+
+       status = efi_call_early(locate_protocol, &tcg2_guid, NULL,
+                       &tcg2_protocol);

Please indent appropriately.

Quoted 4 lines
drivers/firmware/efi/libstub/tpm.c +4 -0
+       if (status != EFI_SUCCESS)
+               return;
+
+       status = efi_call_proto(efi_tcg2_protocol, get_event_log, tcg2_protocol,

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 92

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+                               EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2,
+                               &log_location, &log_last_entry, &truncated);
+       if (status != EFI_SUCCESS)
+               return;
+
+       if (!log_location)
+               return;
+
+       /*
+        * If the logs are empty, we still populate the EFI table to surface

I am not a native English speaker, but 'to surface' sounds a bit awkward here.

Quoted 7 lines
drivers/firmware/efi/libstub/tpm.c +7 -0
+        * this information to userland.
+        */
+       if (!log_last_entry) {
+               log_size = 0;
+       } else {
+               /*
+                * get_event_log only returns the address of the last entry.

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 109

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+                * We need to calculate its size to deduce the full size of
+                * the logs.
+                */
+               last_entry_size = sizeof(struct tcpa_event) +
+                       ((struct tcpa_event *)log_last_entry)->event_size;
+               log_size = log_last_entry - log_location + last_entry_size;
+       }
+
+       /* Allocate space for the logs and copy them. */
+       status = efi_call_early(allocate_pool, EFI_RUNTIME_SERVICES_DATA,

Why is this runtime services data? You are passing your own data across the UEFI / kernel handover, and so if you parse your config table early enough, you can just reserve this memory.

Runtime services data will be given a virtual mapping by the firmware so it is accessible to the firmware itself during runtime services invocations, and on ARM/arm64, we actually have to omit if from the ordinary direct mapping of memory, which may lead to page table fragmentation and increased TLB pressure.

In summary, don't use it as a convenience so you don'y have to think about when/how to reserve it from normal allocation by the kernel.

Quoted 5 lines
drivers/firmware/efi/libstub/tpm.c +5 -0
+                               sizeof(*log_tbl) + log_size, &log_tbl);
+
+       if (status != EFI_SUCCESS) {
+               efi_printk(sys_table_arg,
+                          "Unable to allocate memory for event log\n");

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 124

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+               return;
+       }
+
+       memset(log_tbl, 0, sizeof(*log_tbl) + log_size);
+       log_tbl->size = log_size;
+       log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+       memcpy(log_tbl->log, (void *)log_location, log_size);
+
+       status = efi_call_early(install_configuration_table,
+                       &linux_eventlog_guid, log_tbl);

Indentation please.

Quoted 93 lines
drivers/firmware/efi/libstub/tpm.c +14 -0
+       if (status != EFI_SUCCESS)
+               goto err_free;
+       return;
+
+err_free:
+       efi_call_early(free_pool, log_tbl);
+}
+
+void efi_retrieve_tpm2_eventlog(struct boot_params *boot_params,
+                               efi_system_table_t *sys_table_arg)
+{
+       /* Only try to retrieve the logs in 1.2 format. */
+       efi_retrieve_tpm2_eventlog_1_2(boot_params, sys_table_arg);
+}
include/linux/efi.h +35 -1
index 8dc3d94a3e3c..5eaaa68ac58a 100644
@@ -472,6 +472,26 @@ typedef struct {
        u64 get_all;
 } apple_properties_protocol_64_t;
 
+typedef struct {
+       u32 get_capability;
+       u32 get_event_log;
+       u32 hash_log_extend_event;
+       u32 submit_command;
+       u32 get_active_pcr_banks;
+       u32 set_active_pcr_banks;
+       u32 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_32_t;
+
+typedef struct {
+       u64 get_capability;
+       u64 get_event_log;
+       u64 hash_log_extend_event;
+       u64 submit_command;
+       u64 get_active_pcr_banks;
+       u64 set_active_pcr_banks;
+       u64 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_64_t;
+
 /*
  * Types and defines for EFI ResetSystem
  */
@@ -622,6 +642,7 @@ void efi_native_runtime_setup(void);
 #define EFI_MEMORY_ATTRIBUTES_TABLE_GUID       EFI_GUID(0xdcfa911d, 0x26eb, 0x469f,  0xa2, 0x20, 0x38, 0xb7, 0xdc, 0x46, 0x12, 0x20)
 #define EFI_CONSOLE_OUT_DEVICE_GUID            EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4,  0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 #define APPLE_PROPERTIES_PROTOCOL_GUID         EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb,  0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)
+#define EFI_TCG2_PROTOCOL_GUID                 EFI_GUID(0x607f766c, 0x7455, 0x42be,  0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
 
 #define EFI_IMAGE_SECURITY_DATABASE_GUID       EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596,  0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
 #define EFI_SHIM_LOCK_GUID                     EFI_GUID(0x605dab50, 0xe046, 0x4300,  0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
@@ -634,6 +655,7 @@ void efi_native_runtime_setup(void);
 #define LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID   EFI_GUID(0xe03fc20a, 0x85dc, 0x406e,  0xb9, 0x0e, 0x4a, 0xb5, 0x02, 0x37, 0x1d, 0x95)
 #define LINUX_EFI_LOADER_ENTRY_GUID            EFI_GUID(0x4a67b082, 0x0a4c, 0x41cf,  0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f)
 #define LINUX_EFI_RANDOM_SEED_TABLE_GUID       EFI_GUID(0x1ce1e5bc, 0x7ceb, 0x42f2,  0x81, 0xe5, 0x8a, 0xad, 0xf1, 0x80, 0xf5, 0x7b)
+#define LINUX_EFI_TPM_EVENT_LOG_GUID           EFI_GUID(0xb7799cb0, 0xeca2, 0x4943,  0x96, 0x67, 0x1f, 0xae, 0x07, 0xb7, 0x47, 0xfa)
 
 typedef struct {
        efi_guid_t guid;
@@ -1504,6 +1526,9 @@ static inline void
 efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) { }
 #endif
 
+void efi_retrieve_tpm2_eventlog(struct boot_params *boot_params,
+                               efi_system_table_t *sys_table);
+
 /*
  * Arch code can implement the following three template macros, avoiding
  * reptition for the void/non-void return cases of {__,}efi_call_virt():
@@ -1571,4 +1596,14 @@ struct linux_efi_random_seed {
        u8      bits[];
 };
 
+
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 0x1
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2   0x2
+
+struct linux_efi_tpm_eventlog {
+       u32     size;
+       u8      version;
+       u8      log[];
+};
+
 #endif /* _LINUX_EFI_H */
--

2.14.1.581.gf28d330327-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw
Quoted 160 lines · Ard Biesheuvel

Hi Thiebaud,

Quoted 5 lines · Thiebaud Weksteen

With TPM 2.0, access to the event log is only possible by using the EFI TPM2 Boot Service. Modify the EFI stub to copy the log area to a new Linux-specific EFI table so it remains accessible for future use.

Quoted 9 lines
---
 arch/x86/boot/compressed/eboot.c      |  1 +
 drivers/char/tpm/tpm.h                | 29 ++++++++++---
 drivers/char/tpm/tpm_eventlog.h       | 26 +++---------
 drivers/firmware/efi/libstub/Makefile |  3 +-
 drivers/firmware/efi/libstub/tpm.c    | 80 +++++++++++++++++++++++++++++++++++
 include/linux/efi.h                   | 35 +++++++++++++++
 6 files changed, 146 insertions(+), 28 deletions(-)

Why is this x86 only?

Quoted 112 lines
arch/x86/boot/compressed/eboot.c +1 -0
index a1686f3dc295..97bec8df36ff 100644
@@ -999,6 +999,7 @@ struct boot_params *efi_main(struct efi_config *c,
 
        /* Ask the firmware to clear memory on unclean shutdown */
        efi_enable_reset_attack_mitigation(sys_table);
+       efi_retrieve_tpm2_eventlog(boot_params, sys_table);
 
        setup_graphics(boot_params);
 
drivers/char/tpm/tpm.h +24 -5
index 04fbff2edbf3..efede7dc9340 100644
@@ -40,6 +40,8 @@
 #include <asm/intel-family.h>
 #endif
 
+#include "tpm_eventlog.h"
+
 enum tpm_const {
        TPM_MINOR = 224,        /* officially assigned */
        TPM_BUFSIZE = 4096,
@@ -397,11 +399,6 @@ struct tpm_cmd_t {
        tpm_cmd_params  params;
 } __packed;
 
-struct tpm2_digest {
-       u16 alg_id;
-       u8 digest[SHA512_DIGEST_SIZE];
-} __packed;
-
 /* A string buffer type for constructing TPM commands. This is based on the
  * ideas of string buffer code in security/keys/trusted.h but is heap based
  * in order to keep the stack usage minimal.
@@ -581,4 +578,26 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
                       u8 *cmd);
 int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
                      u32 cc, u8 *buf, size_t *bufsiz);
+
+extern const struct seq_operations tpm2_binary_b_measurements_seqops;
+
+#if defined(CONFIG_ACPI)
+int tpm_read_log_acpi(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_acpi(struct tpm_chip *chip)
+{
+       return -ENODEV;
+}
+#endif
+#if defined(CONFIG_OF)
+int tpm_read_log_of(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_of(struct tpm_chip *chip)
+{
+       return -ENODEV;
+}
+#endif
+
+int tpm_bios_log_setup(struct tpm_chip *chip);
+void tpm_bios_log_teardown(struct tpm_chip *chip);
 #endif
drivers/char/tpm/tpm_eventlog.h +5 -21
index b4b549559203..1009a2503985 100644
@@ -104,6 +104,11 @@ struct tcg_event_field {
        u8 event[0];
 } __packed;
 
+struct tpm2_digest {
+       u16 alg_id;
+       u8 digest[SHA512_DIGEST_SIZE];
+} __packed;
+
 struct tcg_pcr_event2 {
        u32 pcr_idx;
        u32 event_type;
@@ -112,26 +117,5 @@ struct tcg_pcr_event2 {
        struct tcg_event_field event;
 } __packed;
 
-extern const struct seq_operations tpm2_binary_b_measurements_seqops;
-
-#if defined(CONFIG_ACPI)
-int tpm_read_log_acpi(struct tpm_chip *chip);
-#else
-static inline int tpm_read_log_acpi(struct tpm_chip *chip)
-{
-       return -ENODEV;
-}
-#endif
-#if defined(CONFIG_OF)
-int tpm_read_log_of(struct tpm_chip *chip);
-#else
-static inline int tpm_read_log_of(struct tpm_chip *chip)
-{
-       return -ENODEV;
-}
-#endif
-
-int tpm_bios_log_setup(struct tpm_chip *chip);
-void tpm_bios_log_teardown(struct tpm_chip *chip);
 
 #endif
drivers/firmware/efi/libstub/Makefile +0 -0
index dedf9bde44db..2abe6d22dc5f 100644

drivers/firmware/efi/libstub/Makefile OBJECT_FILES_NON_STANDARD := y · line 33

Quoted 7 lines
drivers/firmware/efi/libstub/Makefile +1 -2
@@ -29,8 +29,7 @@ OBJECT_FILES_NON_STANDARD     := y
 # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
 KCOV_INSTRUMENT                        := n
 
-lib-y                          := efi-stub-helper.o gop.o secureboot.o
-lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
+lib-y                          := efi-stub-helper.o gop.o secureboot.o tpm.o
 

Did you build test on ARM/arm64 as well?

Quoted 17 lines
drivers/firmware/efi/libstub/Makefile +0 -0
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
drivers/firmware/efi/libstub/tpm.c +1 -0
index 6224cdbc9669..ba2cb68833fb 100644
@@ -4,6 +4,7 @@
  * Copyright (C) 2016 CoreOS, Inc
  * Copyright (C) 2017 Google, Inc.
  *     Matthew Garrett <[email protected]>
+ *     Thiebaud Weksteen <[email protected]>
  *
  * This file is part of the Linux kernel, and is made available under the
  * terms of the GNU General Public License version 2.
@@ -12,7 +13,9 @@
 #include <asm/efi.h>
 
Quoted 3 lines

drivers/firmware/efi/libstub/tpm.c line 17

Quoted 3 lines
drivers/firmware/efi/libstub/tpm.c +1 -0
 #include "efistub.h"
+#include "../../../char/tpm/tpm_eventlog.h"
 

Oh, and please don't use includes like this. If there are data structures that are not local to drivers/char, put them in a .h file by themselves under include/linux somewhere.

Quoted 194 lines

drivers/firmware/efi/libstub/tpm.c line 18

Quoted 1 line
drivers/firmware/efi/libstub/tpm.c +1 -0
+#if IS_ENABLED(CONFIG_RESET_ATTACK_MITIGATION)

IS_ENABLED() is intended for C style if()s rather than preprocessor conditionals. Please replace with #ifdef

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 65

Quoted 11 lines
drivers/firmware/efi/libstub/tpm.c +4 -0
 static const efi_char16_t efi_MemoryOverWriteRequest_name[] = {
        'M', 'e', 'm', 'o', 'r', 'y', 'O', 'v', 'e', 'r', 'w', 'r', 'i', 't',
        'e', 'R', 'e', 'q', 'u', 'e', 's', 't', 'C', 'o', 'n', 't', 'r', 'o',
@@ -56,3 +59,80 @@ void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg)
                    EFI_VARIABLE_BOOTSERVICE_ACCESS |
                    EFI_VARIABLE_RUNTIME_ACCESS, sizeof(val), &val);
 }
+
+#endif
+
+void efi_retrieve_tpm2_eventlog_1_2(struct boot_params *boot_params,

Does this function actually use struct boot_params?

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 78

Quoted 13 lines
drivers/firmware/efi/libstub/tpm.c +13 -0
+                                   efi_system_table_t *sys_table_arg)
+{
+       efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
+       efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
+       efi_status_t status;
+       efi_physical_addr_t log_location, log_last_entry;
+       struct linux_efi_tpm_eventlog *log_tbl;
+       size_t log_size, last_entry_size;
+       efi_bool_t truncated;
+       void *tcg2_protocol;
+
+       status = efi_call_early(locate_protocol, &tcg2_guid, NULL,
+                       &tcg2_protocol);

Please indent appropriately.

Quoted 4 lines
drivers/firmware/efi/libstub/tpm.c +4 -0
+       if (status != EFI_SUCCESS)
+               return;
+
+       status = efi_call_proto(efi_tcg2_protocol, get_event_log, tcg2_protocol,

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 92

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+                               EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2,
+                               &log_location, &log_last_entry, &truncated);
+       if (status != EFI_SUCCESS)
+               return;
+
+       if (!log_location)
+               return;
+
+       /*
+        * If the logs are empty, we still populate the EFI table to surface

I am not a native English speaker, but 'to surface' sounds a bit awkward here.

Quoted 7 lines
drivers/firmware/efi/libstub/tpm.c +7 -0
+        * this information to userland.
+        */
+       if (!log_last_entry) {
+               log_size = 0;
+       } else {
+               /*
+                * get_event_log only returns the address of the last entry.

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 109

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+                * We need to calculate its size to deduce the full size of
+                * the logs.
+                */
+               last_entry_size = sizeof(struct tcpa_event) +
+                       ((struct tcpa_event *)log_last_entry)->event_size;
+               log_size = log_last_entry - log_location + last_entry_size;
+       }
+
+       /* Allocate space for the logs and copy them. */
+       status = efi_call_early(allocate_pool, EFI_RUNTIME_SERVICES_DATA,

Why is this runtime services data? You are passing your own data across the UEFI / kernel handover, and so if you parse your config table early enough, you can just reserve this memory.

Runtime services data will be given a virtual mapping by the firmware so it is accessible to the firmware itself during runtime services invocations, and on ARM/arm64, we actually have to omit if from the ordinary direct mapping of memory, which may lead to page table fragmentation and increased TLB pressure.

In summary, don't use it as a convenience so you don'y have to think about when/how to reserve it from normal allocation by the kernel.

Quoted 5 lines
drivers/firmware/efi/libstub/tpm.c +5 -0
+                               sizeof(*log_tbl) + log_size, &log_tbl);
+
+       if (status != EFI_SUCCESS) {
+               efi_printk(sys_table_arg,
+                          "Unable to allocate memory for event log\n");

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 124

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+               return;
+       }
+
+       memset(log_tbl, 0, sizeof(*log_tbl) + log_size);
+       log_tbl->size = log_size;
+       log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+       memcpy(log_tbl->log, (void *)log_location, log_size);
+
+       status = efi_call_early(install_configuration_table,
+                       &linux_eventlog_guid, log_tbl);

Indentation please.

Quoted 93 lines
drivers/firmware/efi/libstub/tpm.c +14 -0
+       if (status != EFI_SUCCESS)
+               goto err_free;
+       return;
+
+err_free:
+       efi_call_early(free_pool, log_tbl);
+}
+
+void efi_retrieve_tpm2_eventlog(struct boot_params *boot_params,
+                               efi_system_table_t *sys_table_arg)
+{
+       /* Only try to retrieve the logs in 1.2 format. */
+       efi_retrieve_tpm2_eventlog_1_2(boot_params, sys_table_arg);
+}
include/linux/efi.h +35 -1
index 8dc3d94a3e3c..5eaaa68ac58a 100644
@@ -472,6 +472,26 @@ typedef struct {
        u64 get_all;
 } apple_properties_protocol_64_t;
 
+typedef struct {
+       u32 get_capability;
+       u32 get_event_log;
+       u32 hash_log_extend_event;
+       u32 submit_command;
+       u32 get_active_pcr_banks;
+       u32 set_active_pcr_banks;
+       u32 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_32_t;
+
+typedef struct {
+       u64 get_capability;
+       u64 get_event_log;
+       u64 hash_log_extend_event;
+       u64 submit_command;
+       u64 get_active_pcr_banks;
+       u64 set_active_pcr_banks;
+       u64 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_64_t;
+
 /*
  * Types and defines for EFI ResetSystem
  */
@@ -622,6 +642,7 @@ void efi_native_runtime_setup(void);
 #define EFI_MEMORY_ATTRIBUTES_TABLE_GUID       EFI_GUID(0xdcfa911d, 0x26eb, 0x469f,  0xa2, 0x20, 0x38, 0xb7, 0xdc, 0x46, 0x12, 0x20)
 #define EFI_CONSOLE_OUT_DEVICE_GUID            EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4,  0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 #define APPLE_PROPERTIES_PROTOCOL_GUID         EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb,  0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)
+#define EFI_TCG2_PROTOCOL_GUID                 EFI_GUID(0x607f766c, 0x7455, 0x42be,  0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
 
 #define EFI_IMAGE_SECURITY_DATABASE_GUID       EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596,  0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
 #define EFI_SHIM_LOCK_GUID                     EFI_GUID(0x605dab50, 0xe046, 0x4300,  0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
@@ -634,6 +655,7 @@ void efi_native_runtime_setup(void);
 #define LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID   EFI_GUID(0xe03fc20a, 0x85dc, 0x406e,  0xb9, 0x0e, 0x4a, 0xb5, 0x02, 0x37, 0x1d, 0x95)
 #define LINUX_EFI_LOADER_ENTRY_GUID            EFI_GUID(0x4a67b082, 0x0a4c, 0x41cf,  0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f)
 #define LINUX_EFI_RANDOM_SEED_TABLE_GUID       EFI_GUID(0x1ce1e5bc, 0x7ceb, 0x42f2,  0x81, 0xe5, 0x8a, 0xad, 0xf1, 0x80, 0xf5, 0x7b)
+#define LINUX_EFI_TPM_EVENT_LOG_GUID           EFI_GUID(0xb7799cb0, 0xeca2, 0x4943,  0x96, 0x67, 0x1f, 0xae, 0x07, 0xb7, 0x47, 0xfa)
 
 typedef struct {
        efi_guid_t guid;
@@ -1504,6 +1526,9 @@ static inline void
 efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) { }
 #endif
 
+void efi_retrieve_tpm2_eventlog(struct boot_params *boot_params,
+                               efi_system_table_t *sys_table);
+
 /*
  * Arch code can implement the following three template macros, avoiding
  * reptition for the void/non-void return cases of {__,}efi_call_virt():
@@ -1571,4 +1596,14 @@ struct linux_efi_random_seed {
        u8      bits[];
 };
 
+
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 0x1
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2   0x2
+
+struct linux_efi_tpm_eventlog {
+       u32     size;
+       u8      version;
+       u8      log[];
+};
+
 #endif /* _LINUX_EFI_H */
--

2.14.1.581.gf28d330327-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw
In reply to Ard Biesheuvel

Hi Ard,

Thanks for reviewing the patch. (Non-addressed comments are fixed in the next patch set).

Quoted 18 lines · Ard Biesheuvel

Hi Thiebaud,

Quoted 5 lines · Thiebaud Weksteen

With TPM 2.0, access to the event log is only possible by using the EFI TPM2 Boot Service. Modify the EFI stub to copy the log area to a new Linux-specific EFI table so it remains accessible for future use.

Quoted 9 lines
---
 arch/x86/boot/compressed/eboot.c      |  1 +
 drivers/char/tpm/tpm.h                | 29 ++++++++++---
 drivers/char/tpm/tpm_eventlog.h       | 26 +++---------
 drivers/firmware/efi/libstub/Makefile |  3 +-
 drivers/firmware/efi/libstub/tpm.c    | 80 +++++++++++++++++++++++++++++++++++
 include/linux/efi.h                   | 35 +++++++++++++++
 6 files changed, 146 insertions(+), 28 deletions(-)
Quoted 1 line

Why is this x86 only?

I haven't had a chance to test on ARM/ARM64 yet.

Quoted 117 lines
Quoted 117 lines
arch/x86/boot/compressed/eboot.c +1 -0
index a1686f3dc295..97bec8df36ff 100644
@@ -999,6 +999,7 @@ struct boot_params *efi_main(struct efi_config *c,
 
        /* Ask the firmware to clear memory on unclean shutdown */
        efi_enable_reset_attack_mitigation(sys_table);
+       efi_retrieve_tpm2_eventlog(boot_params, sys_table);
 
        setup_graphics(boot_params);
 
drivers/char/tpm/tpm.h +24 -5
index 04fbff2edbf3..efede7dc9340 100644
@@ -40,6 +40,8 @@
 #include <asm/intel-family.h>
 #endif
 
+#include "tpm_eventlog.h"
+
 enum tpm_const {
        TPM_MINOR = 224,        /* officially assigned */
        TPM_BUFSIZE = 4096,
@@ -397,11 +399,6 @@ struct tpm_cmd_t {
        tpm_cmd_params  params;
 } __packed;
 
-struct tpm2_digest {
-       u16 alg_id;
-       u8 digest[SHA512_DIGEST_SIZE];
-} __packed;
-
 /* A string buffer type for constructing TPM commands. This is based on the
  * ideas of string buffer code in security/keys/trusted.h but is heap based
  * in order to keep the stack usage minimal.
@@ -581,4 +578,26 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
                       u8 *cmd);
 int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
                      u32 cc, u8 *buf, size_t *bufsiz);
+
+extern const struct seq_operations tpm2_binary_b_measurements_seqops;
+
+#if defined(CONFIG_ACPI)
+int tpm_read_log_acpi(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_acpi(struct tpm_chip *chip)
+{
+       return -ENODEV;
+}
+#endif
+#if defined(CONFIG_OF)
+int tpm_read_log_of(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_of(struct tpm_chip *chip)
+{
+       return -ENODEV;
+}
+#endif
+
+int tpm_bios_log_setup(struct tpm_chip *chip);
+void tpm_bios_log_teardown(struct tpm_chip *chip);
 #endif
drivers/char/tpm/tpm_eventlog.h +5 -21
index b4b549559203..1009a2503985 100644
@@ -104,6 +104,11 @@ struct tcg_event_field {
        u8 event[0];
 } __packed;
 
+struct tpm2_digest {
+       u16 alg_id;
+       u8 digest[SHA512_DIGEST_SIZE];
+} __packed;
+
 struct tcg_pcr_event2 {
        u32 pcr_idx;
        u32 event_type;
@@ -112,26 +117,5 @@ struct tcg_pcr_event2 {
        struct tcg_event_field event;
 } __packed;
 
-extern const struct seq_operations tpm2_binary_b_measurements_seqops;
-
-#if defined(CONFIG_ACPI)
-int tpm_read_log_acpi(struct tpm_chip *chip);
-#else
-static inline int tpm_read_log_acpi(struct tpm_chip *chip)
-{
-       return -ENODEV;
-}
-#endif
-#if defined(CONFIG_OF)
-int tpm_read_log_of(struct tpm_chip *chip);
-#else
-static inline int tpm_read_log_of(struct tpm_chip *chip)
-{
-       return -ENODEV;
-}
-#endif
-
-int tpm_bios_log_setup(struct tpm_chip *chip);
-void tpm_bios_log_teardown(struct tpm_chip *chip);
 
 #endif
drivers/firmware/efi/libstub/Makefile +0 -0
index dedf9bde44db..2abe6d22dc5f 100644
@@ -29,8 +29,7 @@ OBJECT_FILES_NON_STANDARD     := y
 # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
 KCOV_INSTRUMENT                        := n
 
Quoted 6 lines

drivers/firmware/efi/libstub/Makefile OBJECT_FILES_NON_STANDARD := y · line 33

Quoted 4 lines
drivers/firmware/efi/libstub/Makefile +1 -2
-lib-y                          := efi-stub-helper.o gop.o secureboot.o
-lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
+lib-y                          := efi-stub-helper.o gop.o secureboot.o tpm.o
 

Did you build test on ARM/arm64 as well?

I did not, but the next patch version build fine for both.

Quoted 91 lines
Quoted 16 lines
drivers/firmware/efi/libstub/Makefile +0 -0
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
drivers/firmware/efi/libstub/tpm.c +1 -0
index 6224cdbc9669..ba2cb68833fb 100644
@@ -4,6 +4,7 @@
  * Copyright (C) 2016 CoreOS, Inc
  * Copyright (C) 2017 Google, Inc.
  *     Matthew Garrett <[email protected]>
+ *     Thiebaud Weksteen <[email protected]>
  *
  * This file is part of the Linux kernel, and is made available under the
  * terms of the GNU General Public License version 2.

drivers/firmware/efi/libstub/tpm.c line 18

Quoted 6 lines
drivers/firmware/efi/libstub/tpm.c +2 -0
@@ -12,7 +13,9 @@
 #include <asm/efi.h>
 
 #include "efistub.h"
+#include "../../../char/tpm/tpm_eventlog.h"
 
+#if IS_ENABLED(CONFIG_RESET_ATTACK_MITIGATION)

IS_ENABLED() is intended for C style if()s rather than preprocessor conditionals. Please replace with #ifdef

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 65

Quoted 11 lines
drivers/firmware/efi/libstub/tpm.c +4 -0
 static const efi_char16_t efi_MemoryOverWriteRequest_name[] = {
        'M', 'e', 'm', 'o', 'r', 'y', 'O', 'v', 'e', 'r', 'w', 'r', 'i', 't',
        'e', 'R', 'e', 'q', 'u', 'e', 's', 't', 'C', 'o', 'n', 't', 'r', 'o',
@@ -56,3 +59,80 @@ void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg)
                    EFI_VARIABLE_BOOTSERVICE_ACCESS |
                    EFI_VARIABLE_RUNTIME_ACCESS, sizeof(val), &val);
 }
+
+#endif
+
+void efi_retrieve_tpm2_eventlog_1_2(struct boot_params *boot_params,

Does this function actually use struct boot_params?

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 78

Quoted 13 lines
drivers/firmware/efi/libstub/tpm.c +13 -0
+                                   efi_system_table_t *sys_table_arg)
+{
+       efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
+       efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
+       efi_status_t status;
+       efi_physical_addr_t log_location, log_last_entry;
+       struct linux_efi_tpm_eventlog *log_tbl;
+       size_t log_size, last_entry_size;
+       efi_bool_t truncated;
+       void *tcg2_protocol;
+
+       status = efi_call_early(locate_protocol, &tcg2_guid, NULL,
+                       &tcg2_protocol);

Please indent appropriately.

Quoted 4 lines
drivers/firmware/efi/libstub/tpm.c +4 -0
+       if (status != EFI_SUCCESS)
+               return;
+
+       status = efi_call_proto(efi_tcg2_protocol, get_event_log, tcg2_protocol,

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 92

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+                               EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2,
+                               &log_location, &log_last_entry, &truncated);
+       if (status != EFI_SUCCESS)
+               return;
+
+       if (!log_location)
+               return;
+
+       /*
+        * If the logs are empty, we still populate the EFI table to surface

I am not a native English speaker, but 'to surface' sounds a bit awkward here.

Quoted 7 lines
drivers/firmware/efi/libstub/tpm.c +7 -0
+        * this information to userland.
+        */
+       if (!log_last_entry) {
+               log_size = 0;
+       } else {
+               /*
+                * get_event_log only returns the address of the last entry.

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 109

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+                * We need to calculate its size to deduce the full size of
+                * the logs.
+                */
+               last_entry_size = sizeof(struct tcpa_event) +
+                       ((struct tcpa_event *)log_last_entry)->event_size;
+               log_size = log_last_entry - log_location + last_entry_size;
+       }
+
+       /* Allocate space for the logs and copy them. */
+       status = efi_call_early(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
Quoted 4 lines

Why is this runtime services data? You are passing your own data across the UEFI / kernel handover, and so if you parse your config table early enough, you can just reserve this memory.

I am simply following the specifications here: "In general, UEFI Configuration Tables loaded at boot time (e.g., SMBIOS table) can be contained in memory of type EfiRuntimeServicesData (recommended and the system firmware must not request a virtual mapping), EfiBootServicesData, EfiACPIReclaimMemory or EfiACPIMemoryNVS." [UEFI 2.6].

Although it is my own data, this is still an EFI configuration table and as such must remain available once booted. If you have another solution in mind, could you please be more explicit about how this would look like?

Quoted 120 lines

Runtime services data will be given a virtual mapping by the firmware so it is accessible to the firmware itself during runtime services invocations, and on ARM/arm64, we actually have to omit if from the ordinary direct mapping of memory, which may lead to page table fragmentation and increased TLB pressure.

In summary, don't use it as a convenience so you don'y have to think about when/how to reserve it from normal allocation by the kernel.

Quoted 5 lines
drivers/firmware/efi/libstub/tpm.c +5 -0
+                               sizeof(*log_tbl) + log_size, &log_tbl);
+
+       if (status != EFI_SUCCESS) {
+               efi_printk(sys_table_arg,
+                          "Unable to allocate memory for event log\n");

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 124

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+               return;
+       }
+
+       memset(log_tbl, 0, sizeof(*log_tbl) + log_size);
+       log_tbl->size = log_size;
+       log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+       memcpy(log_tbl->log, (void *)log_location, log_size);
+
+       status = efi_call_early(install_configuration_table,
+                       &linux_eventlog_guid, log_tbl);

Indentation please.

Quoted 93 lines
drivers/firmware/efi/libstub/tpm.c +14 -0
+       if (status != EFI_SUCCESS)
+               goto err_free;
+       return;
+
+err_free:
+       efi_call_early(free_pool, log_tbl);
+}
+
+void efi_retrieve_tpm2_eventlog(struct boot_params *boot_params,
+                               efi_system_table_t *sys_table_arg)
+{
+       /* Only try to retrieve the logs in 1.2 format. */
+       efi_retrieve_tpm2_eventlog_1_2(boot_params, sys_table_arg);
+}
include/linux/efi.h +35 -1
index 8dc3d94a3e3c..5eaaa68ac58a 100644
@@ -472,6 +472,26 @@ typedef struct {
        u64 get_all;
 } apple_properties_protocol_64_t;
 
+typedef struct {
+       u32 get_capability;
+       u32 get_event_log;
+       u32 hash_log_extend_event;
+       u32 submit_command;
+       u32 get_active_pcr_banks;
+       u32 set_active_pcr_banks;
+       u32 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_32_t;
+
+typedef struct {
+       u64 get_capability;
+       u64 get_event_log;
+       u64 hash_log_extend_event;
+       u64 submit_command;
+       u64 get_active_pcr_banks;
+       u64 set_active_pcr_banks;
+       u64 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_64_t;
+
 /*
  * Types and defines for EFI ResetSystem
  */
@@ -622,6 +642,7 @@ void efi_native_runtime_setup(void);
 #define EFI_MEMORY_ATTRIBUTES_TABLE_GUID       EFI_GUID(0xdcfa911d, 0x26eb, 0x469f,  0xa2, 0x20, 0x38, 0xb7, 0xdc, 0x46, 0x12, 0x20)
 #define EFI_CONSOLE_OUT_DEVICE_GUID            EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4,  0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 #define APPLE_PROPERTIES_PROTOCOL_GUID         EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb,  0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)
+#define EFI_TCG2_PROTOCOL_GUID                 EFI_GUID(0x607f766c, 0x7455, 0x42be,  0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
 
 #define EFI_IMAGE_SECURITY_DATABASE_GUID       EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596,  0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
 #define EFI_SHIM_LOCK_GUID                     EFI_GUID(0x605dab50, 0xe046, 0x4300,  0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
@@ -634,6 +655,7 @@ void efi_native_runtime_setup(void);
 #define LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID   EFI_GUID(0xe03fc20a, 0x85dc, 0x406e,  0xb9, 0x0e, 0x4a, 0xb5, 0x02, 0x37, 0x1d, 0x95)
 #define LINUX_EFI_LOADER_ENTRY_GUID            EFI_GUID(0x4a67b082, 0x0a4c, 0x41cf,  0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f)
 #define LINUX_EFI_RANDOM_SEED_TABLE_GUID       EFI_GUID(0x1ce1e5bc, 0x7ceb, 0x42f2,  0x81, 0xe5, 0x8a, 0xad, 0xf1, 0x80, 0xf5, 0x7b)
+#define LINUX_EFI_TPM_EVENT_LOG_GUID           EFI_GUID(0xb7799cb0, 0xeca2, 0x4943,  0x96, 0x67, 0x1f, 0xae, 0x07, 0xb7, 0x47, 0xfa)
 
 typedef struct {
        efi_guid_t guid;
@@ -1504,6 +1526,9 @@ static inline void
 efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) { }
 #endif
 
+void efi_retrieve_tpm2_eventlog(struct boot_params *boot_params,
+                               efi_system_table_t *sys_table);
+
 /*
  * Arch code can implement the following three template macros, avoiding
  * reptition for the void/non-void return cases of {__,}efi_call_virt():
@@ -1571,4 +1596,14 @@ struct linux_efi_random_seed {
        u8      bits[];
 };
 
+
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 0x1
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2   0x2
+
+struct linux_efi_tpm_eventlog {
+       u32     size;
+       u8      version;
+       u8      log[];
+};
+
 #endif /* _LINUX_EFI_H */
--

2.14.1.581.gf28d330327-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw
Quoted 145 lines · Thiebaud Weksteen

Hi Ard,

Thanks for reviewing the patch. (Non-addressed comments are fixed in the next patch set).

Quoted 18 lines · Ard Biesheuvel

Hi Thiebaud,

Quoted 5 lines · Thiebaud Weksteen

With TPM 2.0, access to the event log is only possible by using the EFI TPM2 Boot Service. Modify the EFI stub to copy the log area to a new Linux-specific EFI table so it remains accessible for future use.

Quoted 9 lines
---
 arch/x86/boot/compressed/eboot.c      |  1 +
 drivers/char/tpm/tpm.h                | 29 ++++++++++---
 drivers/char/tpm/tpm_eventlog.h       | 26 +++---------
 drivers/firmware/efi/libstub/Makefile |  3 +-
 drivers/firmware/efi/libstub/tpm.c    | 80 +++++++++++++++++++++++++++++++++++
 include/linux/efi.h                   | 35 +++++++++++++++
 6 files changed, 146 insertions(+), 28 deletions(-)
Quoted 1 line

Why is this x86 only?

I haven't had a chance to test on ARM/ARM64 yet.

Quoted 117 lines
Quoted 117 lines
arch/x86/boot/compressed/eboot.c +1 -0
index a1686f3dc295..97bec8df36ff 100644
@@ -999,6 +999,7 @@ struct boot_params *efi_main(struct efi_config *c,
 
        /* Ask the firmware to clear memory on unclean shutdown */
        efi_enable_reset_attack_mitigation(sys_table);
+       efi_retrieve_tpm2_eventlog(boot_params, sys_table);
 
        setup_graphics(boot_params);
 
drivers/char/tpm/tpm.h +24 -5
index 04fbff2edbf3..efede7dc9340 100644
@@ -40,6 +40,8 @@
 #include <asm/intel-family.h>
 #endif
 
+#include "tpm_eventlog.h"
+
 enum tpm_const {
        TPM_MINOR = 224,        /* officially assigned */
        TPM_BUFSIZE = 4096,
@@ -397,11 +399,6 @@ struct tpm_cmd_t {
        tpm_cmd_params  params;
 } __packed;
 
-struct tpm2_digest {
-       u16 alg_id;
-       u8 digest[SHA512_DIGEST_SIZE];
-} __packed;
-
 /* A string buffer type for constructing TPM commands. This is based on the
  * ideas of string buffer code in security/keys/trusted.h but is heap based
  * in order to keep the stack usage minimal.
@@ -581,4 +578,26 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
                       u8 *cmd);
 int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
                      u32 cc, u8 *buf, size_t *bufsiz);
+
+extern const struct seq_operations tpm2_binary_b_measurements_seqops;
+
+#if defined(CONFIG_ACPI)
+int tpm_read_log_acpi(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_acpi(struct tpm_chip *chip)
+{
+       return -ENODEV;
+}
+#endif
+#if defined(CONFIG_OF)
+int tpm_read_log_of(struct tpm_chip *chip);
+#else
+static inline int tpm_read_log_of(struct tpm_chip *chip)
+{
+       return -ENODEV;
+}
+#endif
+
+int tpm_bios_log_setup(struct tpm_chip *chip);
+void tpm_bios_log_teardown(struct tpm_chip *chip);
 #endif
drivers/char/tpm/tpm_eventlog.h +5 -21
index b4b549559203..1009a2503985 100644
@@ -104,6 +104,11 @@ struct tcg_event_field {
        u8 event[0];
 } __packed;
 
+struct tpm2_digest {
+       u16 alg_id;
+       u8 digest[SHA512_DIGEST_SIZE];
+} __packed;
+
 struct tcg_pcr_event2 {
        u32 pcr_idx;
        u32 event_type;
@@ -112,26 +117,5 @@ struct tcg_pcr_event2 {
        struct tcg_event_field event;
 } __packed;
 
-extern const struct seq_operations tpm2_binary_b_measurements_seqops;
-
-#if defined(CONFIG_ACPI)
-int tpm_read_log_acpi(struct tpm_chip *chip);
-#else
-static inline int tpm_read_log_acpi(struct tpm_chip *chip)
-{
-       return -ENODEV;
-}
-#endif
-#if defined(CONFIG_OF)
-int tpm_read_log_of(struct tpm_chip *chip);
-#else
-static inline int tpm_read_log_of(struct tpm_chip *chip)
-{
-       return -ENODEV;
-}
-#endif
-
-int tpm_bios_log_setup(struct tpm_chip *chip);
-void tpm_bios_log_teardown(struct tpm_chip *chip);
 
 #endif
drivers/firmware/efi/libstub/Makefile +0 -0
index dedf9bde44db..2abe6d22dc5f 100644
@@ -29,8 +29,7 @@ OBJECT_FILES_NON_STANDARD     := y
 # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
 KCOV_INSTRUMENT                        := n
 
Quoted 9 lines
Quoted 6 lines

drivers/firmware/efi/libstub/Makefile OBJECT_FILES_NON_STANDARD := y · line 33

Quoted 4 lines
drivers/firmware/efi/libstub/Makefile +1 -2
-lib-y                          := efi-stub-helper.o gop.o secureboot.o
-lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
+lib-y                          := efi-stub-helper.o gop.o secureboot.o tpm.o
 

Did you build test on ARM/arm64 as well?

I did not, but the next patch version build fine for both.

OK.

Quoted 96 lines
Quoted 91 lines
Quoted 16 lines
drivers/firmware/efi/libstub/Makefile +0 -0
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
drivers/firmware/efi/libstub/tpm.c +1 -0
index 6224cdbc9669..ba2cb68833fb 100644
@@ -4,6 +4,7 @@
  * Copyright (C) 2016 CoreOS, Inc
  * Copyright (C) 2017 Google, Inc.
  *     Matthew Garrett <[email protected]>
+ *     Thiebaud Weksteen <[email protected]>
  *
  * This file is part of the Linux kernel, and is made available under the
  * terms of the GNU General Public License version 2.

drivers/firmware/efi/libstub/tpm.c line 18

Quoted 6 lines
drivers/firmware/efi/libstub/tpm.c +2 -0
@@ -12,7 +13,9 @@
 #include <asm/efi.h>
 
 #include "efistub.h"
+#include "../../../char/tpm/tpm_eventlog.h"
 
+#if IS_ENABLED(CONFIG_RESET_ATTACK_MITIGATION)

IS_ENABLED() is intended for C style if()s rather than preprocessor conditionals. Please replace with #ifdef

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 65

Quoted 11 lines
drivers/firmware/efi/libstub/tpm.c +4 -0
 static const efi_char16_t efi_MemoryOverWriteRequest_name[] = {
        'M', 'e', 'm', 'o', 'r', 'y', 'O', 'v', 'e', 'r', 'w', 'r', 'i', 't',
        'e', 'R', 'e', 'q', 'u', 'e', 's', 't', 'C', 'o', 'n', 't', 'r', 'o',
@@ -56,3 +59,80 @@ void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg)
                    EFI_VARIABLE_BOOTSERVICE_ACCESS |
                    EFI_VARIABLE_RUNTIME_ACCESS, sizeof(val), &val);
 }
+
+#endif
+
+void efi_retrieve_tpm2_eventlog_1_2(struct boot_params *boot_params,

Does this function actually use struct boot_params?

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 78

Quoted 13 lines
drivers/firmware/efi/libstub/tpm.c +13 -0
+                                   efi_system_table_t *sys_table_arg)
+{
+       efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
+       efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
+       efi_status_t status;
+       efi_physical_addr_t log_location, log_last_entry;
+       struct linux_efi_tpm_eventlog *log_tbl;
+       size_t log_size, last_entry_size;
+       efi_bool_t truncated;
+       void *tcg2_protocol;
+
+       status = efi_call_early(locate_protocol, &tcg2_guid, NULL,
+                       &tcg2_protocol);

Please indent appropriately.

Quoted 4 lines
drivers/firmware/efi/libstub/tpm.c +4 -0
+       if (status != EFI_SUCCESS)
+               return;
+
+       status = efi_call_proto(efi_tcg2_protocol, get_event_log, tcg2_protocol,

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 92

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+                               EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2,
+                               &log_location, &log_last_entry, &truncated);
+       if (status != EFI_SUCCESS)
+               return;
+
+       if (!log_location)
+               return;
+
+       /*
+        * If the logs are empty, we still populate the EFI table to surface

I am not a native English speaker, but 'to surface' sounds a bit awkward here.

Quoted 7 lines
drivers/firmware/efi/libstub/tpm.c +7 -0
+        * this information to userland.
+        */
+       if (!log_last_entry) {
+               log_size = 0;
+       } else {
+               /*
+                * get_event_log only returns the address of the last entry.

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 109

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+                * We need to calculate its size to deduce the full size of
+                * the logs.
+                */
+               last_entry_size = sizeof(struct tcpa_event) +
+                       ((struct tcpa_event *)log_last_entry)->event_size;
+               log_size = log_last_entry - log_location + last_entry_size;
+       }
+
+       /* Allocate space for the logs and copy them. */
+       status = efi_call_early(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
Quoted 4 lines

Why is this runtime services data? You are passing your own data across the UEFI / kernel handover, and so if you parse your config table early enough, you can just reserve this memory.

Quoted 6 lines

I am simply following the specifications here: "In general, UEFI Configuration Tables loaded at boot time (e.g., SMBIOS table) can be contained in memory of type EfiRuntimeServicesData (recommended and the system firmware must not request a virtual mapping), EfiBootServicesData, EfiACPIReclaimMemory or EfiACPIMemoryNVS." [UEFI 2.6].

Actually, the use of EfiRuntimeServicesData without a good reason is also frowned upon in USWG circles, and this recommendation is a bit out of place, tbh. Not requesting a virtual mapping is impossible: the APIs don't allow you to specify whether a RTservicesdata allocation requires a virtual mapping or not, so this does not make any sense.

If you need memory that is reserved automatically, use EfiACPIReclaimMemory, otherwise, please use EfiBootServicesData instead.

Quoted 5 lines

Although it is my own data, this is still an EFI configuration table and as such must remain available once booted. If you have another solution in mind, could you please be more explicit about how this would look like?

This is not true. EFI configuration tables should not be stored in EfiRuntimeServicesData, because that will cause the memory to be reserved for all OSes, even ones that have no use for the data.

For example, the new memory attributes table is also stored in boot services data, for precisely the same reason.

Quoted 116 lines
Quoted 116 lines

Runtime services data will be given a virtual mapping by the firmware so it is accessible to the firmware itself during runtime services invocations, and on ARM/arm64, we actually have to omit if from the ordinary direct mapping of memory, which may lead to page table fragmentation and increased TLB pressure.

In summary, don't use it as a convenience so you don'y have to think about when/how to reserve it from normal allocation by the kernel.

Quoted 5 lines
drivers/firmware/efi/libstub/tpm.c +5 -0
+                               sizeof(*log_tbl) + log_size, &log_tbl);
+
+       if (status != EFI_SUCCESS) {
+               efi_printk(sys_table_arg,
+                          "Unable to allocate memory for event log\n");

drivers/firmware/efi/libstub/tpm.c void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) · line 124

Quoted 10 lines
drivers/firmware/efi/libstub/tpm.c +10 -0
+               return;
+       }
+
+       memset(log_tbl, 0, sizeof(*log_tbl) + log_size);
+       log_tbl->size = log_size;
+       log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+       memcpy(log_tbl->log, (void *)log_location, log_size);
+
+       status = efi_call_early(install_configuration_table,
+                       &linux_eventlog_guid, log_tbl);

Indentation please.

Quoted 89 lines
drivers/firmware/efi/libstub/tpm.c +14 -0
+       if (status != EFI_SUCCESS)
+               goto err_free;
+       return;
+
+err_free:
+       efi_call_early(free_pool, log_tbl);
+}
+
+void efi_retrieve_tpm2_eventlog(struct boot_params *boot_params,
+                               efi_system_table_t *sys_table_arg)
+{
+       /* Only try to retrieve the logs in 1.2 format. */
+       efi_retrieve_tpm2_eventlog_1_2(boot_params, sys_table_arg);
+}
include/linux/efi.h +35 -1
index 8dc3d94a3e3c..5eaaa68ac58a 100644
@@ -472,6 +472,26 @@ typedef struct {
        u64 get_all;
 } apple_properties_protocol_64_t;
 
+typedef struct {
+       u32 get_capability;
+       u32 get_event_log;
+       u32 hash_log_extend_event;
+       u32 submit_command;
+       u32 get_active_pcr_banks;
+       u32 set_active_pcr_banks;
+       u32 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_32_t;
+
+typedef struct {
+       u64 get_capability;
+       u64 get_event_log;
+       u64 hash_log_extend_event;
+       u64 submit_command;
+       u64 get_active_pcr_banks;
+       u64 set_active_pcr_banks;
+       u64 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_64_t;
+
 /*
  * Types and defines for EFI ResetSystem
  */
@@ -622,6 +642,7 @@ void efi_native_runtime_setup(void);
 #define EFI_MEMORY_ATTRIBUTES_TABLE_GUID       EFI_GUID(0xdcfa911d, 0x26eb, 0x469f,  0xa2, 0x20, 0x38, 0xb7, 0xdc, 0x46, 0x12, 0x20)
 #define EFI_CONSOLE_OUT_DEVICE_GUID            EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4,  0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 #define APPLE_PROPERTIES_PROTOCOL_GUID         EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb,  0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)
+#define EFI_TCG2_PROTOCOL_GUID                 EFI_GUID(0x607f766c, 0x7455, 0x42be,  0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
 
 #define EFI_IMAGE_SECURITY_DATABASE_GUID       EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596,  0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
 #define EFI_SHIM_LOCK_GUID                     EFI_GUID(0x605dab50, 0xe046, 0x4300,  0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
@@ -634,6 +655,7 @@ void efi_native_runtime_setup(void);
 #define LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID   EFI_GUID(0xe03fc20a, 0x85dc, 0x406e,  0xb9, 0x0e, 0x4a, 0xb5, 0x02, 0x37, 0x1d, 0x95)
 #define LINUX_EFI_LOADER_ENTRY_GUID            EFI_GUID(0x4a67b082, 0x0a4c, 0x41cf,  0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f)
 #define LINUX_EFI_RANDOM_SEED_TABLE_GUID       EFI_GUID(0x1ce1e5bc, 0x7ceb, 0x42f2,  0x81, 0xe5, 0x8a, 0xad, 0xf1, 0x80, 0xf5, 0x7b)
+#define LINUX_EFI_TPM_EVENT_LOG_GUID           EFI_GUID(0xb7799cb0, 0xeca2, 0x4943,  0x96, 0x67, 0x1f, 0xae, 0x07, 0xb7, 0x47, 0xfa)
 
 typedef struct {
        efi_guid_t guid;
@@ -1504,6 +1526,9 @@ static inline void
 efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) { }
 #endif
 
+void efi_retrieve_tpm2_eventlog(struct boot_params *boot_params,
+                               efi_system_table_t *sys_table);
+
 /*
  * Arch code can implement the following three template macros, avoiding
  * reptition for the void/non-void return cases of {__,}efi_call_virt():
@@ -1571,4 +1596,14 @@ struct linux_efi_random_seed {
        u8      bits[];
 };
 
+
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 0x1
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2   0x2
+
+struct linux_efi_tpm_eventlog {
+       u32     size;
+       u8      version;
+       u8      log[];
+};
+
 #endif /* _LINUX_EFI_H */
--

2.14.1.581.gf28d330327-goog

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot