[PATCH v2 0/3] Call GetEventLog before ExitBootServices

[PATCH v2 0/3] Call GetEventLog before ExitBootServices

[Thiebaud Weksteen]

With TPM 1.2, the ACPI table ("TCPA") has two fields to recover the Event Log
Area (LAML and LASA). These logs are useful to understand and rebuild the
final values of PCRs.

With TPM 2.0, the ACPI table ("TPM2") does not contain these fields anymore.
The recommended method is now to call the GetEventLog EFI protocol before
ExitBootServices.

Implement this method within the EFI stub and create copy of the logs for the
TPM device. This will create /sys/kernel/security/tpm0/binary_bios_measurements
for TPM 2.0 devices (similarly to the current behaviour for TPM 1.2 devices).

-------------------------------------------------------------------------------

Patchset Changelog:

Version 2:
- Move tpm_eventlog.h to top include directory, add commit for this.
- Use EFI_LOADER_DATA to store the configuration table
- Whitespace and new lines fixes


Thiebaud Weksteen (3):
  tpm: move tpm_eventlog.h outside of drivers folder
  efi: call get_event_log before ExitBootServices
  tpm: parse TPM event logs based on EFI table

 arch/x86/boot/compressed/eboot.c                   |  1 +
 drivers/char/tpm/Makefile                          |  2 +-
 drivers/char/tpm/tpm-chip.c                        |  3 +-
 drivers/char/tpm/tpm-interface.c                   |  2 +-
 drivers/char/tpm/tpm.h                             | 35 ++++++++--
 drivers/char/tpm/tpm1_eventlog.c                   | 17 +++--
 drivers/char/tpm/tpm2_eventlog.c                   |  2 +-
 drivers/char/tpm/tpm_acpi.c                        |  2 +-
 drivers/char/tpm/tpm_efi.c                         | 66 ++++++++++++++++++
 drivers/char/tpm/tpm_of.c                          |  2 +-
 drivers/firmware/efi/Makefile                      |  2 +-
 drivers/firmware/efi/efi.c                         |  4 ++
 drivers/firmware/efi/libstub/Makefile              |  3 +-
 drivers/firmware/efi/libstub/tpm.c                 | 81 ++++++++++++++++++++++
 drivers/firmware/efi/tpm.c                         | 39 +++++++++++
 include/linux/efi.h                                | 50 +++++++++++++
 {drivers/char/tpm => include/linux}/tpm_eventlog.h | 32 ++-------
 17 files changed, 301 insertions(+), 42 deletions(-)
 create mode 100644 drivers/char/tpm/tpm_efi.c
 create mode 100644 drivers/firmware/efi/tpm.c
 rename {drivers/char/tpm => include/linux}/tpm_eventlog.h (77%)

-- 
2.14.1.581.gf28d330327-goog

[PATCH v2 0/3] Call GetEventLog before ExitBootServices

[Thiebaud Weksteen]

With TPM 1.2, the ACPI table ("TCPA") has two fields to recover the Event Log
Area (LAML and LASA). These logs are useful to understand and rebuild the
final values of PCRs.

With TPM 2.0, the ACPI table ("TPM2") does not contain these fields anymore.
The recommended method is now to call the GetEventLog EFI protocol before
ExitBootServices.

Implement this method within the EFI stub and create copy of the logs for the
TPM device. This will create /sys/kernel/security/tpm0/binary_bios_measurements
for TPM 2.0 devices (similarly to the current behaviour for TPM 1.2 devices).

-------------------------------------------------------------------------------

Patchset Changelog:

Version 2:
- Move tpm_eventlog.h to top include directory, add commit for this.
- Use EFI_LOADER_DATA to store the configuration table
- Whitespace and new lines fixes


Thiebaud Weksteen (3):
  tpm: move tpm_eventlog.h outside of drivers folder
  efi: call get_event_log before ExitBootServices
  tpm: parse TPM event logs based on EFI table

 arch/x86/boot/compressed/eboot.c                   |  1 +
 drivers/char/tpm/Makefile                          |  2 +-
 drivers/char/tpm/tpm-chip.c                        |  3 +-
 drivers/char/tpm/tpm-interface.c                   |  2 +-
 drivers/char/tpm/tpm.h                             | 35 ++++++++--
 drivers/char/tpm/tpm1_eventlog.c                   | 17 +++--
 drivers/char/tpm/tpm2_eventlog.c                   |  2 +-
 drivers/char/tpm/tpm_acpi.c                        |  2 +-
 drivers/char/tpm/tpm_efi.c                         | 66 ++++++++++++++++++
 drivers/char/tpm/tpm_of.c                          |  2 +-
 drivers/firmware/efi/Makefile                      |  2 +-
 drivers/firmware/efi/efi.c                         |  4 ++
 drivers/firmware/efi/libstub/Makefile              |  3 +-
 drivers/firmware/efi/libstub/tpm.c                 | 81 ++++++++++++++++++++++
 drivers/firmware/efi/tpm.c                         | 39 +++++++++++
 include/linux/efi.h                                | 50 +++++++++++++
 {drivers/char/tpm => include/linux}/tpm_eventlog.h | 32 ++-------
 17 files changed, 301 insertions(+), 42 deletions(-)
 create mode 100644 drivers/char/tpm/tpm_efi.c
 create mode 100644 drivers/firmware/efi/tpm.c
 rename {drivers/char/tpm => include/linux}/tpm_eventlog.h (77%)

-- 
2.14.1.581.gf28d330327-goog

[PATCH v2 1/3] tpm: move tpm_eventlog.h outside of drivers folder

[Thiebaud Weksteen]

The generic definitions of data structures in tpm_eventlog.h are
required by other part of the kernel (namely, the EFI stub).

Signed-off-by: Thiebaud Weksteen <tweek@google.com>
---
 drivers/char/tpm/tpm-chip.c                        |  3 +-
 drivers/char/tpm/tpm-interface.c                   |  2 +-
 drivers/char/tpm/tpm.h                             | 27 +++++++++++++++---
 drivers/char/tpm/tpm1_eventlog.c                   |  2 +-
 drivers/char/tpm/tpm2_eventlog.c                   |  2 +-
 drivers/char/tpm/tpm_acpi.c                        |  2 +-
 drivers/char/tpm/tpm_of.c                          |  2 +-
 {drivers/char/tpm => include/linux}/tpm_eventlog.h | 32 +++++-----------------
 8 files changed, 37 insertions(+), 35 deletions(-)
 rename {drivers/char/tpm => include/linux}/tpm_eventlog.h (77%)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 67ec9d3d04f5..de2680118181 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -26,8 +26,9 @@
 #include <linux/spinlock.h>
 #include <linux/freezer.h>
 #include <linux/major.h>
+#include <linux/tpm_eventlog.h>
+
 #include "tpm.h"
-#include "tpm_eventlog.h"
 
 DEFINE_IDR(dev_nums_idr);
 static DEFINE_MUTEX(idr_lock);
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index fe597e6c55c4..bd7091d510bd 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -30,9 +30,9 @@
 #include <linux/spinlock.h>
 #include <linux/freezer.h>
 #include <linux/pm_runtime.h>
+#include <linux/tpm_eventlog.h>
 
 #include "tpm.h"
-#include "tpm_eventlog.h"
 
 #define TPM_MAX_ORDINAL 243
 #define TSC_MAX_ORDINAL 12
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 04fbff2edbf3..46caccf6fd1a 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -34,6 +34,7 @@
 #include <linux/acpi.h>
 #include <linux/cdev.h>
 #include <linux/highmem.h>
+#include <linux/tpm_eventlog.h>
 #include <crypto/hash_info.h>
 
 #ifdef CONFIG_X86
@@ -397,10 +398,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
@@ -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
diff --git a/drivers/char/tpm/tpm1_eventlog.c b/drivers/char/tpm/tpm1_eventlog.c
index 9a8605e500b5..d6f70f365443 100644
--- a/drivers/char/tpm/tpm1_eventlog.c
+++ b/drivers/char/tpm/tpm1_eventlog.c
@@ -25,9 +25,9 @@
 #include <linux/security.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/tpm_eventlog.h>
 
 #include "tpm.h"
-#include "tpm_eventlog.h"
 
 
 static const char* tcpa_event_type_strings[] = {
diff --git a/drivers/char/tpm/tpm2_eventlog.c b/drivers/char/tpm/tpm2_eventlog.c
index 34a8afa69138..1ce4411292ba 100644
--- a/drivers/char/tpm/tpm2_eventlog.c
+++ b/drivers/char/tpm/tpm2_eventlog.c
@@ -21,9 +21,9 @@
 #include <linux/security.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/tpm_eventlog.h>
 
 #include "tpm.h"
-#include "tpm_eventlog.h"
 
 /*
  * calc_tpm2_event_size() - calculate the event size, where event
diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
index 169edf3ce86d..acc990ba376a 100644
--- a/drivers/char/tpm/tpm_acpi.c
+++ b/drivers/char/tpm/tpm_acpi.c
@@ -25,9 +25,9 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
+#include <linux/tpm_eventlog.h>
 
 #include "tpm.h"
-#include "tpm_eventlog.h"
 
 struct acpi_tcpa {
 	struct acpi_table_header hdr;
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index aadb7f464076..4a2f8c79231e 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -17,9 +17,9 @@
 
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/tpm_eventlog.h>
 
 #include "tpm.h"
-#include "tpm_eventlog.h"
 
 int tpm_read_log_of(struct tpm_chip *chip)
 {
diff --git a/drivers/char/tpm/tpm_eventlog.h b/include/linux/tpm_eventlog.h
similarity index 77%
rename from drivers/char/tpm/tpm_eventlog.h
rename to include/linux/tpm_eventlog.h
index b4b549559203..446656d1f317 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -1,6 +1,5 @@
-
-#ifndef __TPM_EVENTLOG_H__
-#define __TPM_EVENTLOG_H__
+#ifndef __LINUX_TPM_EVENTLOG_H__
+#define __LINUX_TPM_EVENTLOG_H__
 
 #include <crypto/hash_info.h>
 
@@ -104,6 +103,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 +116,4 @@ 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
-- 
2.14.1.581.gf28d330327-goog

[PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Thiebaud Weksteen]

With TPM 2.0 specification, the event logs may only be accessible by
calling an EFI Boot Service. Modify the EFI stub to copy the log area to
a new Linux-specific EFI configuration table so it remains accessible
once booted.

When calling this service, it is possible to specify the expected format
of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
first format is retrieved.

Signed-off-by: Thiebaud Weksteen <tweek@google.com>
---
 arch/x86/boot/compressed/eboot.c      |  1 +
 drivers/firmware/efi/Makefile         |  2 +-
 drivers/firmware/efi/efi.c            |  2 +
 drivers/firmware/efi/libstub/Makefile |  3 +-
 drivers/firmware/efi/libstub/tpm.c    | 81 +++++++++++++++++++++++++++++++++++
 drivers/firmware/efi/tpm.c            | 39 +++++++++++++++++
 include/linux/efi.h                   | 49 +++++++++++++++++++++
 7 files changed, 174 insertions(+), 3 deletions(-)
 create mode 100644 drivers/firmware/efi/tpm.c

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index a1686f3dc295..ef6abe8b3788 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -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(sys_table);
 
 	setup_graphics(boot_params);
 
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 0329d319d89a..2f074b5cde87 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -10,7 +10,7 @@
 KASAN_SANITIZE_runtime-wrappers.o	:= n
 
 obj-$(CONFIG_ACPI_BGRT) 		+= efi-bgrt.o
-obj-$(CONFIG_EFI)			+= efi.o vars.o reboot.o memattr.o
+obj-$(CONFIG_EFI)			+= efi.o vars.o reboot.o memattr.o tpm.o
 obj-$(CONFIG_EFI)			+= capsule.o memmap.o
 obj-$(CONFIG_EFI_VARS)			+= efivars.o
 obj-$(CONFIG_EFI_ESRT)			+= esrt.o
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index f97f272e16ee..03fbaf8eb248 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -532,6 +532,8 @@ int __init efi_config_parse_tables(void *config_tables, int count, int sz,
 	if (efi_enabled(EFI_MEMMAP))
 		efi_memattr_init();
 
+	efi_tpm_eventlog_init();
+
 	/* Parse the EFI Properties table if it exists */
 	if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
 		efi_properties_table_t *tbl;
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index dedf9bde44db..2abe6d22dc5f 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -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
diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index 6224cdbc9669..da661bf8cb96 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -4,15 +4,18 @@
  * Copyright (C) 2016 CoreOS, Inc
  * Copyright (C) 2017 Google, Inc.
  *     Matthew Garrett <mjg59@google.com>
+ *     Thiebaud Weksteen <tweek@google.com>
  *
  * This file is part of the Linux kernel, and is made available under the
  * terms of the GNU General Public License version 2.
  */
 #include <linux/efi.h>
+#include <linux/tpm_eventlog.h>
 #include <asm/efi.h>
 
 #include "efistub.h"
 
+#ifdef CONFIG_RESET_ATTACK_MITIGATION
 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,81 @@ 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(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;
+	unsigned long first_entry_addr, last_entry_addr;
+	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);
+	if (status != EFI_SUCCESS)
+		return;
+
+	status = efi_call_proto(efi_tcg2_protocol, get_event_log, tcg2_protocol,
+				EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2,
+				&log_location, &log_last_entry, &truncated);
+	if (status != EFI_SUCCESS)
+		return;
+
+	if (!log_location)
+		return;
+	first_entry_addr = (unsigned long) log_location;
+
+	/*
+	 * We populate the EFI table even if the logs are empty.
+	 */
+	if (!log_last_entry) {
+		log_size = 0;
+	} else {
+		last_entry_addr = (unsigned long) log_last_entry;
+		/*
+		 * get_event_log only returns the address of the last entry.
+		 * We need to calculate its size to deduce the full size of
+		 * the logs.
+		 */
+		last_entry_size = sizeof(struct tcpa_event) +
+			((struct tcpa_event *) last_entry_addr)->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_LOADER_DATA,
+				sizeof(*log_tbl) + log_size,
+				(void **) &log_tbl);
+
+	if (status != EFI_SUCCESS) {
+		efi_printk(sys_table_arg,
+			   "Unable to allocate memory for event log\n");
+		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 *) first_entry_addr, log_size);
+
+	status = efi_call_early(install_configuration_table,
+				&linux_eventlog_guid, log_tbl);
+	if (status != EFI_SUCCESS)
+		goto err_free;
+	return;
+
+err_free:
+	efi_call_early(free_pool, log_tbl);
+}
+
+void efi_retrieve_tpm2_eventlog(efi_system_table_t *sys_table_arg)
+{
+	/* Only try to retrieve the logs in 1.2 format. */
+	efi_retrieve_tpm2_eventlog_1_2(sys_table_arg);
+}
diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
new file mode 100644
index 000000000000..07d217d68f47
--- /dev/null
+++ b/drivers/firmware/efi/tpm.c
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2017 Google, Inc.
+ *     Thiebaud Weksteen <tweek@google.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/efi.h>
+#include <linux/init.h>
+#include <linux/memblock.h>
+
+#include <asm/early_ioremap.h>
+
+/*
+ * Reserve the memory associated with the TPM Event Log configuration table.
+ */
+int __init efi_tpm_eventlog_init(void)
+{
+	struct linux_efi_tpm_eventlog *tbl;
+	unsigned int tbl_size;
+
+	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
+		return 0;
+
+	tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
+	if (!tbl) {
+		pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
+			efi.tpm_log);
+		return -ENOMEM;
+	}
+
+	tbl_size = sizeof(*tbl) + tbl->size;
+	memblock_reserve(efi.tpm_log, tbl_size);
+	early_memunmap(tbl, sizeof(*tbl));
+	return 0;
+}
+
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 8dc3d94a3e3c..e671ea9a462e 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -472,6 +472,39 @@ 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;
+
+typedef u32 efi_tcg2_event_log_format;
+
+typedef struct {
+	void *get_capability;
+	efi_status_t (*get_event_log)(efi_handle_t, efi_tcg2_event_log_format,
+		efi_physical_addr_t *, efi_physical_addr_t *, efi_bool_t *);
+	void *hash_log_extend_event;
+	void *submit_command;
+	void *get_active_pcr_banks;
+	void *set_active_pcr_banks;
+	void *get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_t;
+
 /*
  * Types and defines for EFI ResetSystem
  */
@@ -622,6 +655,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 +668,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 +1539,8 @@ static inline void
 efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) { }
 #endif
 
+void efi_retrieve_tpm2_eventlog(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 +1608,16 @@ 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[];
+};
+
+extern int efi_tpm_eventlog_init(void);
+
 #endif /* _LINUX_EFI_H */
-- 
2.14.1.581.gf28d330327-goog

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3894 bytes --]

Hi Thiebaud,

[auto build test ERROR on efi/next]
[also build test ERROR on next-20170913]
[cannot apply to char-misc/char-misc-testing linus/master v4.13]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thiebaud-Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

Note: the linux-review/Thiebaud-Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312 HEAD 58dc8ee5de3bb5fb1ef216b80a76102d2de2b141 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/firmware/efi/tpm.c: In function 'efi_tpm_eventlog_init':
>> drivers/firmware/efi/tpm.c:24:9: error: 'struct efi' has no member named 'tpm_log'
     if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
            ^
   In file included from arch/ia64/include/asm/io.h:23:0,
                    from arch/ia64/include/asm/smp.h:20,
                    from include/linux/smp.h:59,
                    from include/linux/percpu.h:6,
                    from include/linux/percpu-rwsem.h:6,
                    from include/linux/fs.h:32,
                    from include/linux/proc_fs.h:8,
                    from include/linux/efi.h:18,
                    from drivers/firmware/efi/tpm.c:10:
   drivers/firmware/efi/tpm.c:27:26: error: 'struct efi' has no member named 'tpm_log'
     tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
                             ^
   arch/ia64/include/asm/early_ioremap.h:5:62: note: in definition of macro 'early_memremap'
    #define early_memremap(phys_addr, size)        early_ioremap(phys_addr, size)
                                                                 ^~~~~~~~~
   In file included from include/linux/kernel.h:13:0,
                    from include/linux/list.h:8,
                    from include/linux/preempt.h:10,
                    from include/linux/spinlock.h:50,
                    from include/linux/seqlock.h:35,
                    from include/linux/time.h:5,
                    from include/linux/efi.h:16,
                    from drivers/firmware/efi/tpm.c:10:
   drivers/firmware/efi/tpm.c:30:7: error: 'struct efi' has no member named 'tpm_log'
       efi.tpm_log);
          ^
   include/linux/printk.h:301:33: note: in definition of macro 'pr_err'
     printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
                                    ^~~~~~~~~~~
   drivers/firmware/efi/tpm.c:35:22: error: 'struct efi' has no member named 'tpm_log'
     memblock_reserve(efi.tpm_log, tbl_size);
                         ^

vim +24 drivers/firmware/efi/tpm.c

    15	
    16	/*
    17	 * Reserve the memory associated with the TPM Event Log configuration table.
    18	 */
    19	int __init efi_tpm_eventlog_init(void)
    20	{
    21		struct linux_efi_tpm_eventlog *tbl;
    22		unsigned int tbl_size;
    23	
  > 24		if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
    25			return 0;
    26	
    27		tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
    28		if (!tbl) {
    29			pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
    30				efi.tpm_log);
    31			return -ENOMEM;
    32		}
    33	
    34		tbl_size = sizeof(*tbl) + tbl->size;
    35		memblock_reserve(efi.tpm_log, tbl_size);
    36		early_memunmap(tbl, sizeof(*tbl));
    37		return 0;
    38	}
    39	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48397 bytes --]

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3894 bytes --]

Hi Thiebaud,

[auto build test ERROR on efi/next]
[also build test ERROR on next-20170913]
[cannot apply to char-misc/char-misc-testing linus/master v4.13]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thiebaud-Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

Note: the linux-review/Thiebaud-Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312 HEAD 58dc8ee5de3bb5fb1ef216b80a76102d2de2b141 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/firmware/efi/tpm.c: In function 'efi_tpm_eventlog_init':
>> drivers/firmware/efi/tpm.c:24:9: error: 'struct efi' has no member named 'tpm_log'
     if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
            ^
   In file included from arch/ia64/include/asm/io.h:23:0,
                    from arch/ia64/include/asm/smp.h:20,
                    from include/linux/smp.h:59,
                    from include/linux/percpu.h:6,
                    from include/linux/percpu-rwsem.h:6,
                    from include/linux/fs.h:32,
                    from include/linux/proc_fs.h:8,
                    from include/linux/efi.h:18,
                    from drivers/firmware/efi/tpm.c:10:
   drivers/firmware/efi/tpm.c:27:26: error: 'struct efi' has no member named 'tpm_log'
     tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
                             ^
   arch/ia64/include/asm/early_ioremap.h:5:62: note: in definition of macro 'early_memremap'
    #define early_memremap(phys_addr, size)        early_ioremap(phys_addr, size)
                                                                 ^~~~~~~~~
   In file included from include/linux/kernel.h:13:0,
                    from include/linux/list.h:8,
                    from include/linux/preempt.h:10,
                    from include/linux/spinlock.h:50,
                    from include/linux/seqlock.h:35,
                    from include/linux/time.h:5,
                    from include/linux/efi.h:16,
                    from drivers/firmware/efi/tpm.c:10:
   drivers/firmware/efi/tpm.c:30:7: error: 'struct efi' has no member named 'tpm_log'
       efi.tpm_log);
          ^
   include/linux/printk.h:301:33: note: in definition of macro 'pr_err'
     printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
                                    ^~~~~~~~~~~
   drivers/firmware/efi/tpm.c:35:22: error: 'struct efi' has no member named 'tpm_log'
     memblock_reserve(efi.tpm_log, tbl_size);
                         ^

vim +24 drivers/firmware/efi/tpm.c

    15	
    16	/*
    17	 * Reserve the memory associated with the TPM Event Log configuration table.
    18	 */
    19	int __init efi_tpm_eventlog_init(void)
    20	{
    21		struct linux_efi_tpm_eventlog *tbl;
    22		unsigned int tbl_size;
    23	
  > 24		if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
    25			return 0;
    26	
    27		tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
    28		if (!tbl) {
    29			pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
    30				efi.tpm_log);
    31			return -ENOMEM;
    32		}
    33	
    34		tbl_size = sizeof(*tbl) + tbl->size;
    35		memblock_reserve(efi.tpm_log, tbl_size);
    36		early_memunmap(tbl, sizeof(*tbl));
    37		return 0;
    38	}
    39	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48397 bytes --]

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Thiebaud Weksteen via tpmdd-devel]

[-- Attachment #1.1: Type: text/plain, Size: 4482 bytes --]

efi_tpm_eventlog_init should be in patch 0003 and not 0002.
I'll send a new version of the patch set once I've received more feedback.

On Wed, Sep 13, 2017 at 4:49 PM, kbuild test robot <lkp-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> Hi Thiebaud,
>
> [auto build test ERROR on efi/next]
> [also build test ERROR on next-20170913]
> [cannot apply to char-misc/char-misc-testing linus/master v4.13]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Thiebaud-
> Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
> config: ia64-allmodconfig (attached as .config)
> compiler: ia64-linux-gcc (GCC) 6.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/
> sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=ia64
>
> Note: the linux-review/Thiebaud-Weksteen/Call-GetEventLog-
> before-ExitBootServices/20170913-221312 HEAD
> 58dc8ee5de3bb5fb1ef216b80a76102d2de2b141 builds fine.
>       It only hurts bisectibility.
>
> All errors (new ones prefixed by >>):
>
>    drivers/firmware/efi/tpm.c: In function 'efi_tpm_eventlog_init':
> >> drivers/firmware/efi/tpm.c:24:9: error: 'struct efi' has no member
> named 'tpm_log'
>      if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>             ^
>    In file included from arch/ia64/include/asm/io.h:23:0,
>                     from arch/ia64/include/asm/smp.h:20,
>                     from include/linux/smp.h:59,
>                     from include/linux/percpu.h:6,
>                     from include/linux/percpu-rwsem.h:6,
>                     from include/linux/fs.h:32,
>                     from include/linux/proc_fs.h:8,
>                     from include/linux/efi.h:18,
>                     from drivers/firmware/efi/tpm.c:10:
>    drivers/firmware/efi/tpm.c:27:26: error: 'struct efi' has no member
> named 'tpm_log'
>      tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
>                              ^
>    arch/ia64/include/asm/early_ioremap.h:5:62: note: in definition of
> macro 'early_memremap'
>     #define early_memremap(phys_addr, size)
> early_ioremap(phys_addr, size)
>                                                                  ^~~~~~~~~
>    In file included from include/linux/kernel.h:13:0,
>                     from include/linux/list.h:8,
>                     from include/linux/preempt.h:10,
>                     from include/linux/spinlock.h:50,
>                     from include/linux/seqlock.h:35,
>                     from include/linux/time.h:5,
>                     from include/linux/efi.h:16,
>                     from drivers/firmware/efi/tpm.c:10:
>    drivers/firmware/efi/tpm.c:30:7: error: 'struct efi' has no member
> named 'tpm_log'
>        efi.tpm_log);
>           ^
>    include/linux/printk.h:301:33: note: in definition of macro 'pr_err'
>      printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
>                                     ^~~~~~~~~~~
>    drivers/firmware/efi/tpm.c:35:22: error: 'struct efi' has no member
> named 'tpm_log'
>      memblock_reserve(efi.tpm_log, tbl_size);
>                          ^
>
> vim +24 drivers/firmware/efi/tpm.c
>
>     15
>     16  /*
>     17   * Reserve the memory associated with the TPM Event Log
> configuration table.
>     18   */
>     19  int __init efi_tpm_eventlog_init(void)
>     20  {
>     21          struct linux_efi_tpm_eventlog *tbl;
>     22          unsigned int tbl_size;
>     23
>   > 24          if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>     25                  return 0;
>     26
>     27          tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
>     28          if (!tbl) {
>     29                  pr_err("Failed to map TPM Event Log table @
> 0x%lx\n",
>     30                          efi.tpm_log);
>     31                  return -ENOMEM;
>     32          }
>     33
>     34          tbl_size = sizeof(*tbl) + tbl->size;
>     35          memblock_reserve(efi.tpm_log, tbl_size);
>     36          early_memunmap(tbl, sizeof(*tbl));
>     37          return 0;
>     38  }
>     39
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology
> Center
> https://lists.01.org/pipermail/kbuild-all                   Intel
> Corporation
>

[-- Attachment #1.2: Type: text/html, Size: 6206 bytes --]

[-- Attachment #2: Type: text/plain, Size: 202 bytes --]

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

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Javier Martinez Canillas]

On 09/11/2017 12:00 PM, Thiebaud Weksteen via tpmdd-devel wrote:
> With TPM 2.0 specification, the event logs may only be accessible by
> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> a new Linux-specific EFI configuration table so it remains accessible
> once booted.
> 
> When calling this service, it is possible to specify the expected format
> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> first format is retrieved.
> 
> Signed-off-by: Thiebaud Weksteen <tweek@google.com>
> ---

[snip]

> +void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
> +{

[snip]

> +
> +	/* Allocate space for the logs and copy them. */
> +	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
> +				sizeof(*log_tbl) + log_size,
> +				(void **) &log_tbl);
> +
> +	if (status != EFI_SUCCESS) {
> +		efi_printk(sys_table_arg,
> +			   "Unable to allocate memory for event log\n");
> +		return;
> +	}

If this fails or any previous error that will prevent the event log table + logs
to be allocated, shouldn't tpm_read_log_efi() be notified somehow? Since AFAICT
it will still try to access them even if the EFI allocate_pool did not succeed.

> + */
> +int __init efi_tpm_eventlog_init(void)
> +{
> +	struct linux_efi_tpm_eventlog *tbl;
> +	unsigned int tbl_size;
> +

The functions efi_retrieve_tpm2_eventlog_1_2() and tpm_read_log_efi() are using
log_tbl as variable name, so I would use it here too for consistency.

> +	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
> +		return 0;
> +
> +	tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
> +	if (!tbl) {
> +		pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
> +			efi.tpm_log);
> +		return -ENOMEM;
> +	}
> +

Same question than before, if this fails then the table + logs memory won't be
reserved but tpm_read_log_efi() will still try to access it. I'm not sure what
is the correct way to notify though, maybe setting efi.tpm_log to 0 and then in
tpm_read_log_efi() check efi.tpm_log for 0 or EFI_INVALID_TABLE_ADDR instead?

> +	tbl_size = sizeof(*tbl) + tbl->size;
> +	memblock_reserve(efi.tpm_log, tbl_size);
> +	early_memunmap(tbl, sizeof(*tbl));
> +	return 0;

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [tpmdd-devel] [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Javier Martinez Canillas]

On 09/11/2017 12:00 PM, Thiebaud Weksteen via tpmdd-devel wrote:
> With TPM 2.0 specification, the event logs may only be accessible by
> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> a new Linux-specific EFI configuration table so it remains accessible
> once booted.
> 
> When calling this service, it is possible to specify the expected format
> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> first format is retrieved.
> 
> Signed-off-by: Thiebaud Weksteen <tweek-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---

[snip]

> +void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
> +{

[snip]

> +
> +	/* Allocate space for the logs and copy them. */
> +	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
> +				sizeof(*log_tbl) + log_size,
> +				(void **) &log_tbl);
> +
> +	if (status != EFI_SUCCESS) {
> +		efi_printk(sys_table_arg,
> +			   "Unable to allocate memory for event log\n");
> +		return;
> +	}

If this fails or any previous error that will prevent the event log table + logs
to be allocated, shouldn't tpm_read_log_efi() be notified somehow? Since AFAICT
it will still try to access them even if the EFI allocate_pool did not succeed.

> + */
> +int __init efi_tpm_eventlog_init(void)
> +{
> +	struct linux_efi_tpm_eventlog *tbl;
> +	unsigned int tbl_size;
> +

The functions efi_retrieve_tpm2_eventlog_1_2() and tpm_read_log_efi() are using
log_tbl as variable name, so I would use it here too for consistency.

> +	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
> +		return 0;
> +
> +	tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
> +	if (!tbl) {
> +		pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
> +			efi.tpm_log);
> +		return -ENOMEM;
> +	}
> +

Same question than before, if this fails then the table + logs memory won't be
reserved but tpm_read_log_efi() will still try to access it. I'm not sure what
is the correct way to notify though, maybe setting efi.tpm_log to 0 and then in
tpm_read_log_efi() check efi.tpm_log for 0 or EFI_INVALID_TABLE_ADDR instead?

> +	tbl_size = sizeof(*tbl) + tbl->size;
> +	memblock_reserve(efi.tpm_log, tbl_size);
> +	early_memunmap(tbl, sizeof(*tbl));
> +	return 0;

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [tpmdd-devel] [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Thiebaud Weksteen]

On Thu, Sep 14, 2017 at 12:24 PM, Javier Martinez Canillas
<javierm@redhat.com> wrote:
> On 09/11/2017 12:00 PM, Thiebaud Weksteen via tpmdd-devel wrote:
>> With TPM 2.0 specification, the event logs may only be accessible by
>> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
>> a new Linux-specific EFI configuration table so it remains accessible
>> once booted.
>>
>> When calling this service, it is possible to specify the expected format
>> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
>> first format is retrieved.
>>
>> Signed-off-by: Thiebaud Weksteen <tweek@google.com>
>> ---
>
> [snip]
>
>> +void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
>> +{
>
> [snip]
>
>> +
>> +     /* Allocate space for the logs and copy them. */
>> +     status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
>> +                             sizeof(*log_tbl) + log_size,
>> +                             (void **) &log_tbl);
>> +
>> +     if (status != EFI_SUCCESS) {
>> +             efi_printk(sys_table_arg,
>> +                        "Unable to allocate memory for event log\n");
>> +             return;
>> +     }
>
> If this fails or any previous error that will prevent the event log table + logs
> to be allocated, shouldn't tpm_read_log_efi() be notified somehow? Since AFAICT
> it will still try to access them even if the EFI allocate_pool did not succeed.
>

The implicit part that covers this case is in
drivers/firmware/efi/efi.c. The match_config_table function will go
through all the installed configuration tables and only fill up the
associated member of the efi structure if it exists. In this case,
.tpm_log will remains at EFI_INVALID_TABLE_ADDR unless
efi_call_early(install_configuration_table, ...) is called. So no
further processing is to be expected should the allocation failed.

>> + */
>> +int __init efi_tpm_eventlog_init(void)
>> +{
>> +     struct linux_efi_tpm_eventlog *tbl;
>> +     unsigned int tbl_size;
>> +
>
> The functions efi_retrieve_tpm2_eventlog_1_2() and tpm_read_log_efi() are using
> log_tbl as variable name, so I would use it here too for consistency.
>

Done.

>> +     if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>> +             return 0;
>> +
>> +     tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
>> +     if (!tbl) {
>> +             pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
>> +                     efi.tpm_log);
>> +             return -ENOMEM;
>> +     }
>> +
>
> Same question than before, if this fails then the table + logs memory won't be
> reserved but tpm_read_log_efi() will still try to access it. I'm not sure what
> is the correct way to notify though, maybe setting efi.tpm_log to 0 and then in
> tpm_read_log_efi() check efi.tpm_log for 0 or EFI_INVALID_TABLE_ADDR instead?
>

That's right. To keep it simple, it might be easier to just set
tpm_log to EFI_INVALID_TABLE_ADDR if that happens. Added in the next
version of the patch set.

>> +     tbl_size = sizeof(*tbl) + tbl->size;
>> +     memblock_reserve(efi.tpm_log, tbl_size);
>> +     early_memunmap(tbl, sizeof(*tbl));
>> +     return 0;
>
> Best regards,
> --
> Javier Martinez Canillas
> Software Engineer - Desktop Hardware Enablement
> Red Hat

Re: [tpmdd-devel] [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Thiebaud Weksteen]

On Thu, Sep 14, 2017 at 12:24 PM, Javier Martinez Canillas
<javierm-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 09/11/2017 12:00 PM, Thiebaud Weksteen via tpmdd-devel wrote:
>> With TPM 2.0 specification, the event logs may only be accessible by
>> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
>> a new Linux-specific EFI configuration table so it remains accessible
>> once booted.
>>
>> When calling this service, it is possible to specify the expected format
>> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
>> first format is retrieved.
>>
>> Signed-off-by: Thiebaud Weksteen <tweek-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> ---
>
> [snip]
>
>> +void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
>> +{
>
> [snip]
>
>> +
>> +     /* Allocate space for the logs and copy them. */
>> +     status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
>> +                             sizeof(*log_tbl) + log_size,
>> +                             (void **) &log_tbl);
>> +
>> +     if (status != EFI_SUCCESS) {
>> +             efi_printk(sys_table_arg,
>> +                        "Unable to allocate memory for event log\n");
>> +             return;
>> +     }
>
> If this fails or any previous error that will prevent the event log table + logs
> to be allocated, shouldn't tpm_read_log_efi() be notified somehow? Since AFAICT
> it will still try to access them even if the EFI allocate_pool did not succeed.
>

The implicit part that covers this case is in
drivers/firmware/efi/efi.c. The match_config_table function will go
through all the installed configuration tables and only fill up the
associated member of the efi structure if it exists. In this case,
.tpm_log will remains at EFI_INVALID_TABLE_ADDR unless
efi_call_early(install_configuration_table, ...) is called. So no
further processing is to be expected should the allocation failed.

>> + */
>> +int __init efi_tpm_eventlog_init(void)
>> +{
>> +     struct linux_efi_tpm_eventlog *tbl;
>> +     unsigned int tbl_size;
>> +
>
> The functions efi_retrieve_tpm2_eventlog_1_2() and tpm_read_log_efi() are using
> log_tbl as variable name, so I would use it here too for consistency.
>

Done.

>> +     if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>> +             return 0;
>> +
>> +     tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
>> +     if (!tbl) {
>> +             pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
>> +                     efi.tpm_log);
>> +             return -ENOMEM;
>> +     }
>> +
>
> Same question than before, if this fails then the table + logs memory won't be
> reserved but tpm_read_log_efi() will still try to access it. I'm not sure what
> is the correct way to notify though, maybe setting efi.tpm_log to 0 and then in
> tpm_read_log_efi() check efi.tpm_log for 0 or EFI_INVALID_TABLE_ADDR instead?
>

That's right. To keep it simple, it might be easier to just set
tpm_log to EFI_INVALID_TABLE_ADDR if that happens. Added in the next
version of the patch set.

>> +     tbl_size = sizeof(*tbl) + tbl->size;
>> +     memblock_reserve(efi.tpm_log, tbl_size);
>> +     early_memunmap(tbl, sizeof(*tbl));
>> +     return 0;
>
> Best regards,
> --
> Javier Martinez Canillas
> Software Engineer - Desktop Hardware Enablement
> Red Hat

Re: [tpmdd-devel] [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Javier Martinez Canillas]

On 09/18/2017 02:11 PM, Thiebaud Weksteen wrote:
> On Thu, Sep 14, 2017 at 12:24 PM, Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> On 09/11/2017 12:00 PM, Thiebaud Weksteen via tpmdd-devel wrote:

[snip]

>>> +
>>> +     if (status != EFI_SUCCESS) {
>>> +             efi_printk(sys_table_arg,
>>> +                        "Unable to allocate memory for event log\n");
>>> +             return;
>>> +     }
>>
>> If this fails or any previous error that will prevent the event log table + logs
>> to be allocated, shouldn't tpm_read_log_efi() be notified somehow? Since AFAICT
>> it will still try to access them even if the EFI allocate_pool did not succeed.
>>
> 
> The implicit part that covers this case is in
> drivers/firmware/efi/efi.c. The match_config_table function will go
> through all the installed configuration tables and only fill up the
> associated member of the efi structure if it exists. In this case,
> .tpm_log will remains at EFI_INVALID_TABLE_ADDR unless
> efi_call_early(install_configuration_table, ...) is called. So no
> further processing is to be expected should the allocation failed.
>

I see, missed that. Thanks a lot for the explanation.

>>> + */
>>> +int __init efi_tpm_eventlog_init(void)
>>> +{
>>> +     struct linux_efi_tpm_eventlog *tbl;
>>> +     unsigned int tbl_size;
>>> +
>>
>> The functions efi_retrieve_tpm2_eventlog_1_2() and tpm_read_log_efi() are using
>> log_tbl as variable name, so I would use it here too for consistency.
>>
> 
> Done.
> 
>>> +     if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>>> +             return 0;
>>> +
>>> +     tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
>>> +     if (!tbl) {
>>> +             pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
>>> +                     efi.tpm_log);
>>> +             return -ENOMEM;
>>> +     }
>>> +
>>
>> Same question than before, if this fails then the table + logs memory won't be
>> reserved but tpm_read_log_efi() will still try to access it. I'm not sure what
>> is the correct way to notify though, maybe setting efi.tpm_log to 0 and then in
>> tpm_read_log_efi() check efi.tpm_log for 0 or EFI_INVALID_TABLE_ADDR instead?
>>
> 
> That's right. To keep it simple, it might be easier to just set
> tpm_log to EFI_INVALID_TABLE_ADDR if that happens. Added in the next
> version of the patch set.
>

Right. I wasn't sure if you wanted to distinguish between the two cases, but that
is simpler indeed.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> With TPM 2.0 specification, the event logs may only be accessible by
> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> a new Linux-specific EFI configuration table so it remains accessible
> once booted.
> 
> When calling this service, it is possible to specify the expected format
> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> first format is retrieved.
> 
> Signed-off-by: Thiebaud Weksteen <tweek@google.com>

With a quick skim the code change looks good but I remember from
Matthew's talk that there was this issue that ExitBootServices() would
cause a yet another event?

I guess you could manually synthetize that event by reading the PCR
values right after ExitBootServices()?

Anyway, great work, thanks for making this effort.

/Jarkko

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> With TPM 2.0 specification, the event logs may only be accessible by
> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> a new Linux-specific EFI configuration table so it remains accessible
> once booted.
> 
> When calling this service, it is possible to specify the expected format
> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> first format is retrieved.
> 
> Signed-off-by: Thiebaud Weksteen <tweek-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

With a quick skim the code change looks good but I remember from
Matthew's talk that there was this issue that ExitBootServices() would
cause a yet another event?

I guess you could manually synthetize that event by reading the PCR
values right after ExitBootServices()?

Anyway, great work, thanks for making this effort.

/Jarkko

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Matthew Garrett]

On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
> On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
>> With TPM 2.0 specification, the event logs may only be accessible by
>> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
>> a new Linux-specific EFI configuration table so it remains accessible
>> once booted.
>>
>> When calling this service, it is possible to specify the expected format
>> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
>> first format is retrieved.
>>
>> Signed-off-by: Thiebaud Weksteen <tweek@google.com>
>
> With a quick skim the code change looks good but I remember from
> Matthew's talk that there was this issue that ExitBootServices() would
> cause a yet another event?
>
> I guess you could manually synthetize that event by reading the PCR
> values right after ExitBootServices()?

I think that would involve breaking SHA1… the information should be
available in the TCG_TREE_FINAL_EVENTS configuration table, so it
/should/ just be a matter of merging the events from that into the
event log.

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Matthew Garrett]

On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
<jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
>> With TPM 2.0 specification, the event logs may only be accessible by
>> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
>> a new Linux-specific EFI configuration table so it remains accessible
>> once booted.
>>
>> When calling this service, it is possible to specify the expected format
>> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
>> first format is retrieved.
>>
>> Signed-off-by: Thiebaud Weksteen <tweek-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>
> With a quick skim the code change looks good but I remember from
> Matthew's talk that there was this issue that ExitBootServices() would
> cause a yet another event?
>
> I guess you could manually synthetize that event by reading the PCR
> values right after ExitBootServices()?

I think that would involve breaking SHA1… the information should be
available in the TCG_TREE_FINAL_EVENTS configuration table, so it
/should/ just be a matter of merging the events from that into the
event log.

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Thu, Sep 14, 2017 at 11:48:54AM -0700, Matthew Garrett wrote:
> On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> > On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> >> With TPM 2.0 specification, the event logs may only be accessible by
> >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> >> a new Linux-specific EFI configuration table so it remains accessible
> >> once booted.
> >>
> >> When calling this service, it is possible to specify the expected format
> >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> >> first format is retrieved.
> >>
> >> Signed-off-by: Thiebaud Weksteen <tweek@google.com>
> >
> > With a quick skim the code change looks good but I remember from
> > Matthew's talk that there was this issue that ExitBootServices() would
> > cause a yet another event?
> >
> > I guess you could manually synthetize that event by reading the PCR
> > values right after ExitBootServices()?
> 
> I think that would involve breaking SHA1… the information should be

You are absolutely right, was not thinking clearly :-)

> available in the TCG_TREE_FINAL_EVENTS configuration table, so it
> /should/ just be a matter of merging the events from that into the
> event log.

Right, it is available through runtime services. Why this isn't part
of the patch set?

/Jrakko

/Jarkko

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Thu, Sep 14, 2017 at 12:02:47PM -0700, Jarkko Sakkinen wrote:
> On Thu, Sep 14, 2017 at 11:48:54AM -0700, Matthew Garrett wrote:
> > On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > > On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> > >> With TPM 2.0 specification, the event logs may only be accessible by
> > >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> > >> a new Linux-specific EFI configuration table so it remains accessible
> > >> once booted.
> > >>
> > >> When calling this service, it is possible to specify the expected format
> > >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> > >> first format is retrieved.
> > >>
> > >> Signed-off-by: Thiebaud Weksteen <tweek@google.com>
> > >
> > > With a quick skim the code change looks good but I remember from
> > > Matthew's talk that there was this issue that ExitBootServices() would
> > > cause a yet another event?
> > >
> > > I guess you could manually synthetize that event by reading the PCR
> > > values right after ExitBootServices()?
> > 
> > I think that would involve breaking SHA1… the information should be
> 
> You are absolutely right, was not thinking clearly :-)
> 
> > available in the TCG_TREE_FINAL_EVENTS configuration table, so it
> > /should/ just be a matter of merging the events from that into the
> > event log.
> 
> Right, it is available through runtime services. Why this isn't part
> of the patch set?

Anyway, I'll try this out out when I get back to Finland. Still before
landing this to mainline I think it would make sense to make it complete
wouldn't it?

/Jarkko

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Thu, Sep 14, 2017 at 12:02:47PM -0700, Jarkko Sakkinen wrote:
> On Thu, Sep 14, 2017 at 11:48:54AM -0700, Matthew Garrett wrote:
> > On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
> > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> > > On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> > >> With TPM 2.0 specification, the event logs may only be accessible by
> > >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> > >> a new Linux-specific EFI configuration table so it remains accessible
> > >> once booted.
> > >>
> > >> When calling this service, it is possible to specify the expected format
> > >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> > >> first format is retrieved.
> > >>
> > >> Signed-off-by: Thiebaud Weksteen <tweek-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > >
> > > With a quick skim the code change looks good but I remember from
> > > Matthew's talk that there was this issue that ExitBootServices() would
> > > cause a yet another event?
> > >
> > > I guess you could manually synthetize that event by reading the PCR
> > > values right after ExitBootServices()?
> > 
> > I think that would involve breaking SHA1… the information should be
> 
> You are absolutely right, was not thinking clearly :-)
> 
> > available in the TCG_TREE_FINAL_EVENTS configuration table, so it
> > /should/ just be a matter of merging the events from that into the
> > event log.
> 
> Right, it is available through runtime services. Why this isn't part
> of the patch set?

Anyway, I'll try this out out when I get back to Finland. Still before
landing this to mainline I think it would make sense to make it complete
wouldn't it?

/Jarkko

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Thiebaud Weksteen]

On Thu, Sep 14, 2017 at 9:02 PM, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
> On Thu, Sep 14, 2017 at 11:48:54AM -0700, Matthew Garrett wrote:
>> On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
>> <jarkko.sakkinen@linux.intel.com> wrote:
>> > On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
>> >> With TPM 2.0 specification, the event logs may only be accessible by
>> >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
>> >> a new Linux-specific EFI configuration table so it remains accessible
>> >> once booted.
>> >>
>> >> When calling this service, it is possible to specify the expected format
>> >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
>> >> first format is retrieved.
>> >>
>> >> Signed-off-by: Thiebaud Weksteen <tweek@google.com>
>> >
>> > With a quick skim the code change looks good but I remember from
>> > Matthew's talk that there was this issue that ExitBootServices() would
>> > cause a yet another event?
>> >
>> > I guess you could manually synthetize that event by reading the PCR
>> > values right after ExitBootServices()?
>>
>> I think that would involve breaking SHA1… the information should be
>
> You are absolutely right, was not thinking clearly :-)
>
>> available in the TCG_TREE_FINAL_EVENTS configuration table, so it
>> /should/ just be a matter of merging the events from that into the
>> event log.
>
> Right, it is available through runtime services. Why this isn't part
> of the patch set?

This is not included yet as this table
(EFI_TCG2_FINAL_EVENTS_TABLE_GUID) relies on the TPM2 format for the
log entries (TCG_PCR_EVENT2, "Crypto Agile"). I first plan to add the
parsing of this log version (ie, efi_retrieve_tpm2_eventlog_2) before
adding the merging of both tables. But these will be separate patch
sets.

>
> /Jrakko
>
> /Jarkko

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Thiebaud Weksteen]

On Thu, Sep 14, 2017 at 9:02 PM, Jarkko Sakkinen
<jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On Thu, Sep 14, 2017 at 11:48:54AM -0700, Matthew Garrett wrote:
>> On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
>> <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>> > On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
>> >> With TPM 2.0 specification, the event logs may only be accessible by
>> >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
>> >> a new Linux-specific EFI configuration table so it remains accessible
>> >> once booted.
>> >>
>> >> When calling this service, it is possible to specify the expected format
>> >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
>> >> first format is retrieved.
>> >>
>> >> Signed-off-by: Thiebaud Weksteen <tweek-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> >
>> > With a quick skim the code change looks good but I remember from
>> > Matthew's talk that there was this issue that ExitBootServices() would
>> > cause a yet another event?
>> >
>> > I guess you could manually synthetize that event by reading the PCR
>> > values right after ExitBootServices()?
>>
>> I think that would involve breaking SHA1… the information should be
>
> You are absolutely right, was not thinking clearly :-)
>
>> available in the TCG_TREE_FINAL_EVENTS configuration table, so it
>> /should/ just be a matter of merging the events from that into the
>> event log.
>
> Right, it is available through runtime services. Why this isn't part
> of the patch set?

This is not included yet as this table
(EFI_TCG2_FINAL_EVENTS_TABLE_GUID) relies on the TPM2 format for the
log entries (TCG_PCR_EVENT2, "Crypto Agile"). I first plan to add the
parsing of this log version (ie, efi_retrieve_tpm2_eventlog_2) before
adding the merging of both tables. But these will be separate patch
sets.

>
> /Jrakko
>
> /Jarkko

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 18, 2017 at 02:28:45PM +0200, Thiebaud Weksteen wrote:
> On Thu, Sep 14, 2017 at 9:02 PM, Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> > On Thu, Sep 14, 2017 at 11:48:54AM -0700, Matthew Garrett wrote:
> >> On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
> >> <jarkko.sakkinen@linux.intel.com> wrote:
> >> > On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> >> >> With TPM 2.0 specification, the event logs may only be accessible by
> >> >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> >> >> a new Linux-specific EFI configuration table so it remains accessible
> >> >> once booted.
> >> >>
> >> >> When calling this service, it is possible to specify the expected format
> >> >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> >> >> first format is retrieved.
> >> >>
> >> >> Signed-off-by: Thiebaud Weksteen <tweek@google.com>
> >> >
> >> > With a quick skim the code change looks good but I remember from
> >> > Matthew's talk that there was this issue that ExitBootServices() would
> >> > cause a yet another event?
> >> >
> >> > I guess you could manually synthetize that event by reading the PCR
> >> > values right after ExitBootServices()?
> >>
> >> I think that would involve breaking SHA1… the information should be
> >
> > You are absolutely right, was not thinking clearly :-)
> >
> >> available in the TCG_TREE_FINAL_EVENTS configuration table, so it
> >> /should/ just be a matter of merging the events from that into the
> >> event log.
> >
> > Right, it is available through runtime services. Why this isn't part
> > of the patch set?
> 
> This is not included yet as this table
> (EFI_TCG2_FINAL_EVENTS_TABLE_GUID) relies on the TPM2 format for the
> log entries (TCG_PCR_EVENT2, "Crypto Agile"). I first plan to add the
> parsing of this log version (ie, efi_retrieve_tpm2_eventlog_2) before
> adding the merging of both tables. But these will be separate patch
> sets.

OK, this should be documented to the commit message to make it clear.

linux-integrity@vger.kernel.org is now up and running. I'm still
surviving from jetlag etc. so testing might be postponed either near end
of the week or next week.

Thanks for doing this. This is really important stuff in order to get the
Linux TPM 2.0 support feature complete.

/Jarkko

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 18, 2017 at 02:28:45PM +0200, Thiebaud Weksteen wrote:
> On Thu, Sep 14, 2017 at 9:02 PM, Jarkko Sakkinen
> <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> > On Thu, Sep 14, 2017 at 11:48:54AM -0700, Matthew Garrett wrote:
> >> On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
> >> <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> >> > On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> >> >> With TPM 2.0 specification, the event logs may only be accessible by
> >> >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> >> >> a new Linux-specific EFI configuration table so it remains accessible
> >> >> once booted.
> >> >>
> >> >> When calling this service, it is possible to specify the expected format
> >> >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> >> >> first format is retrieved.
> >> >>
> >> >> Signed-off-by: Thiebaud Weksteen <tweek-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> >> >
> >> > With a quick skim the code change looks good but I remember from
> >> > Matthew's talk that there was this issue that ExitBootServices() would
> >> > cause a yet another event?
> >> >
> >> > I guess you could manually synthetize that event by reading the PCR
> >> > values right after ExitBootServices()?
> >>
> >> I think that would involve breaking SHA1… the information should be
> >
> > You are absolutely right, was not thinking clearly :-)
> >
> >> available in the TCG_TREE_FINAL_EVENTS configuration table, so it
> >> /should/ just be a matter of merging the events from that into the
> >> event log.
> >
> > Right, it is available through runtime services. Why this isn't part
> > of the patch set?
> 
> This is not included yet as this table
> (EFI_TCG2_FINAL_EVENTS_TABLE_GUID) relies on the TPM2 format for the
> log entries (TCG_PCR_EVENT2, "Crypto Agile"). I first plan to add the
> parsing of this log version (ie, efi_retrieve_tpm2_eventlog_2) before
> adding the merging of both tables. But these will be separate patch
> sets.

OK, this should be documented to the commit message to make it clear.

linux-integrity-u79uwXL29TY76Z2rM5mHXA@public.gmane.org is now up and running. I'm still
surviving from jetlag etc. so testing might be postponed either near end
of the week or next week.

Thanks for doing this. This is really important stuff in order to get the
Linux TPM 2.0 support feature complete.

/Jarkko

[PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Thiebaud Weksteen]

If we are not able to retrieve the TPM event logs from the ACPI table,
check the EFI configuration table (Linux-specific GUID).

The format version of the log may be returned by the function. If not
specified (by previous implementation: tpm_acpi and tpm_of), we default
to the version of the chip (previous behaviour).

Signed-off-by: Thiebaud Weksteen <tweek@google.com>
---
 drivers/char/tpm/Makefile        |  2 +-
 drivers/char/tpm/tpm.h           |  8 +++++
 drivers/char/tpm/tpm1_eventlog.c | 15 +++++++--
 drivers/char/tpm/tpm_efi.c       | 66 ++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/efi/efi.c       |  2 ++
 include/linux/efi.h              |  1 +
 6 files changed, 90 insertions(+), 4 deletions(-)
 create mode 100644 drivers/char/tpm/tpm_efi.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 23681f01f95a..74182a63eef2 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -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
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 46caccf6fd1a..1bd97e01df50 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -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);
diff --git a/drivers/char/tpm/tpm1_eventlog.c b/drivers/char/tpm/tpm1_eventlog.c
index d6f70f365443..7e25e6bff6ce 100644
--- a/drivers/char/tpm/tpm1_eventlog.c
+++ b/drivers/char/tpm/tpm1_eventlog.c
@@ -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>
@@ -371,6 +372,10 @@ static int tpm_read_log(struct tpm_chip *chip)
 	if (rc != -ENODEV)
 		return rc;
 
+	rc = tpm_read_log_efi(chip);
+	if (rc != -ENODEV)
+		return rc;
+
 	return tpm_read_log_of(chip);
 }
 
@@ -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,7 +411,9 @@ 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
diff --git a/drivers/char/tpm/tpm_efi.c b/drivers/char/tpm/tpm_efi.c
new file mode 100644
index 000000000000..c8247fc45bb0
--- /dev/null
+++ b/drivers/char/tpm/tpm_efi.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2017 Google
+ *
+ * Authors:
+ *      Thiebaud Weksteen <tweek@google.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * 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 <linux/tpm_eventlog.h>
+
+#include "tpm.h"
+
+/* 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;
+
+	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+		return -ENODEV;
+
+	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
+		return -ENODEV;
+
+	log = &chip->log;
+
+	log_tbl = memremap(efi.tpm_log, sizeof(*log_tbl), MEMREMAP_WB);
+	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 = memremap(efi.tpm_log, sizeof(*log_tbl) + log_size,
+			   MEMREMAP_WB);
+	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;
+}
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 03fbaf8eb248..0308acfaaf76 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -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},
 };
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index e671ea9a462e..ab78950e6911 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -943,6 +943,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

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Jason Gunthorpe]

On Mon, Sep 11, 2017 at 12:00:22PM +0200, Thiebaud Weksteen wrote:
  
>  	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;

Lets have all the read_log_* versions return the postitive log_version
and get rid of the chip->flags check here.

ie Doesn't ACPI always return the TPM 1 version?

Jason

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Jason Gunthorpe]

On Mon, Sep 11, 2017 at 12:00:22PM +0200, Thiebaud Weksteen wrote:
  
>  	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;

Lets have all the read_log_* versions return the postitive log_version
and get rid of the chip->flags check here.

ie Doesn't ACPI always return the TPM 1 version?

Jason

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Thiebaud Weksteen]

On Mon, Sep 11, 2017 at 10:47:50AM -0600, Jason Gunthorpe wrote:
> On Mon, Sep 11, 2017 at 12:00:22PM +0200, Thiebaud Weksteen wrote:
>   
> >  	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;
> 
> Lets have all the read_log_* versions return the postitive log_version
> and get rid of the chip->flags check here.
> 
> ie Doesn't ACPI always return the TPM 1 version?

That is my understanding. Ashley, Nayna, could you confirm the format
version expected by tpm_of? Could it be both?

> 
> Jason

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Thiebaud Weksteen]

On Tue, Sep 12, 2017 at 10:48 AM, Thiebaud Weksteen <tweek@google.com> wrote:
> On Mon, Sep 11, 2017 at 10:47:50AM -0600, Jason Gunthorpe wrote:
>> On Mon, Sep 11, 2017 at 12:00:22PM +0200, Thiebaud Weksteen wrote:
>>
>> >     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;
>>
>> Lets have all the read_log_* versions return the postitive log_version
>> and get rid of the chip->flags check here.
>>
>> ie Doesn't ACPI always return the TPM 1 version?
>
> That is my understanding. Ashley, Nayna, could you confirm the format
> version expected by tpm_of? Could it be both?
>

I've changed the returned code for ACPI but not for DeviceTree.
Without confirmation for tpm_of, I am reluctant to modify the current
behaviour.

>>
>> Jason

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Thiebaud Weksteen]

On Tue, Sep 12, 2017 at 10:48 AM, Thiebaud Weksteen <tweek-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, Sep 11, 2017 at 10:47:50AM -0600, Jason Gunthorpe wrote:
>> On Mon, Sep 11, 2017 at 12:00:22PM +0200, Thiebaud Weksteen wrote:
>>
>> >     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;
>>
>> Lets have all the read_log_* versions return the postitive log_version
>> and get rid of the chip->flags check here.
>>
>> ie Doesn't ACPI always return the TPM 1 version?
>
> That is my understanding. Ashley, Nayna, could you confirm the format
> version expected by tpm_of? Could it be both?
>

I've changed the returned code for ACPI but not for DeviceTree.
Without confirmation for tpm_of, I am reluctant to modify the current
behaviour.

>>
>> Jason

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Jason Gunthorpe]

On Mon, Sep 18, 2017 at 02:38:03PM +0200, Thiebaud Weksteen wrote:
> On Tue, Sep 12, 2017 at 10:48 AM, Thiebaud Weksteen <tweek@google.com> wrote:
> > On Mon, Sep 11, 2017 at 10:47:50AM -0600, Jason Gunthorpe wrote:
> >> On Mon, Sep 11, 2017 at 12:00:22PM +0200, Thiebaud Weksteen wrote:
> >>
> >> >     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;
> >>
> >> Lets have all the read_log_* versions return the postitive log_version
> >> and get rid of the chip->flags check here.
> >>
> >> ie Doesn't ACPI always return the TPM 1 version?
> >
> > That is my understanding. Ashley, Nayna, could you confirm the format
> > version expected by tpm_of? Could it be both?
> >
> 
> I've changed the returned code for ACPI but not for DeviceTree.
> Without confirmation for tpm_of, I am reluctant to modify the current
> behaviour.

Move the TPM_CHIP_FLAG_TPM2 check into tpm_of to keep the current behavior
but still return the code..

Jason

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]

Hi Thiebaud,

[auto build test ERROR on efi/next]
[also build test ERROR on next-20170913]
[cannot apply to char-misc/char-misc-testing linus/master v4.13]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thiebaud-Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: i386-randconfig-x076-201737 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/char/tpm/tpm_efi.c:20:5: error: redefinition of 'tpm_read_log_efi'
    int tpm_read_log_efi(struct tpm_chip *chip)
        ^~~~~~~~~~~~~~~~
   In file included from drivers/char/tpm/tpm_efi.c:17:0:
   drivers/char/tpm/tpm.h:603:19: note: previous definition of 'tpm_read_log_efi' was here
    static inline int tpm_read_log_efi(struct tpm_chip *chip)
                      ^~~~~~~~~~~~~~~~

vim +/tpm_read_log_efi +20 drivers/char/tpm/tpm_efi.c

    18	
    19	/* read binary bios log from EFI configuration table */
  > 20	int tpm_read_log_efi(struct tpm_chip *chip)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24212 bytes --]

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]

Hi Thiebaud,

[auto build test ERROR on efi/next]
[also build test ERROR on next-20170913]
[cannot apply to char-misc/char-misc-testing linus/master v4.13]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thiebaud-Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: i386-randconfig-x076-201737 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/char/tpm/tpm_efi.c:20:5: error: redefinition of 'tpm_read_log_efi'
    int tpm_read_log_efi(struct tpm_chip *chip)
        ^~~~~~~~~~~~~~~~
   In file included from drivers/char/tpm/tpm_efi.c:17:0:
   drivers/char/tpm/tpm.h:603:19: note: previous definition of 'tpm_read_log_efi' was here
    static inline int tpm_read_log_efi(struct tpm_chip *chip)
                      ^~~~~~~~~~~~~~~~

vim +/tpm_read_log_efi +20 drivers/char/tpm/tpm_efi.c

    18	
    19	/* read binary bios log from EFI configuration table */
  > 20	int tpm_read_log_efi(struct tpm_chip *chip)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24212 bytes --]

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Thiebaud Weksteen]

Fixed in next patch set

On Wed, Sep 13, 2017 at 6:27 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Thiebaud,
>
> [auto build test ERROR on efi/next]
> [also build test ERROR on next-20170913]
> [cannot apply to char-misc/char-misc-testing linus/master v4.13]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Thiebaud-Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
> config: i386-randconfig-x076-201737 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
>>> drivers/char/tpm/tpm_efi.c:20:5: error: redefinition of 'tpm_read_log_efi'
>     int tpm_read_log_efi(struct tpm_chip *chip)
>         ^~~~~~~~~~~~~~~~
>    In file included from drivers/char/tpm/tpm_efi.c:17:0:
>    drivers/char/tpm/tpm.h:603:19: note: previous definition of 'tpm_read_log_efi' was here
>     static inline int tpm_read_log_efi(struct tpm_chip *chip)
>                       ^~~~~~~~~~~~~~~~
>
> vim +/tpm_read_log_efi +20 drivers/char/tpm/tpm_efi.c
>
>     18
>     19  /* read binary bios log from EFI configuration table */
>   > 20  int tpm_read_log_efi(struct tpm_chip *chip)
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Thiebaud Weksteen]

Fixed in next patch set

On Wed, Sep 13, 2017 at 6:27 PM, kbuild test robot <lkp-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> Hi Thiebaud,
>
> [auto build test ERROR on efi/next]
> [also build test ERROR on next-20170913]
> [cannot apply to char-misc/char-misc-testing linus/master v4.13]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Thiebaud-Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
> config: i386-randconfig-x076-201737 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
>>> drivers/char/tpm/tpm_efi.c:20:5: error: redefinition of 'tpm_read_log_efi'
>     int tpm_read_log_efi(struct tpm_chip *chip)
>         ^~~~~~~~~~~~~~~~
>    In file included from drivers/char/tpm/tpm_efi.c:17:0:
>    drivers/char/tpm/tpm.h:603:19: note: previous definition of 'tpm_read_log_efi' was here
>     static inline int tpm_read_log_efi(struct tpm_chip *chip)
>                       ^~~~~~~~~~~~~~~~
>
> vim +/tpm_read_log_efi +20 drivers/char/tpm/tpm_efi.c
>
>     18
>     19  /* read binary bios log from EFI configuration table */
>   > 20  int tpm_read_log_efi(struct tpm_chip *chip)
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:22PM +0200, Thiebaud Weksteen wrote:
> If we are not able to retrieve the TPM event logs from the ACPI table,
> check the EFI configuration table (Linux-specific GUID).
> 
> The format version of the log may be returned by the function. If not
> specified (by previous implementation: tpm_acpi and tpm_of), we default
> to the version of the chip (previous behaviour).
> 
> Signed-off-by: Thiebaud Weksteen <tweek@google.com>

You saw my comment about file naming. I.e. tpm_eventlog_efi.c would be
a more senseful name.

> ---
>  drivers/char/tpm/Makefile        |  2 +-
>  drivers/char/tpm/tpm.h           |  8 +++++
>  drivers/char/tpm/tpm1_eventlog.c | 15 +++++++--
>  drivers/char/tpm/tpm_efi.c       | 66 ++++++++++++++++++++++++++++++++++++++++
>  drivers/firmware/efi/efi.c       |  2 ++
>  include/linux/efi.h              |  1 +
>  6 files changed, 90 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/char/tpm/tpm_efi.c
> 
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 23681f01f95a..74182a63eef2 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -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
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 46caccf6fd1a..1bd97e01df50 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -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);
> diff --git a/drivers/char/tpm/tpm1_eventlog.c b/drivers/char/tpm/tpm1_eventlog.c
> index d6f70f365443..7e25e6bff6ce 100644
> --- a/drivers/char/tpm/tpm1_eventlog.c
> +++ b/drivers/char/tpm/tpm1_eventlog.c
> @@ -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>
> @@ -371,6 +372,10 @@ static int tpm_read_log(struct tpm_chip *chip)
>  	if (rc != -ENODEV)
>  		return rc;
>  
> +	rc = tpm_read_log_efi(chip);
> +	if (rc != -ENODEV)
> +		return rc;
> +
>  	return tpm_read_log_of(chip);
>  }
>  
> @@ -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;

A tid bit, one declaration per line.

> +
>  
>  	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,7 +411,9 @@ 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
> diff --git a/drivers/char/tpm/tpm_efi.c b/drivers/char/tpm/tpm_efi.c
> new file mode 100644
> index 000000000000..c8247fc45bb0
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_efi.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (C) 2017 Google
> + *
> + * Authors:
> + *      Thiebaud Weksteen <tweek@google.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * 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 <linux/tpm_eventlog.h>
> +
> +#include "tpm.h"
> +
> +/* 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;
> +
> +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> +		return -ENODEV;
> +
> +	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
> +		return -ENODEV;
> +
> +	log = &chip->log;
> +
> +	log_tbl = memremap(efi.tpm_log, sizeof(*log_tbl), MEMREMAP_WB);
> +	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 = memremap(efi.tpm_log, sizeof(*log_tbl) + log_size,
> +			   MEMREMAP_WB);
> +	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;
> +}
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 03fbaf8eb248..0308acfaaf76 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -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},
>  };
>  
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index e671ea9a462e..ab78950e6911 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -943,6 +943,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
> 

I would not put efi changes to the same commit.

I did not have too detailed look but code changes look nice and
clean overally. I'll give this more detailed review once it is
sent to linux-integrity@vger.kernel.org and CC'd to
linux-security-module@vger.kernel.org.

/Jarkko

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Thiebaud Weksteen]

On Thu, Sep 14, 2017 at 8:47 PM, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
> On Mon, Sep 11, 2017 at 12:00:22PM +0200, Thiebaud Weksteen wrote:
>> If we are not able to retrieve the TPM event logs from the ACPI table,
>> check the EFI configuration table (Linux-specific GUID).
>>
>> The format version of the log may be returned by the function. If not
>> specified (by previous implementation: tpm_acpi and tpm_of), we default
>> to the version of the chip (previous behaviour).
>>
>> Signed-off-by: Thiebaud Weksteen <tweek@google.com>
>
> You saw my comment about file naming. I.e. tpm_eventlog_efi.c would be
> a more senseful name.

Done.

>
>> ---
>>  drivers/char/tpm/Makefile        |  2 +-
>>  drivers/char/tpm/tpm.h           |  8 +++++
>>  drivers/char/tpm/tpm1_eventlog.c | 15 +++++++--
>>  drivers/char/tpm/tpm_efi.c       | 66 ++++++++++++++++++++++++++++++++++++++++
>>  drivers/firmware/efi/efi.c       |  2 ++
>>  include/linux/efi.h              |  1 +
>>  6 files changed, 90 insertions(+), 4 deletions(-)
>>  create mode 100644 drivers/char/tpm/tpm_efi.c
>>
>> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
>> index 23681f01f95a..74182a63eef2 100644
>> --- a/drivers/char/tpm/Makefile
>> +++ b/drivers/char/tpm/Makefile
>> @@ -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
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 46caccf6fd1a..1bd97e01df50 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -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);
>> diff --git a/drivers/char/tpm/tpm1_eventlog.c b/drivers/char/tpm/tpm1_eventlog.c
>> index d6f70f365443..7e25e6bff6ce 100644
>> --- a/drivers/char/tpm/tpm1_eventlog.c
>> +++ b/drivers/char/tpm/tpm1_eventlog.c
>> @@ -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>
>> @@ -371,6 +372,10 @@ static int tpm_read_log(struct tpm_chip *chip)
>>       if (rc != -ENODEV)
>>               return rc;
>>
>> +     rc = tpm_read_log_efi(chip);
>> +     if (rc != -ENODEV)
>> +             return rc;
>> +
>>       return tpm_read_log_of(chip);
>>  }
>>
>> @@ -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;
>
> A tid bit, one declaration per line.

Done.

>
>> +
>>
>>       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,7 +411,9 @@ 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
>> diff --git a/drivers/char/tpm/tpm_efi.c b/drivers/char/tpm/tpm_efi.c
>> new file mode 100644
>> index 000000000000..c8247fc45bb0
>> --- /dev/null
>> +++ b/drivers/char/tpm/tpm_efi.c
>> @@ -0,0 +1,66 @@
>> +/*
>> + * Copyright (C) 2017 Google
>> + *
>> + * Authors:
>> + *      Thiebaud Weksteen <tweek@google.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * 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 <linux/tpm_eventlog.h>
>> +
>> +#include "tpm.h"
>> +
>> +/* 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;
>> +
>> +     if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
>> +             return -ENODEV;
>> +
>> +     if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>> +             return -ENODEV;
>> +
>> +     log = &chip->log;
>> +
>> +     log_tbl = memremap(efi.tpm_log, sizeof(*log_tbl), MEMREMAP_WB);
>> +     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 = memremap(efi.tpm_log, sizeof(*log_tbl) + log_size,
>> +                        MEMREMAP_WB);
>> +     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;
>> +}
>> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
>> index 03fbaf8eb248..0308acfaaf76 100644
>> --- a/drivers/firmware/efi/efi.c
>> +++ b/drivers/firmware/efi/efi.c
>> @@ -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},
>>  };
>>
>> diff --git a/include/linux/efi.h b/include/linux/efi.h
>> index e671ea9a462e..ab78950e6911 100644
>> --- a/include/linux/efi.h
>> +++ b/include/linux/efi.h
>> @@ -943,6 +943,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
>>
>
> I would not put efi changes to the same commit.

Done.

>
> I did not have too detailed look but code changes look nice and
> clean overally. I'll give this more detailed review once it is
> sent to linux-integrity@vger.kernel.org and CC'd to
> linux-security-module@vger.kernel.org.

Will do.

>
> /Jarkko

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Thiebaud Weksteen]

On Thu, Sep 14, 2017 at 8:47 PM, Jarkko Sakkinen
<jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On Mon, Sep 11, 2017 at 12:00:22PM +0200, Thiebaud Weksteen wrote:
>> If we are not able to retrieve the TPM event logs from the ACPI table,
>> check the EFI configuration table (Linux-specific GUID).
>>
>> The format version of the log may be returned by the function. If not
>> specified (by previous implementation: tpm_acpi and tpm_of), we default
>> to the version of the chip (previous behaviour).
>>
>> Signed-off-by: Thiebaud Weksteen <tweek-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>
> You saw my comment about file naming. I.e. tpm_eventlog_efi.c would be
> a more senseful name.

Done.

>
>> ---
>>  drivers/char/tpm/Makefile        |  2 +-
>>  drivers/char/tpm/tpm.h           |  8 +++++
>>  drivers/char/tpm/tpm1_eventlog.c | 15 +++++++--
>>  drivers/char/tpm/tpm_efi.c       | 66 ++++++++++++++++++++++++++++++++++++++++
>>  drivers/firmware/efi/efi.c       |  2 ++
>>  include/linux/efi.h              |  1 +
>>  6 files changed, 90 insertions(+), 4 deletions(-)
>>  create mode 100644 drivers/char/tpm/tpm_efi.c
>>
>> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
>> index 23681f01f95a..74182a63eef2 100644
>> --- a/drivers/char/tpm/Makefile
>> +++ b/drivers/char/tpm/Makefile
>> @@ -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
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 46caccf6fd1a..1bd97e01df50 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -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);
>> diff --git a/drivers/char/tpm/tpm1_eventlog.c b/drivers/char/tpm/tpm1_eventlog.c
>> index d6f70f365443..7e25e6bff6ce 100644
>> --- a/drivers/char/tpm/tpm1_eventlog.c
>> +++ b/drivers/char/tpm/tpm1_eventlog.c
>> @@ -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>
>> @@ -371,6 +372,10 @@ static int tpm_read_log(struct tpm_chip *chip)
>>       if (rc != -ENODEV)
>>               return rc;
>>
>> +     rc = tpm_read_log_efi(chip);
>> +     if (rc != -ENODEV)
>> +             return rc;
>> +
>>       return tpm_read_log_of(chip);
>>  }
>>
>> @@ -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;
>
> A tid bit, one declaration per line.

Done.

>
>> +
>>
>>       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,7 +411,9 @@ 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
>> diff --git a/drivers/char/tpm/tpm_efi.c b/drivers/char/tpm/tpm_efi.c
>> new file mode 100644
>> index 000000000000..c8247fc45bb0
>> --- /dev/null
>> +++ b/drivers/char/tpm/tpm_efi.c
>> @@ -0,0 +1,66 @@
>> +/*
>> + * Copyright (C) 2017 Google
>> + *
>> + * Authors:
>> + *      Thiebaud Weksteen <tweek-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * 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 <linux/tpm_eventlog.h>
>> +
>> +#include "tpm.h"
>> +
>> +/* 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;
>> +
>> +     if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
>> +             return -ENODEV;
>> +
>> +     if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>> +             return -ENODEV;
>> +
>> +     log = &chip->log;
>> +
>> +     log_tbl = memremap(efi.tpm_log, sizeof(*log_tbl), MEMREMAP_WB);
>> +     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 = memremap(efi.tpm_log, sizeof(*log_tbl) + log_size,
>> +                        MEMREMAP_WB);
>> +     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;
>> +}
>> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
>> index 03fbaf8eb248..0308acfaaf76 100644
>> --- a/drivers/firmware/efi/efi.c
>> +++ b/drivers/firmware/efi/efi.c
>> @@ -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},
>>  };
>>
>> diff --git a/include/linux/efi.h b/include/linux/efi.h
>> index e671ea9a462e..ab78950e6911 100644
>> --- a/include/linux/efi.h
>> +++ b/include/linux/efi.h
>> @@ -943,6 +943,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
>>
>
> I would not put efi changes to the same commit.

Done.

>
> I did not have too detailed look but code changes look nice and
> clean overally. I'll give this more detailed review once it is
> sent to linux-integrity-u79uwXL29TY76Z2rM5mHXA@public.gmane.org and CC'd to
> linux-security-module-u79uwXL29TaiAVqoAR/hOA@public.gmane.org

Will do.

>
> /Jarkko

Re: [PATCH v2 0/3] Call GetEventLog before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:19PM +0200, Thiebaud Weksteen wrote:
> With TPM 1.2, the ACPI table ("TCPA") has two fields to recover the Event Log
> Area (LAML and LASA). These logs are useful to understand and rebuild the
> final values of PCRs.
> 
> With TPM 2.0, the ACPI table ("TPM2") does not contain these fields anymore.
> The recommended method is now to call the GetEventLog EFI protocol before
> ExitBootServices.
> 
> Implement this method within the EFI stub and create copy of the logs for the
> TPM device. This will create /sys/kernel/security/tpm0/binary_bios_measurements
> for TPM 2.0 devices (similarly to the current behaviour for TPM 1.2 devices).
> 
> -------------------------------------------------------------------------------
> 
> Patchset Changelog:
> 
> Version 2:
> - Move tpm_eventlog.h to top include directory, add commit for this.
> - Use EFI_LOADER_DATA to store the configuration table
> - Whitespace and new lines fixes
> 
> 
> Thiebaud Weksteen (3):
>   tpm: move tpm_eventlog.h outside of drivers folder
>   efi: call get_event_log before ExitBootServices
>   tpm: parse TPM event logs based on EFI table
> 
>  arch/x86/boot/compressed/eboot.c                   |  1 +
>  drivers/char/tpm/Makefile                          |  2 +-
>  drivers/char/tpm/tpm-chip.c                        |  3 +-
>  drivers/char/tpm/tpm-interface.c                   |  2 +-
>  drivers/char/tpm/tpm.h                             | 35 ++++++++--
>  drivers/char/tpm/tpm1_eventlog.c                   | 17 +++--
>  drivers/char/tpm/tpm2_eventlog.c                   |  2 +-
>  drivers/char/tpm/tpm_acpi.c                        |  2 +-
>  drivers/char/tpm/tpm_efi.c                         | 66 ++++++++++++++++++
>  drivers/char/tpm/tpm_of.c                          |  2 +-

I think these filenames are just awful. Now that you are introducing
completely a new file, it would make sense rename these as

* tpm_eventlog_acpi.c
* tpm_eventlog_efi.c
* tpm_eventlog_of.c

Please wait for further review comments before sending a refined patch
set. Please have renames for tpm_acpi.c and tpm_of.c in its own commit
before introducing other changes.

/Jarkko

Re: [PATCH v2 0/3] Call GetEventLog before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:19PM +0200, Thiebaud Weksteen wrote:
> With TPM 1.2, the ACPI table ("TCPA") has two fields to recover the Event Log
> Area (LAML and LASA). These logs are useful to understand and rebuild the
> final values of PCRs.
> 
> With TPM 2.0, the ACPI table ("TPM2") does not contain these fields anymore.
> The recommended method is now to call the GetEventLog EFI protocol before
> ExitBootServices.
> 
> Implement this method within the EFI stub and create copy of the logs for the
> TPM device. This will create /sys/kernel/security/tpm0/binary_bios_measurements
> for TPM 2.0 devices (similarly to the current behaviour for TPM 1.2 devices).
> 
> -------------------------------------------------------------------------------
> 
> Patchset Changelog:
> 
> Version 2:
> - Move tpm_eventlog.h to top include directory, add commit for this.
> - Use EFI_LOADER_DATA to store the configuration table
> - Whitespace and new lines fixes
> 
> 
> Thiebaud Weksteen (3):
>   tpm: move tpm_eventlog.h outside of drivers folder
>   efi: call get_event_log before ExitBootServices
>   tpm: parse TPM event logs based on EFI table
> 
>  arch/x86/boot/compressed/eboot.c                   |  1 +
>  drivers/char/tpm/Makefile                          |  2 +-
>  drivers/char/tpm/tpm-chip.c                        |  3 +-
>  drivers/char/tpm/tpm-interface.c                   |  2 +-
>  drivers/char/tpm/tpm.h                             | 35 ++++++++--
>  drivers/char/tpm/tpm1_eventlog.c                   | 17 +++--
>  drivers/char/tpm/tpm2_eventlog.c                   |  2 +-
>  drivers/char/tpm/tpm_acpi.c                        |  2 +-
>  drivers/char/tpm/tpm_efi.c                         | 66 ++++++++++++++++++
>  drivers/char/tpm/tpm_of.c                          |  2 +-

I think these filenames are just awful. Now that you are introducing
completely a new file, it would make sense rename these as

* tpm_eventlog_acpi.c
* tpm_eventlog_efi.c
* tpm_eventlog_of.c

Please wait for further review comments before sending a refined patch
set. Please have renames for tpm_acpi.c and tpm_of.c in its own commit
before introducing other changes.

/Jarkko

Re: [tpmdd-devel] [PATCH v2 0/3] Call GetEventLog before ExitBootServices

[Javier Martinez Canillas]

Hello Thiebaud,

On 09/11/2017 12:00 PM, Thiebaud Weksteen via tpmdd-devel wrote:
> With TPM 1.2, the ACPI table ("TCPA") has two fields to recover the Event Log
> Area (LAML and LASA). These logs are useful to understand and rebuild the
> final values of PCRs.
> 
> With TPM 2.0, the ACPI table ("TPM2") does not contain these fields anymore.
> The recommended method is now to call the GetEventLog EFI protocol before
> ExitBootServices.
> 
> Implement this method within the EFI stub and create copy of the logs for the
> TPM device. This will create /sys/kernel/security/tpm0/binary_bios_measurements
> for TPM 2.0 devices (similarly to the current behaviour for TPM 1.2 devices).
> 

I've tested your patches on a system with an Intel PTT firmware based TPM2.0 and
the measurements securityfs entry was correctly created and was able to read it:

$ cat /sys/class/tpm/tpm0/device/description
TPM 2.0 Device

$ hexdump /sys/kernel/security/tpm0/binary_bios_measurements | head -n2
0000000 0000 0000 0008 0000 f504 15a0 1810 bf44
0000010 63d0 4fdb b8a4 f278 8dc7 c8aa 0014 0000

So please feel free to add:

Tested-by: Javier Martinez Canillas <javierm@redhat.com>

I also reviewed the patches and look good to me, I have just one question for
patch #2, but I'll comment there.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH v2 0/3] Call GetEventLog before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:19PM +0200, Thiebaud Weksteen wrote:
> With TPM 1.2, the ACPI table ("TCPA") has two fields to recover the Event Log
> Area (LAML and LASA). These logs are useful to understand and rebuild the
> final values of PCRs.
> 
> With TPM 2.0, the ACPI table ("TPM2") does not contain these fields anymore.
> The recommended method is now to call the GetEventLog EFI protocol before
> ExitBootServices.
> 
> Implement this method within the EFI stub and create copy of the logs for the
> TPM device. This will create /sys/kernel/security/tpm0/binary_bios_measurements
> for TPM 2.0 devices (similarly to the current behaviour for TPM 1.2 devices).
> 
> -------------------------------------------------------------------------------
> 
> Patchset Changelog:
> 
> Version 2:
> - Move tpm_eventlog.h to top include directory, add commit for this.
> - Use EFI_LOADER_DATA to store the configuration table
> - Whitespace and new lines fixes
> 
> 
> Thiebaud Weksteen (3):
>   tpm: move tpm_eventlog.h outside of drivers folder
>   efi: call get_event_log before ExitBootServices
>   tpm: parse TPM event logs based on EFI table
> 
>  arch/x86/boot/compressed/eboot.c                   |  1 +
>  drivers/char/tpm/Makefile                          |  2 +-
>  drivers/char/tpm/tpm-chip.c                        |  3 +-
>  drivers/char/tpm/tpm-interface.c                   |  2 +-
>  drivers/char/tpm/tpm.h                             | 35 ++++++++--
>  drivers/char/tpm/tpm1_eventlog.c                   | 17 +++--
>  drivers/char/tpm/tpm2_eventlog.c                   |  2 +-
>  drivers/char/tpm/tpm_acpi.c                        |  2 +-
>  drivers/char/tpm/tpm_efi.c                         | 66 ++++++++++++++++++
>  drivers/char/tpm/tpm_of.c                          |  2 +-
>  drivers/firmware/efi/Makefile                      |  2 +-
>  drivers/firmware/efi/efi.c                         |  4 ++
>  drivers/firmware/efi/libstub/Makefile              |  3 +-
>  drivers/firmware/efi/libstub/tpm.c                 | 81 ++++++++++++++++++++++
>  drivers/firmware/efi/tpm.c                         | 39 +++++++++++
>  include/linux/efi.h                                | 50 +++++++++++++
>  {drivers/char/tpm => include/linux}/tpm_eventlog.h | 32 ++-------
>  17 files changed, 301 insertions(+), 42 deletions(-)
>  create mode 100644 drivers/char/tpm/tpm_efi.c
>  create mode 100644 drivers/firmware/efi/tpm.c
>  rename {drivers/char/tpm => include/linux}/tpm_eventlog.h (77%)
> 
> -- 
> 2.14.1.581.gf28d330327-goog
> 

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.cpm>

/Jarkko

Re: [PATCH v2 0/3] Call GetEventLog before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:19PM +0200, Thiebaud Weksteen wrote:
> With TPM 1.2, the ACPI table ("TCPA") has two fields to recover the Event Log
> Area (LAML and LASA). These logs are useful to understand and rebuild the
> final values of PCRs.
> 
> With TPM 2.0, the ACPI table ("TPM2") does not contain these fields anymore.
> The recommended method is now to call the GetEventLog EFI protocol before
> ExitBootServices.
> 
> Implement this method within the EFI stub and create copy of the logs for the
> TPM device. This will create /sys/kernel/security/tpm0/binary_bios_measurements
> for TPM 2.0 devices (similarly to the current behaviour for TPM 1.2 devices).
> 
> -------------------------------------------------------------------------------
> 
> Patchset Changelog:
> 
> Version 2:
> - Move tpm_eventlog.h to top include directory, add commit for this.
> - Use EFI_LOADER_DATA to store the configuration table
> - Whitespace and new lines fixes
> 
> 
> Thiebaud Weksteen (3):
>   tpm: move tpm_eventlog.h outside of drivers folder
>   efi: call get_event_log before ExitBootServices
>   tpm: parse TPM event logs based on EFI table
> 
>  arch/x86/boot/compressed/eboot.c                   |  1 +
>  drivers/char/tpm/Makefile                          |  2 +-
>  drivers/char/tpm/tpm-chip.c                        |  3 +-
>  drivers/char/tpm/tpm-interface.c                   |  2 +-
>  drivers/char/tpm/tpm.h                             | 35 ++++++++--
>  drivers/char/tpm/tpm1_eventlog.c                   | 17 +++--
>  drivers/char/tpm/tpm2_eventlog.c                   |  2 +-
>  drivers/char/tpm/tpm_acpi.c                        |  2 +-
>  drivers/char/tpm/tpm_efi.c                         | 66 ++++++++++++++++++
>  drivers/char/tpm/tpm_of.c                          |  2 +-
>  drivers/firmware/efi/Makefile                      |  2 +-
>  drivers/firmware/efi/efi.c                         |  4 ++
>  drivers/firmware/efi/libstub/Makefile              |  3 +-
>  drivers/firmware/efi/libstub/tpm.c                 | 81 ++++++++++++++++++++++
>  drivers/firmware/efi/tpm.c                         | 39 +++++++++++
>  include/linux/efi.h                                | 50 +++++++++++++
>  {drivers/char/tpm => include/linux}/tpm_eventlog.h | 32 ++-------
>  17 files changed, 301 insertions(+), 42 deletions(-)
>  create mode 100644 drivers/char/tpm/tpm_efi.c
>  create mode 100644 drivers/firmware/efi/tpm.c
>  rename {drivers/char/tpm => include/linux}/tpm_eventlog.h (77%)
> 
> -- 
> 2.14.1.581.gf28d330327-goog
> 

Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1560T/Km9Ha9FPA@public.gmane.org>

/Jarkko