Add support for TCG2 log format on UEFI systems

Add support for TCG2 log format on UEFI systems

[Matthew Garrett]

Identical to V4, but based on tpmdd-next

[PATCH V5 1/4] tpm: Abstract crypto agile event size calculations

[Matthew Garrett]

From: Matthew Garrett <mjg59@google.com>

We need to calculate the size of crypto agile events in multiple
locations, including in the EFI boot stub. The easiest way to do this is
to put it in a header file as an inline and leave a wrapper to ensure we
don't end up with multiple copies of it embedded in the existing code.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 drivers/char/tpm/eventlog/tpm2.c | 47 +---------------------
 include/linux/tpm_eventlog.h     | 68 ++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 46 deletions(-)

diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
index f824563fc28d..1a977bdd3bd2 100644
--- a/drivers/char/tpm/eventlog/tpm2.c
+++ b/drivers/char/tpm/eventlog/tpm2.c
@@ -40,52 +40,7 @@
 static size_t calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 				   struct tcg_pcr_event *event_header)
 {
-	struct tcg_efi_specid_event_head *efispecid;
-	struct tcg_event_field *event_field;
-	void *marker;
-	void *marker_start;
-	u32 halg_size;
-	size_t size;
-	u16 halg;
-	int i;
-	int j;
-
-	marker = event;
-	marker_start = marker;
-	marker = marker + sizeof(event->pcr_idx) + sizeof(event->event_type)
-		+ sizeof(event->count);
-
-	efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
-
-	/* Check if event is malformed. */
-	if (event->count > efispecid->num_algs)
-		return 0;
-
-	for (i = 0; i < event->count; i++) {
-		halg_size = sizeof(event->digests[i].alg_id);
-		memcpy(&halg, marker, halg_size);
-		marker = marker + halg_size;
-		for (j = 0; j < efispecid->num_algs; j++) {
-			if (halg == efispecid->digest_sizes[j].alg_id) {
-				marker +=
-					efispecid->digest_sizes[j].digest_size;
-				break;
-			}
-		}
-		/* Algorithm without known length. Such event is unparseable. */
-		if (j == efispecid->num_algs)
-			return 0;
-	}
-
-	event_field = (struct tcg_event_field *)marker;
-	marker = marker + sizeof(event_field->event_size)
-		+ event_field->event_size;
-	size = marker - marker_start;
-
-	if ((event->event_type == 0) && (event_field->event_size == 0))
-		return 0;
-
-	return size;
+	return __calc_tpm2_event_size(event, event_header);
 }
 
 static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 81519f163211..6a86144e13f1 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -112,4 +112,72 @@ struct tcg_pcr_event2_head {
 	struct tpm_digest digests[];
 } __packed;
 
+/**
+ * __calc_tpm2_event_size - calculate the size of a TPM2 event log entry
+ * @event:        Pointer to the event whose size should be calculated
+ * @event_header: Pointer to the initial event containing the digest lengths
+ *
+ * The TPM2 event log format can contain multiple digests corresponding to
+ * separate PCR banks, and also contains a variable length of the data that
+ * was measured. This requires knowledge of how long each digest type is,
+ * and this information is contained within the first event in the log.
+ *
+ * We calculate the length by examining the number of events, and then looking
+ * at each event in turn to determine how much space is used for events in
+ * total. Once we've done this we know the offset of the data length field,
+ * and can calculate the total size of the event.
+ *
+ * Return: size of the event on success, <0 on failure
+ */
+
+static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
+					 struct tcg_pcr_event *event_header)
+{
+	struct tcg_efi_specid_event_head *efispecid;
+	struct tcg_event_field *event_field;
+	void *marker;
+	void *marker_start;
+	u32 halg_size;
+	size_t size;
+	u16 halg;
+	int i;
+	int j;
+
+	marker = event;
+	marker_start = marker;
+	marker = marker + sizeof(event->pcr_idx) + sizeof(event->event_type)
+		+ sizeof(event->count);
+
+	efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
+
+	/* Check if event is malformed. */
+	if (event->count > efispecid->num_algs)
+		return 0;
+
+	for (i = 0; i < event->count; i++) {
+		halg_size = sizeof(event->digests[i].alg_id);
+		memcpy(&halg, marker, halg_size);
+		marker = marker + halg_size;
+		for (j = 0; j < efispecid->num_algs; j++) {
+			if (halg == efispecid->digest_sizes[j].alg_id) {
+				marker +=
+					efispecid->digest_sizes[j].digest_size;
+				break;
+			}
+		}
+		/* Algorithm without known length. Such event is unparseable. */
+		if (j == efispecid->num_algs)
+			return 0;
+	}
+
+	event_field = (struct tcg_event_field *)marker;
+	marker = marker + sizeof(event_field->event_size)
+		+ event_field->event_size;
+	size = marker - marker_start;
+
+	if ((event->event_type == 0) && (event_field->event_size == 0))
+		return 0;
+
+	return size;
+}
 #endif
-- 
2.21.0.352.gf09ad66450-goog

[PATCH V5 2/4] tpm: Reserve the TPM final events table

[Matthew Garrett]

From: Matthew Garrett <mjg59@google.com>

UEFI systems provide a boot services protocol for obtaining the TPM
event log, but this is unusable after ExitBootServices() is called.
Unfortunately ExitBootServices() itself triggers additional TPM events
that then can't be obtained using this protocol. The platform provides a
mechanism for the OS to obtain these events by recording them to a
separate UEFI configuration table which the OS can then map.

Unfortunately this table isn't self describing in terms of providing its
length, so we need to parse the events inside it to figure out how long
it is. Since the table isn't mapped at this point, we need to extend the
length calculation function to be able to map the event as it goes
along.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 drivers/char/tpm/eventlog/tpm2.c |  2 +-
 drivers/firmware/efi/efi.c       |  2 +
 drivers/firmware/efi/tpm.c       | 51 ++++++++++++++++-
 include/linux/efi.h              |  9 +++
 include/linux/tpm_eventlog.h     | 94 +++++++++++++++++++++++++++++---
 5 files changed, 148 insertions(+), 10 deletions(-)

diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
index 1a977bdd3bd2..de1d9f7e5a92 100644
--- a/drivers/char/tpm/eventlog/tpm2.c
+++ b/drivers/char/tpm/eventlog/tpm2.c
@@ -40,7 +40,7 @@
 static size_t calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 				   struct tcg_pcr_event *event_header)
 {
-	return __calc_tpm2_event_size(event, event_header);
+	return __calc_tpm2_event_size(event, event_header, false);
 }
 
 static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 4c46ff6f2242..bf4e9a254e23 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -53,6 +53,7 @@ struct efi __read_mostly efi = {
 	.mem_attr_table		= EFI_INVALID_TABLE_ADDR,
 	.rng_seed		= EFI_INVALID_TABLE_ADDR,
 	.tpm_log		= EFI_INVALID_TABLE_ADDR,
+	.tpm_final_log		= EFI_INVALID_TABLE_ADDR,
 	.mem_reserve		= EFI_INVALID_TABLE_ADDR,
 };
 EXPORT_SYMBOL(efi);
@@ -485,6 +486,7 @@ static __initdata efi_config_table_type_t common_tables[] = {
 	{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},
+	{LINUX_EFI_TPM_FINAL_LOG_GUID, "TPMFinalLog", &efi.tpm_final_log},
 	{LINUX_EFI_MEMRESERVE_TABLE_GUID, "MEMRESERVE", &efi.mem_reserve},
 	{NULL_GUID, NULL, NULL},
 };
diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index 0cbeb3d46b18..2ccaa6661aaf 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -10,24 +10,50 @@
 #include <linux/efi.h>
 #include <linux/init.h>
 #include <linux/memblock.h>
+#include <linux/tpm_eventlog.h>
 
 #include <asm/early_ioremap.h>
 
+int efi_tpm_final_log_size;
+EXPORT_SYMBOL(efi_tpm_final_log_size);
+
+static int tpm2_calc_event_log_size(void *data, int count, void *size_info)
+{
+	struct tcg_pcr_event2_head *header;
+	int event_size, size = 0;
+
+	while (count > 0) {
+		header = data + size;
+		event_size = __calc_tpm2_event_size(header, size_info, true);
+		if (event_size == 0)
+			return -1;
+		size += event_size;
+	}
+
+	return size;
+}
+
 /*
  * Reserve the memory associated with the TPM Event Log configuration table.
  */
 int __init efi_tpm_eventlog_init(void)
 {
 	struct linux_efi_tpm_eventlog *log_tbl;
+	struct efi_tcg2_final_events_table *final_tbl;
 	unsigned int tbl_size;
 
-	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
+	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
+		/*
+		 * We can't calculate the size of the final events without the
+		 * first entry in the TPM log, so bail here.
+		 */
 		return 0;
+	}
 
 	log_tbl = early_memremap(efi.tpm_log, sizeof(*log_tbl));
 	if (!log_tbl) {
 		pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
-			efi.tpm_log);
+		       efi.tpm_log);
 		efi.tpm_log = EFI_INVALID_TABLE_ADDR;
 		return -ENOMEM;
 	}
@@ -35,6 +61,27 @@ int __init efi_tpm_eventlog_init(void)
 	tbl_size = sizeof(*log_tbl) + log_tbl->size;
 	memblock_reserve(efi.tpm_log, tbl_size);
 	early_memunmap(log_tbl, sizeof(*log_tbl));
+
+	if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
+		return 0;
+
+	final_tbl = early_memremap(efi.tpm_final_log, sizeof(*final_tbl));
+
+	if (!final_tbl) {
+		pr_err("Failed to map TPM Final Event Log table @ 0x%lx\n",
+		       efi.tpm_final_log);
+		efi.tpm_final_log = EFI_INVALID_TABLE_ADDR;
+		return -ENOMEM;
+	}
+
+	tbl_size = tpm2_calc_event_log_size(final_tbl->events,
+					    final_tbl->nr_events,
+					    (void *)efi.tpm_log);
+	memblock_reserve((unsigned long)final_tbl,
+			 tbl_size + sizeof(*final_tbl));
+	early_memunmap(final_tbl, sizeof(*final_tbl));
+	efi_tpm_final_log_size = tbl_size;
+
 	return 0;
 }
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 45ff763fba76..730dae84a932 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -676,6 +676,7 @@ void efi_native_runtime_setup(void);
 #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)
+#define LINUX_EFI_TPM_FINAL_LOG_GUID		EFI_GUID(0x1e2ed096, 0x30e2, 0x4254,  0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25)
 #define LINUX_EFI_MEMRESERVE_TABLE_GUID		EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5,  0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
 
 typedef struct {
@@ -983,6 +984,7 @@ extern struct efi {
 	unsigned long mem_attr_table;	/* memory attributes table */
 	unsigned long rng_seed;		/* UEFI firmware random seed */
 	unsigned long tpm_log;		/* TPM2 Event Log table */
+	unsigned long tpm_final_log;	/* TPM2 Final Events Log table */
 	unsigned long mem_reserve;	/* Linux EFI memreserve table */
 	efi_get_time_t *get_time;
 	efi_set_time_t *set_time;
@@ -1700,6 +1702,13 @@ struct linux_efi_tpm_eventlog {
 
 extern int efi_tpm_eventlog_init(void);
 
+struct efi_tcg2_final_events_table {
+	u64 version;
+	u64 nr_events;
+	u8 events[];
+};
+extern int efi_tpm_final_log_size;
+
 /*
  * efi_runtime_service() function identifiers.
  * "NONE" is used by efi_recover_from_page_fault() to check if the page
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 6a86144e13f1..d889e12047d9 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -112,10 +112,27 @@ struct tcg_pcr_event2_head {
 	struct tpm_digest digests[];
 } __packed;
 
+struct tcg_algorithm_size {
+	u16 algorithm_id;
+	u16 algorithm_size;
+};
+
+struct tcg_algorithm_info {
+	u8 signature[16];
+	u32 platform_class;
+	u8 spec_version_minor;
+	u8 spec_version_major;
+	u8 spec_errata;
+	u8 uintn_size;
+	u32 number_of_algorithms;
+	struct tcg_algorithm_size digest_sizes[];
+};
+
 /**
  * __calc_tpm2_event_size - calculate the size of a TPM2 event log entry
  * @event:        Pointer to the event whose size should be calculated
  * @event_header: Pointer to the initial event containing the digest lengths
+ * @do_mapping:   Whether or not the event needs to be mapped
  *
  * The TPM2 event log format can contain multiple digests corresponding to
  * separate PCR banks, and also contains a variable length of the data that
@@ -131,10 +148,13 @@ struct tcg_pcr_event2_head {
  */
 
 static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
-					 struct tcg_pcr_event *event_header)
+					 struct tcg_pcr_event *event_header,
+					 bool do_mapping)
 {
 	struct tcg_efi_specid_event_head *efispecid;
 	struct tcg_event_field *event_field;
+	void *mapping = NULL;
+	int mapping_size;
 	void *marker;
 	void *marker_start;
 	u32 halg_size;
@@ -148,36 +168,96 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	marker = marker + sizeof(event->pcr_idx) + sizeof(event->event_type)
 		+ sizeof(event->count);
 
+	/* Map the event header */
+	if (do_mapping) {
+		mapping_size = marker - marker_start;
+		mapping = early_memremap((unsigned long)marker_start,
+					 mapping_size);
+		if (!mapping) {
+			size = 0;
+			goto out;
+		}
+	}
+
 	efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
 
 	/* Check if event is malformed. */
-	if (event->count > efispecid->num_algs)
-		return 0;
+	if (event->count > efispecid->num_algs) {
+		size = 0;
+		goto out;
+	}
 
 	for (i = 0; i < event->count; i++) {
 		halg_size = sizeof(event->digests[i].alg_id);
+
+		/* Map the digest's algorithm identifier */
+		if (do_mapping) {
+			early_memunmap(mapping, mapping_size);
+			mapping_size = marker - marker_start + halg_size;
+			mapping = early_memremap((unsigned long)marker_start,
+						 mapping_size);
+			if (!mapping) {
+				size = 0;
+				goto out;
+			}
+		}
+
 		memcpy(&halg, marker, halg_size);
 		marker = marker + halg_size;
+
 		for (j = 0; j < efispecid->num_algs; j++) {
 			if (halg == efispecid->digest_sizes[j].alg_id) {
 				marker +=
 					efispecid->digest_sizes[j].digest_size;
+
+				/* Map the digest content itself */
+				if (do_mapping) {
+					early_memunmap(mapping, mapping_size);
+					mapping_size = marker - marker_start;
+					mapping = early_memremap((unsigned long)marker_start,
+						      mapping_size);
+					if (!mapping) {
+						size = 0;
+						goto out;
+					}
+				}
 				break;
 			}
 		}
 		/* Algorithm without known length. Such event is unparseable. */
-		if (j == efispecid->num_algs)
-			return 0;
+		if (j == efispecid->num_algs) {
+			size = 0;
+			goto out;
+		}
 	}
 
 	event_field = (struct tcg_event_field *)marker;
+
+	/*
+	 * Map the event size - we don't read from the event itself, so
+	 * we don't need to map it
+	 */
+	if (do_mapping) {
+		early_memunmap(marker_start, mapping_size);
+		mapping_size += sizeof(event_field->event_size);
+		mapping = early_memremap((unsigned long)marker_start,
+					 mapping_size);
+		if (!mapping) {
+			size = 0;
+			goto out;
+		}
+	}
+
 	marker = marker + sizeof(event_field->event_size)
 		+ event_field->event_size;
 	size = marker - marker_start;
 
 	if ((event->event_type == 0) && (event_field->event_size == 0))
-		return 0;
-
+		size = 0;
+out:
+	if (do_mapping)
+		early_memunmap(mapping, mapping_size);
 	return size;
 }
+
 #endif
-- 
2.21.0.352.gf09ad66450-goog

[PATCH V5 3/4] tpm: Append the final event log to the TPM event log

[Matthew Garrett]

From: Matthew Garrett <mjg59@google.com>

Any events that are logged after GetEventsLog() is called are logged to
the EFI Final Events table. These events are defined as being in the
crypto agile log format, so we can just append them directly to the
existing log if it's in the same format. In theory we can also construct
old-style SHA1 log entries for devices that only return logs in that
format, but EDK2 doesn't generate the final event log in that case so
it doesn't seem worth it at the moment.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 drivers/char/tpm/eventlog/efi.c | 50 ++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/drivers/char/tpm/eventlog/efi.c b/drivers/char/tpm/eventlog/efi.c
index 3e673ab22cb4..9179cf6bdee9 100644
--- a/drivers/char/tpm/eventlog/efi.c
+++ b/drivers/char/tpm/eventlog/efi.c
@@ -21,10 +21,13 @@
 int tpm_read_log_efi(struct tpm_chip *chip)
 {
 
+	struct efi_tcg2_final_events_table *final_tbl = NULL;
 	struct linux_efi_tpm_eventlog *log_tbl;
 	struct tpm_bios_log *log;
 	u32 log_size;
 	u8 tpm_log_version;
+	void *tmp;
+	int ret;
 
 	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
 		return -ENODEV;
@@ -52,15 +55,48 @@ int tpm_read_log_efi(struct tpm_chip *chip)
 
 	/* malloc EventLog space */
 	log->bios_event_log = kmemdup(log_tbl->log, log_size, GFP_KERNEL);
-	if (!log->bios_event_log)
-		goto err_memunmap;
-	log->bios_event_log_end = log->bios_event_log + log_size;
+	if (!log->bios_event_log) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
+	log->bios_event_log_end = log->bios_event_log + log_size;
 	tpm_log_version = log_tbl->version;
-	memunmap(log_tbl);
-	return tpm_log_version;
 
-err_memunmap:
+	ret = tpm_log_version;
+
+	if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR ||
+	    efi_tpm_final_log_size == 0 ||
+	    tpm_log_version != EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)
+		goto out;
+
+	final_tbl = memremap(efi.tpm_final_log,
+			     sizeof(*final_tbl) + efi_tpm_final_log_size,
+			     MEMREMAP_WB);
+	if (!final_tbl) {
+		pr_err("Could not map UEFI TPM final log\n");
+		kfree(log->bios_event_log);
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	tmp = krealloc(log->bios_event_log,
+		       log_size + efi_tpm_final_log_size,
+		       GFP_KERNEL);
+	if (!tmp) {
+		kfree(log->bios_event_log);
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	log->bios_event_log = tmp;
+	memcpy((void *)log->bios_event_log + log_size,
+	       final_tbl->events, efi_tpm_final_log_size);
+	log->bios_event_log_end = log->bios_event_log +
+		log_size + efi_tpm_final_log_size;
+
+out:
+	memunmap(final_tbl);
 	memunmap(log_tbl);
-	return -ENOMEM;
+	return ret;
 }
-- 
2.21.0.352.gf09ad66450-goog

[PATCH V5 4/4] efi: Attempt to get the TCG2 event log in the boot stub

[Matthew Garrett]

From: Matthew Garrett <mjg59@google.com>

Right now we only attempt to obtain the SHA1-only event log. The
protocol also supports a crypto agile log format, which contains digests
for all algorithms in use. Attempt to obtain this first, and fall back
to obtaining the older format if the system doesn't support it. This is
lightly complicated by the event sizes being variable (as we don't know
in advance which algorithms are in use), and the interface giving us
back a pointer to the start of the final entry rather than a pointer to
the end of the log - as a result, we need to parse the final entry to
figure out its length in order to know how much data to copy up to the
OS.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 drivers/firmware/efi/libstub/tpm.c | 50 ++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 17 deletions(-)

diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index a90b0b8fc69a..523cd07c551c 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -59,7 +59,7 @@ void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg)
 
 #endif
 
-static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
+void efi_retrieve_tpm2_eventlog(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;
@@ -69,6 +69,7 @@ static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
 	unsigned long first_entry_addr, last_entry_addr;
 	size_t log_size, last_entry_size;
 	efi_bool_t truncated;
+	int version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
 	void *tcg2_protocol = NULL;
 
 	status = efi_call_early(locate_protocol, &tcg2_guid, NULL,
@@ -76,14 +77,20 @@ static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
 	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;
+	status = efi_call_proto(efi_tcg2_protocol, get_event_log,
+				tcg2_protocol, version, &log_location,
+				&log_last_entry, &truncated);
+
+	if (status != EFI_SUCCESS || !log_location) {
+		version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+		status = efi_call_proto(efi_tcg2_protocol, get_event_log,
+					tcg2_protocol, version, &log_location,
+					&log_last_entry, &truncated);
+		if (status != EFI_SUCCESS || !log_location)
+			return;
+
+	}
 
-	if (!log_location)
-		return;
 	first_entry_addr = (unsigned long) log_location;
 
 	/*
@@ -98,8 +105,23 @@ static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
 		 * 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;
+		if (version == EFI_TCG2_EVENT_LOG_FORMAT_TCG_2) {
+			/*
+			 * The TCG2 log format has variable length entries,
+			 * and the information to decode the hash algorithms
+			 * back into a size is contained in the first entry -
+			 * pass a pointer to the final entry (to calculate its
+			 * size) and the first entry (so we know how long each
+			 * digest is)
+			 */
+			last_entry_size =
+				__calc_tpm2_event_size((void *)last_entry_addr,
+						       (void *)log_location,
+						       false);
+		} else {
+			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;
 	}
 
@@ -116,7 +138,7 @@ static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
 
 	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;
+	log_tbl->version = version;
 	memcpy(log_tbl->log, (void *) first_entry_addr, log_size);
 
 	status = efi_call_early(install_configuration_table,
@@ -128,9 +150,3 @@ static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
 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);
-}
-- 
2.21.0.352.gf09ad66450-goog

Re: Add support for TCG2 log format on UEFI systems

[Jarkko Sakkinen]

On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> Identical to V4, but based on tpmdd-next

This is not found /sys/kernel/security/tpm0/ascii_bios_measurements

But still

[    0.000000] efi:  ACPI 2.0=0x69ca2000  ACPI=0x69ca2000  TPMFinalLog=0x69ce4000  SMBIOS=0x69f63000  SMBIOS 3.0=0x69f62000  ESRT=0x69f3e818  MEMATTR=0x63448018

Tried this with too machines now.

I wonder if anyone else has had success...

/Jarkko

Re: Add support for TCG2 log format on UEFI systems

[Matthew Garrett]

On Thu, Mar 14, 2019 at 2:35 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> > Identical to V4, but based on tpmdd-next
>
> This is not found /sys/kernel/security/tpm0/ascii_bios_measurements

That's expected - the existing kernel TCG2 log code doesn't expose
ascii_bios_measurements, only binary_bios_measurements. This patchset
doesn't change that.

Re: Add support for TCG2 log format on UEFI systems

[Jarkko Sakkinen]

On Thu, Mar 14, 2019 at 02:04:02PM -0700, Matthew Garrett wrote:
> On Thu, Mar 14, 2019 at 2:35 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> > > Identical to V4, but based on tpmdd-next
> >
> > This is not found /sys/kernel/security/tpm0/ascii_bios_measurements
> 
> That's expected - the existing kernel TCG2 log code doesn't expose
> ascii_bios_measurements, only binary_bios_measurements. This patchset
> doesn't change that.

Oops, I meant to point out that the binary_bios_measurents is not found
(i.e. ascii was a typo). The whole tpm0 directory is missing.

I'll try to pinpoint next week where things might go wrong on my system.

/Jarkko

Re: Add support for TCG2 log format on UEFI systems

[Jarkko Sakkinen]

On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> Identical to V4, but based on tpmdd-next

OK, so on my GLK NUC I get valid final log and invalid event log
after adding some extra klogs.

I.e.

-       if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
+       if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
+               pr_err("No event log\n");
                return -ENODEV;
+       }

will result

[    2.654392] No event log 

but still

[    0.000000] efi:  ACPI 2.0=0x69ca2000  ACPI=0x69ca2000  TPMFinalLog=0x69ce4000  SMBIOS=0x69f63000  SMBIOS 3.0=0x69f62000  ESRT=0x69f3e818  MEMATTR=0x63475118

Tomas, I wonder if you are able to get the log out with some machine?

/Jarkko

Re: Add support for TCG2 log format on UEFI systems

[Matthew Garrett]

On Mon, Apr 1, 2019 at 4:52 PM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> > Identical to V4, but based on tpmdd-next
>
> OK, so on my GLK NUC I get valid final log and invalid event log
> after adding some extra klogs.
>
> I.e.
>
> -       if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
> +       if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {

Just to make sure - are you booting via the EFI boot stub? We need to
obtain the boot log before ExitBootServices() is called, so if you're
booting directly into the 32-bit entry point (eg, by using the "linux"
command in grub) then you won't get a log.

Re: Add support for TCG2 log format on UEFI systems

[Jarkko Sakkinen]

On Mon, Apr 01, 2019 at 08:32:26PM -0700, Matthew Garrett wrote:
> On Mon, Apr 1, 2019 at 4:52 PM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> > > Identical to V4, but based on tpmdd-next
> >
> > OK, so on my GLK NUC I get valid final log and invalid event log
> > after adding some extra klogs.
> >
> > I.e.
> >
> > -       if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
> > +       if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
> 
> Just to make sure - are you booting via the EFI boot stub? We need to
> obtain the boot log before ExitBootServices() is called, so if you're
> booting directly into the 32-bit entry point (eg, by using the "linux"
> command in grub) then you won't get a log.

... and I was wondering why it used to work when I tested the first
flush of patches. Ugh, sorry. The only excuse is too much multitasking
lately.

Anyway:

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

I'll apply all patches soonish and include them to the next PR.

/Jarkko

Re: Add support for TCG2 log format on UEFI systems

[Matthew Garrett]

On Tue, Apr 2, 2019 at 6:07 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
> I'll apply all patches soonish and include them to the next PR.

Thanks! Looks like I need some fixes to deal with non-x86
architectures, I'll get on that today.

Re: Add support for TCG2 log format on UEFI systems

[Jarkko Sakkinen]

On Tue, Apr 02, 2019 at 10:15:39AM -0700, Matthew Garrett wrote:
> On Tue, Apr 2, 2019 at 6:07 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >
> > I'll apply all patches soonish and include them to the next PR.
> 
> Thanks! Looks like I need some fixes to deal with non-x86
> architectures, I'll get on that today.

Great thanks. I'll check them tomorrow (Thu).

/Jarkko

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Bartosz Szczepanek]

I may be a little late with this comment, but I've just tested these
patches on aarch64 platform (from the top of jjs/master) and got
kernel panic ("Unable to handle kernel read", full log at the end of
mail). I think there's problem with below call to
tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
passed as (void *) and never remapped:

> +       tbl_size = tpm2_calc_event_log_size(final_tbl->events,
> +                                           final_tbl->nr_events,
> +                                           (void *)efi.tpm_log);

This is later used to get efispecid:

>         efispecid = (struct tcg_efi_specid_event_head *)event_header->event;

It seems event_header is not mapped during dereference. This is
somewhat expected, because it comes from different, already unmapped
memory region (region of initial TPM log) than "event" itself (which
comes from TPM final log).

Also, value passed as size_info shouldn't be pointing to
linux_efi_tpm_eventlog with its size and version fields, but to the
first event (header event) within. I tried with log_tbl->log and it
worked fine (I omitted unmapping part). On the other hand, with bare
log_tbl it still fails. Not sure how does it even work on other
platforms.

One more thing that's not clear for me – shouldn't the value returned
from early_memremap be used for further accesses? Throughout
__calc_tpm2_event_size() "mapping" is only checked for being zero.
When it is, you're still unmapping it – is it correct?

> +       while (count > 0) {
> +               header = data + size;
> +               event_size = __calc_tpm2_event_size(header, size_info, true);
> +               if (event_size == 0)
> +                       return -1;
> +               size += event_size;
> +       }

Loop condition here is always true, by the way.

One information about my setup – I'm working with below local diff to
enable operation on ARM:
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -194,6 +194,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,
>
>       /* Ask the firmware to clear memory on unclean shutdown */
>        efi_enable_reset_attack_mitigation(sys_table);
> +       efi_retrieve_tpm2_eventlog(sys_table);

Full log of kernel panic follows.

EFI stub: Booting Linux Kernel...
EFI stub: EFI_RNG_PROTOCOL unavailable, no randomness supplied
EFI stub: Using DTB from configuration table
EFI stub: Exiting boot services and installing virtual address map...
[    0.000000] Booting Linux on physical CPU 0x0000000000 [0x420f5162]
[    0.000000] Linux version 5.1.0-rc2+ (root@localhost.localdomain)
(gcc version 7.3.1 20180712 (Red Hat 7.3.1-6) (GCC)) #69 SMP Fri Apr
26 03:20:57 EDT 2019
[    0.000000] earlycon: pl11 at MMIO 0x0000000402020000 (options '115200n8')
[    0.000000] printk: bootconsole [pl11] enabled
[    0.000000] efi: Getting EFI parameters from FDT:
[    0.000000] efi: EFI v2.60 by Cavium Inc.
TX2-FW-Release-7.2-build_08-0-g14f8c5bf8a Apr 15 2019 18:51:41
[    0.000000] efi:  TPMFinalLog=0xed5f0000  SMBIOS=0xfad90000  SMBIOS
3.0=0xed530000  ACPI 2.0=0xeda90000  ESRT=0xfafdb218
MEMATTR=0xf8489018  TPMEventLog=0xedaa9018  MEMRESERVE=0xedaa8018
[    0.000000] Unable to handle kernel read from unreadable memory at
virtual address 00000000edaa9050
[    0.000000] Mem abort info:
[    0.000000]   ESR = 0x96000004
[    0.000000]   Exception class = DABT (current EL), IL = 32 bits
[    0.000000]   SET = 0, FnV = 0
[    0.000000]   EA = 0, S1PTW = 0
[    0.000000] Data abort info:
[    0.000000]   ISV = 0, ISS = 0x00000004
[    0.000000]   CM = 0, WnR = 0
[    0.000000] [00000000edaa9050] user address but active_mm is swapper
[    0.000000] Internal error: Oops: 96000004 [#1] SMP
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.1.0-rc2+ #69
[    0.000000] pstate: 60400089 (nZCv daIf +PAN -UAO)
[    0.000000] pc : efi_tpm_eventlog_init+0xfc/0x26c
[    0.000000] lr : efi_tpm_eventlog_init+0xf4/0x26c
[    0.000000] sp : ffff000011533ce0
[    0.000000] x29: ffff000011533ce0 x28: 00000000edaa8018
[    0.000000] x27: ffff7dfffe6fa010 x26: 0000000000000023
[    0.000000] x25: ffff7dfffe6fa000 x24: 00000000edaa9038
[    0.000000] x23: 0000000000000000 x22: ffff7dfffe6fa010
[    0.000000] x21: ffff00001153d000 x20: ffff7dfffe6fa018
[    0.000000] x19: ffff000011542500 x18: ffffffffffffffff
[    0.000000] x17: 0000000000000435 x16: 0000000000000000
[    0.000000] x15: ffff00001153d708 x14: 6576454d50542020
[    0.000000] x13: 3831303938343866 x12: 78303d525454414d
[    0.000000] x11: 454d202038313262 x10: 6466616678303d54
[    0.000000] x9 : ffff00001153ef58 x8 : 0000020000000000
[    0.000000] x7 : 0000000000000a30 x6 : ffff0000110d2a18
[    0.000000] x5 : 000000000000013a x4 : 00000000000004c5
[    0.000000] x3 : ffff000011714000 x2 : 0000000000000002
[    0.000000] x1 : ffff7dfffe6fa000 x0 : ffff7dfffe73a010
[    0.000000] Process swapper (pid: 0, stack limit = 0x(____ptrval____))
[    0.000000] Call trace:
[    0.000000]  efi_tpm_eventlog_init+0xfc/0x26c
[    0.000000]  efi_config_parse_tables+0x180/0x29c
[    0.000000]  uefi_init+0x1d0/0x22c
[    0.000000]  efi_init+0x90/0x180
[    0.000000]  setup_arch+0x1f4/0x5fc
[    0.000000]  start_kernel+0x90/0x51c
[    0.000000] Code: aa1603e0 97ff05c7 b4000860 b9400ac2 (b9401b01)
[    0.000000] random: get_random_bytes called from
print_oops_end_marker+0x54/0x70 with crng_init=0
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
[    0.000000] ---[ end Kernel panic - not syncing: Attempted to kill
the idle task! ]---

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Matthew Garrett]

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

On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek <bsz@semihalf.com> wrote:
>
> I may be a little late with this comment, but I've just tested these
> patches on aarch64 platform (from the top of jjs/master) and got
> kernel panic ("Unable to handle kernel read", full log at the end of
> mail). I think there's problem with below call to
> tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> passed as (void *) and never remapped:

Yes, it looks like this is just broken. Can you try with the attached patch?

[-- Attachment #2: fix_log.diff --]
[-- Type: text/x-patch, Size: 4414 bytes --]

diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index fe48150f06d1..9711bd34f8ae 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -28,6 +28,7 @@ static int tpm2_calc_event_log_size(void *data, int count, void *size_info)
 		if (event_size == 0)
 			return -1;
 		size += event_size;
+		count--;
 	}
 
 	return size;
@@ -41,6 +42,7 @@ int __init efi_tpm_eventlog_init(void)
 	struct linux_efi_tpm_eventlog *log_tbl;
 	struct efi_tcg2_final_events_table *final_tbl;
 	unsigned int tbl_size;
+	int ret = 0;
 
 	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
 		/*
@@ -60,10 +62,9 @@ int __init efi_tpm_eventlog_init(void)
 
 	tbl_size = sizeof(*log_tbl) + log_tbl->size;
 	memblock_reserve(efi.tpm_log, tbl_size);
-	early_memunmap(log_tbl, sizeof(*log_tbl));
 
 	if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
-		return 0;
+		goto out;
 
 	final_tbl = early_memremap(efi.tpm_final_log, sizeof(*final_tbl));
 
@@ -71,17 +72,20 @@ int __init efi_tpm_eventlog_init(void)
 		pr_err("Failed to map TPM Final Event Log table @ 0x%lx\n",
 		       efi.tpm_final_log);
 		efi.tpm_final_log = EFI_INVALID_TABLE_ADDR;
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 
 	tbl_size = tpm2_calc_event_log_size(final_tbl->events,
 					    final_tbl->nr_events,
-					    (void *)efi.tpm_log);
+					    log_tbl->log);
 	memblock_reserve((unsigned long)final_tbl,
 			 tbl_size + sizeof(*final_tbl));
 	early_memunmap(final_tbl, sizeof(*final_tbl));
 	efi_tpm_final_log_size = tbl_size;
 
-	return 0;
+out:
+	early_memunmap(log_tbl, sizeof(*log_tbl));
+	return ret;
 }
 
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 0ca27bc053af..9cfbb14f54e6 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -161,7 +161,6 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 {
 	struct tcg_efi_specid_event_head *efispecid;
 	struct tcg_event_field *event_field;
-	void *mapping = NULL;
 	int mapping_size;
 	void *marker;
 	void *marker_start;
@@ -179,9 +178,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	/* Map the event header */
 	if (do_mapping) {
 		mapping_size = marker - marker_start;
-		mapping = TPM_MEMREMAP((unsigned long)marker_start,
-				       mapping_size);
-		if (!mapping) {
+		event = TPM_MEMREMAP((unsigned long)marker_start,
+				     mapping_size);
+		if (!event) {
 			size = 0;
 			goto out;
 		}
@@ -200,11 +199,11 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 
 		/* Map the digest's algorithm identifier */
 		if (do_mapping) {
-			TPM_MEMUNMAP(mapping, mapping_size);
+			TPM_MEMUNMAP(event, mapping_size);
 			mapping_size = marker - marker_start + halg_size;
-			mapping = TPM_MEMREMAP((unsigned long)marker_start,
-					       mapping_size);
-			if (!mapping) {
+			event = TPM_MEMREMAP((unsigned long)marker_start,
+					     mapping_size);
+			if (!event) {
 				size = 0;
 				goto out;
 			}
@@ -220,11 +219,11 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 
 				/* Map the digest content itself */
 				if (do_mapping) {
-					TPM_MEMUNMAP(mapping, mapping_size);
+					TPM_MEMUNMAP(event, mapping_size);
 					mapping_size = marker - marker_start;
-					mapping = TPM_MEMREMAP((unsigned long)marker_start,
-							       mapping_size);
-					if (!mapping) {
+					event = TPM_MEMREMAP((unsigned long)marker_start,
+							     mapping_size);
+					if (!event) {
 						size = 0;
 						goto out;
 					}
@@ -246,11 +245,11 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	 * we don't need to map it
 	 */
 	if (do_mapping) {
-		TPM_MEMUNMAP(marker_start, mapping_size);
+		TPM_MEMUNMAP(event, mapping_size);
 		mapping_size += sizeof(event_field->event_size);
-		mapping = TPM_MEMREMAP((unsigned long)marker_start,
+		event = TPM_MEMREMAP((unsigned long)marker_start,
 				       mapping_size);
-		if (!mapping) {
+		if (!event) {
 			size = 0;
 			goto out;
 		}
@@ -260,11 +259,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 		+ event_field->event_size;
 	size = marker - marker_start;
 
-	if ((event->event_type == 0) && (event_field->event_size == 0))
-		size = 0;
 out:
 	if (do_mapping)
-		TPM_MEMUNMAP(mapping, mapping_size);
+		TPM_MEMUNMAP(event, mapping_size);
 	return size;
 }
 

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Matthew Garrett]

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

On Tue, Apr 30, 2019 at 12:51 PM Matthew Garrett <mjg59@google.com> wrote:
> Yes, it looks like this is just broken. Can you try with the attached patch?

Actually, please try this one.

[-- Attachment #2: fix_log.diff --]
[-- Type: text/x-patch, Size: 4138 bytes --]

diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index 2ccaa6661aaf..db0fdaa9c666 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -28,6 +28,7 @@ static int tpm2_calc_event_log_size(void *data, int count, void *size_info)
 		if (event_size == 0)
 			return -1;
 		size += event_size;
+		count--;
 	}
 
 	return size;
@@ -41,6 +42,7 @@ int __init efi_tpm_eventlog_init(void)
 	struct linux_efi_tpm_eventlog *log_tbl;
 	struct efi_tcg2_final_events_table *final_tbl;
 	unsigned int tbl_size;
+	int ret = 0;
 
 	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
 		/*
@@ -60,10 +62,9 @@ int __init efi_tpm_eventlog_init(void)
 
 	tbl_size = sizeof(*log_tbl) + log_tbl->size;
 	memblock_reserve(efi.tpm_log, tbl_size);
-	early_memunmap(log_tbl, sizeof(*log_tbl));
 
 	if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
-		return 0;
+		goto out;
 
 	final_tbl = early_memremap(efi.tpm_final_log, sizeof(*final_tbl));
 
@@ -71,17 +72,20 @@ int __init efi_tpm_eventlog_init(void)
 		pr_err("Failed to map TPM Final Event Log table @ 0x%lx\n",
 		       efi.tpm_final_log);
 		efi.tpm_final_log = EFI_INVALID_TABLE_ADDR;
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 
 	tbl_size = tpm2_calc_event_log_size(final_tbl->events,
 					    final_tbl->nr_events,
-					    (void *)efi.tpm_log);
+					    log_tbl->log);
 	memblock_reserve((unsigned long)final_tbl,
 			 tbl_size + sizeof(*final_tbl));
 	early_memunmap(final_tbl, sizeof(*final_tbl));
 	efi_tpm_final_log_size = tbl_size;
 
-	return 0;
+out:
+	early_memunmap(log_tbl, sizeof(*log_tbl));
+	return ret;
 }
 
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index dccc97e6135c..662410710ac0 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -158,7 +158,6 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 {
 	struct tcg_efi_specid_event_head *efispecid;
 	struct tcg_event_field *event_field;
-	void *mapping = NULL;
 	int mapping_size;
 	void *marker;
 	void *marker_start;
@@ -176,9 +175,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	/* Map the event header */
 	if (do_mapping) {
 		mapping_size = marker - marker_start;
-		mapping = early_memremap((unsigned long)marker_start,
+		event = early_memremap((unsigned long)marker_start,
 					 mapping_size);
-		if (!mapping) {
+		if (!event) {
 			size = 0;
 			goto out;
 		}
@@ -199,9 +198,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 		if (do_mapping) {
 			early_memunmap(mapping, mapping_size);
 			mapping_size = marker - marker_start + halg_size;
-			mapping = early_memremap((unsigned long)marker_start,
+			event = early_memremap((unsigned long)marker_start,
 						 mapping_size);
-			if (!mapping) {
+			if (!event) {
 				size = 0;
 				goto out;
 			}
@@ -219,9 +218,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 				if (do_mapping) {
 					early_memunmap(mapping, mapping_size);
 					mapping_size = marker - marker_start;
-					mapping = early_memremap((unsigned long)marker_start,
+					event = early_memremap((unsigned long)marker_start,
 						      mapping_size);
-					if (!mapping) {
+					if (!event) {
 						size = 0;
 						goto out;
 					}
@@ -243,11 +242,11 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	 * we don't need to map it
 	 */
 	if (do_mapping) {
-		early_memunmap(marker_start, mapping_size);
+		early_memunmap(mapping, mapping_size);
 		mapping_size += sizeof(event_field->event_size);
-		mapping = early_memremap((unsigned long)marker_start,
-					 mapping_size);
-		if (!mapping) {
+		event = early_memremap((unsigned long)marker_start,
+				       mapping_size);
+		if (!event) {
 			size = 0;
 			goto out;
 		}
@@ -257,8 +256,6 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 		+ event_field->event_size;
 	size = marker - marker_start;
 
-	if ((event->event_type == 0) && (event_field->event_size == 0))
-		size = 0;
 out:
 	if (do_mapping)
 		early_memunmap(mapping, mapping_size);

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Bartosz Szczepanek]

Second patch tries to unmap "mapping" which is not declared. I'm on
top of jjs/master and your TPM_MEMREMAP patches are already there, so
the first patch applied cleanly. Using it, kernel still panicked on
boot:

EFI stub: Booting Linux Kernel...
EFI stub: EFI_RNG_PROTOCOL unavailable, no randomness supplied
EFI stub: Using DTB from configuration table
EFI stub: Exiting boot services and installing virtual address map...
[    0.000000] Booting Linux on physical CPU 0x0000000000 [0x420f5162]
[    0.000000] Linux version 5.1.0-rc2+ (root@localhost.localdomain)
(gcc version 7.3.1 20180712 (Red Hat 7.3.1-6) (GCC)) #78 SMP Wed May 1
01:05:38 EDT 2019
[    0.000000] earlycon: pl11 at MMIO 0x0000000402020000 (options '115200n8')
[    0.000000] printk: bootconsole [pl11] enabled
[    0.000000] efi: Getting EFI parameters from FDT:
[    0.000000] efi: EFI v2.60 by Cavium Inc.
TX2-FW-Release-7.2-build_08-0-g14f8c5bf8a Apr 15 2019 18:51:41
[    0.000000] efi:  TPMFinalLog=0xed5f0000  SMBIOS=0xfad90000  SMBIOS
3.0=0xed530000  ACPI 2.0=0xeda90000  ESRT=0xfafdb218
MEMATTR=0xf8489018  TPMEventLog=0xedaa9018  MEMRESERVE=0xedaa8018
[    0.000000] Unhandled fault at 0xffff7dfffe77a018
[    0.000000] Mem abort info:
[    0.000000]   ESR = 0x96000003
[    0.000000]   Exception class = DABT (current EL), IL = 32 bits
[    0.000000]   SET = 0, FnV = 0
[    0.000000]   EA = 0, S1PTW = 0
[    0.000000] Data abort info:
[    0.000000]   ISV = 0, ISS = 0x00000003
[    0.000000]   CM = 0, WnR = 0
[    0.000000] swapper pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____)
[    0.000000] [ffff7dfffe77a018] pgd=0000000081b12003
[    0.000000] ------------[ cut here ]------------
[    0.000000] kernel BUG at arch/arm64/mm/fault.c:189!
[    0.000000] Internal error: Oops - BUG: 0 [#1] SMP
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.1.0-rc2+ #78
[    0.000000] pstate: 60400089 (nZCv daIf +PAN -UAO)
[    0.000000] pc : show_pte+0x1d0/0x1f0
[    0.000000] lr : show_pte+0x88/0x1f0
[    0.000000] sp : ffff000011533b30
[    0.000000] x29: ffff000011533b30 x28: ffff0000115473c0
[    0.000000] x27: ffff000011542500 x26: ffff7dfffe73a010
[    0.000000] x25: 0000000000000018 x24: 0000000000000025
[    0.000000] x23: 00000000000000fb x22: ffff0000117fc000
[    0.000000] x21: ffff000010f32000 x20: ffffffffffffffff
[    0.000000] x19: ffff7dfffe77a018 x18: ffffffffffffffff
[    0.000000] x17: 0000000000000000 x16: 0000000000000000
[    0.000000] x15: ffff00001153d708 x14: ffff00001172f420
[    0.000000] x13: ffff00001172f069 x12: ffff000011568000
[    0.000000] x11: ffff000011533800 x10: ffff000011533800
[    0.000000] x9 : ffff00001153ef58 x8 : 303030303030303d
[    0.000000] x7 : 646770205d383130 x6 : ffff00001172e7ff
[    0.000000] x5 : 0000000000000000 x4 : 0000000000000000
[    0.000000] x3 : 0000000000000000 x2 : 0000000000000000
[    0.000000] x1 : 0000000081b12000 x0 : 0000000000000ff8
[    0.000000] Process swapper (pid: 0, stack limit = 0x(____ptrval____))
[    0.000000] Call trace:
[    0.000000]  show_pte+0x1d0/0x1f0
[    0.000000]  do_mem_abort+0xa8/0xb0
[    0.000000]  el1_da+0x20/0xc4
[    0.000000]  efi_tpm_eventlog_init+0xe8/0x268
[    0.000000]  efi_config_parse_tables+0x180/0x29c
[    0.000000]  uefi_init+0x1d0/0x22c
[    0.000000]  efi_init+0x90/0x180
[    0.000000]  setup_arch+0x1f4/0x5fc
[    0.000000]  start_kernel+0x90/0x51c
[    0.000000] Code: 910d6000 94030b20 17ffffe6 d503201f (d4210000)
[    0.000000] random: get_random_bytes called from
print_oops_end_marker+0x54/0x70 with crng_init=0
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
[    0.000000] ---[ end Kernel panic - not syncing: Attempted to kill
the idle task! ]---

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Ard Biesheuvel]

(+ Ingo)

On Tue, 30 Apr 2019 at 21:52, Matthew Garrett <mjg59@google.com> wrote:
>
> On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek <bsz@semihalf.com> wrote:
> >
> > I may be a little late with this comment, but I've just tested these
> > patches on aarch64 platform (from the top of jjs/master) and got
> > kernel panic ("Unable to handle kernel read", full log at the end of
> > mail). I think there's problem with below call to
> > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > passed as (void *) and never remapped:
>
> Yes, it looks like this is just broken. Can you try with the attached patch?

I'm a bit uncomfortable with EFI code that is obviously broken and
untested being queued for the next merge window in another tree.

What is currently queued there? Can we revert this change for the time
being, and resubmit it via the EFI tree for v5.3?

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Jarkko Sakkinen]

On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> I may be a little late with this comment, but I've just tested these
> patches on aarch64 platform (from the top of jjs/master) and got
> kernel panic ("Unable to handle kernel read", full log at the end of
> mail). I think there's problem with below call to
> tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> passed as (void *) and never remapped:

Not late. This is not part of any PR yet. Thank you for the
feedback!

Matthew, can you send an updated version of the whole patch set
with fixes to this issue and also reordering of the includes?

/Jarkko

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Matthew Garrett]

On Thu, May 2, 2019 at 1:32 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> > I may be a little late with this comment, but I've just tested these
> > patches on aarch64 platform (from the top of jjs/master) and got
> > kernel panic ("Unable to handle kernel read", full log at the end of
> > mail). I think there's problem with below call to
> > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > passed as (void *) and never remapped:
>
> Not late. This is not part of any PR yet. Thank you for the
> feedback!
>
> Matthew, can you send an updated version of the whole patch set
> with fixes to this issue and also reordering of the includes?

Yes, I'll resend and let's do this again for 5.3.

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Matthew Garrett]

On Thu, May 2, 2019 at 12:15 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> (+ Ingo)
>
> On Tue, 30 Apr 2019 at 21:52, Matthew Garrett <mjg59@google.com> wrote:
> >
> > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek <bsz@semihalf.com> wrote:
> > >
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Yes, it looks like this is just broken. Can you try with the attached patch?
>
> I'm a bit uncomfortable with EFI code that is obviously broken and
> untested being queued for the next merge window in another tree.

The patchset was Cc:ed to linux-efi@. Is there anything else I should
have done to ensure you picked it up rather than Jarkko?

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Matthew Garrett]

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

Sorry, how about this one? I was confused by why I wasn't hitting
this, but on closer examination it turns out that my system populates
the final event log with 0 events which means we never hit this
codepath :(

[-- Attachment #2: fix_log.diff --]
[-- Type: text/x-patch, Size: 4136 bytes --]

diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index 2ccaa6661aaf..db0fdaa9c666 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -28,6 +28,7 @@ static int tpm2_calc_event_log_size(void *data, int count, void *size_info)
 		if (event_size == 0)
 			return -1;
 		size += event_size;
+		count--;
 	}
 
 	return size;
@@ -41,6 +42,7 @@ int __init efi_tpm_eventlog_init(void)
 	struct linux_efi_tpm_eventlog *log_tbl;
 	struct efi_tcg2_final_events_table *final_tbl;
 	unsigned int tbl_size;
+	int ret = 0;
 
 	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
 		/*
@@ -60,10 +62,9 @@ int __init efi_tpm_eventlog_init(void)
 
 	tbl_size = sizeof(*log_tbl) + log_tbl->size;
 	memblock_reserve(efi.tpm_log, tbl_size);
-	early_memunmap(log_tbl, sizeof(*log_tbl));
 
 	if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
-		return 0;
+		goto out;
 
 	final_tbl = early_memremap(efi.tpm_final_log, sizeof(*final_tbl));
 
@@ -71,17 +72,20 @@ int __init efi_tpm_eventlog_init(void)
 		pr_err("Failed to map TPM Final Event Log table @ 0x%lx\n",
 		       efi.tpm_final_log);
 		efi.tpm_final_log = EFI_INVALID_TABLE_ADDR;
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 
 	tbl_size = tpm2_calc_event_log_size(final_tbl->events,
 					    final_tbl->nr_events,
-					    (void *)efi.tpm_log);
+					    log_tbl->log);
 	memblock_reserve((unsigned long)final_tbl,
 			 tbl_size + sizeof(*final_tbl));
 	early_memunmap(final_tbl, sizeof(*final_tbl));
 	efi_tpm_final_log_size = tbl_size;
 
-	return 0;
+out:
+	early_memunmap(log_tbl, sizeof(*log_tbl));
+	return ret;
 }
 
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index dccc97e6135c..190a33968a91 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -158,7 +158,6 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 {
 	struct tcg_efi_specid_event_head *efispecid;
 	struct tcg_event_field *event_field;
-	void *mapping = NULL;
 	int mapping_size;
 	void *marker;
 	void *marker_start;
@@ -176,9 +175,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	/* Map the event header */
 	if (do_mapping) {
 		mapping_size = marker - marker_start;
-		mapping = early_memremap((unsigned long)marker_start,
+		event = early_memremap((unsigned long)marker_start,
 					 mapping_size);
-		if (!mapping) {
+		if (!event) {
 			size = 0;
 			goto out;
 		}
@@ -199,9 +198,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 		if (do_mapping) {
 			early_memunmap(mapping, mapping_size);
 			mapping_size = marker - marker_start + halg_size;
-			mapping = early_memremap((unsigned long)marker_start,
+			event = early_memremap((unsigned long)marker_start,
 						 mapping_size);
-			if (!mapping) {
+			if (!event) {
 				size = 0;
 				goto out;
 			}
@@ -219,9 +218,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 				if (do_mapping) {
 					early_memunmap(mapping, mapping_size);
 					mapping_size = marker - marker_start;
-					mapping = early_memremap((unsigned long)marker_start,
+					event = early_memremap((unsigned long)marker_start,
 						      mapping_size);
-					if (!mapping) {
+					if (!event) {
 						size = 0;
 						goto out;
 					}
@@ -243,11 +242,11 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	 * we don't need to map it
 	 */
 	if (do_mapping) {
-		early_memunmap(marker_start, mapping_size);
+		early_memunmap(event, mapping_size);
 		mapping_size += sizeof(event_field->event_size);
-		mapping = early_memremap((unsigned long)marker_start,
-					 mapping_size);
-		if (!mapping) {
+		event = early_memremap((unsigned long)marker_start,
+				       mapping_size);
+		if (!event) {
 			size = 0;
 			goto out;
 		}
@@ -257,8 +256,6 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 		+ event_field->event_size;
 	size = marker - marker_start;
 
-	if ((event->event_type == 0) && (event_field->event_size == 0))
-		size = 0;
 out:
 	if (do_mapping)
 		early_memunmap(mapping, mapping_size);

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Ard Biesheuvel]

On Thu, 2 May 2019 at 20:04, Matthew Garrett <mjg59@google.com> wrote:
>
> On Thu, May 2, 2019 at 12:15 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > (+ Ingo)
> >
> > On Tue, 30 Apr 2019 at 21:52, Matthew Garrett <mjg59@google.com> wrote:
> > >
> > > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek <bsz@semihalf.com> wrote:
> > > >
> > > > I may be a little late with this comment, but I've just tested these
> > > > patches on aarch64 platform (from the top of jjs/master) and got
> > > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > > mail). I think there's problem with below call to
> > > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > > passed as (void *) and never remapped:
> > >
> > > Yes, it looks like this is just broken. Can you try with the attached patch?
> >
> > I'm a bit uncomfortable with EFI code that is obviously broken and
> > untested being queued for the next merge window in another tree.
>
> The patchset was Cc:ed to linux-efi@. Is there anything else I should
> have done to ensure you picked it up rather than Jarkko?

No, I am not saying it was you who did anything wrong - Jarkko and I
should probably have aligned better. But my own testing wouldn't have
caught this particular issue either (I am still in the process of
getting access to ARM machines with a TPM), so it wouldn't have made a
huge difference in any case.

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Jarkko Sakkinen]

On Thu, May 02, 2019 at 11:03:08AM -0700, Matthew Garrett wrote:
> On Thu, May 2, 2019 at 1:32 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Not late. This is not part of any PR yet. Thank you for the
> > feedback!
> >
> > Matthew, can you send an updated version of the whole patch set
> > with fixes to this issue and also reordering of the includes?
> 
> Yes, I'll resend and let's do this again for 5.3.

Agreed, better not rush but get it right once.

/Jarkko

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Jarkko Sakkinen]

On Thu, May 02, 2019 at 09:14:49AM +0200, Ard Biesheuvel wrote:
> (+ Ingo)
> 
> On Tue, 30 Apr 2019 at 21:52, Matthew Garrett <mjg59@google.com> wrote:
> >
> > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek <bsz@semihalf.com> wrote:
> > >
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Yes, it looks like this is just broken. Can you try with the attached patch?
> 
> I'm a bit uncomfortable with EFI code that is obviously broken and
> untested being queued for the next merge window in another tree.
> 
> What is currently queued there? Can we revert this change for the time
> being, and resubmit it via the EFI tree for v5.3?

Nothing ATM. The broken code is in my master branch ATM. Nothing
in my next branch nor have I included anything to my PRs. Should
be nothing to worry about in that sense :-)

/Jarkko

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Ingo Molnar]

* Matthew Garrett <mjg59@google.com> wrote:

> On Thu, May 2, 2019 at 12:15 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > (+ Ingo)
> >
> > On Tue, 30 Apr 2019 at 21:52, Matthew Garrett <mjg59@google.com> wrote:
> > >
> > > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek <bsz@semihalf.com> wrote:
> > > >
> > > > I may be a little late with this comment, but I've just tested these
> > > > patches on aarch64 platform (from the top of jjs/master) and got
> > > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > > mail). I think there's problem with below call to
> > > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > > passed as (void *) and never remapped:
> > >
> > > Yes, it looks like this is just broken. Can you try with the attached patch?
> >
> > I'm a bit uncomfortable with EFI code that is obviously broken and
> > untested being queued for the next merge window in another tree.
> 
> The patchset was Cc:ed to linux-efi@. Is there anything else I should
> have done to ensure you picked it up rather than Jarkko?

That's not the workflow rule the Linux kernel is using, if Cc:-ing a 
patchset was the only condition for upstream inclusion then we'd have a 
*LOT* of crap in the Linux kernel.

Just applying those EFI changes without even as much as an Acked-by from 
the EFI maintainers is a *totally* unacceptable workflow.

Please revert/rebase and re-try this on the proper submission channels.

Meanwhile the broken code is NAK-ed by me:

   Nacked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Jarkko Sakkinen]

On Fri, May 03, 2019 at 08:02:18AM +0200, Ingo Molnar wrote:
> 
> * Matthew Garrett <mjg59@google.com> wrote:
> 
> > On Thu, May 2, 2019 at 12:15 AM Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> > >
> > > (+ Ingo)
> > >
> > > On Tue, 30 Apr 2019 at 21:52, Matthew Garrett <mjg59@google.com> wrote:
> > > >
> > > > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek <bsz@semihalf.com> wrote:
> > > > >
> > > > > I may be a little late with this comment, but I've just tested these
> > > > > patches on aarch64 platform (from the top of jjs/master) and got
> > > > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > > > mail). I think there's problem with below call to
> > > > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > > > passed as (void *) and never remapped:
> > > >
> > > > Yes, it looks like this is just broken. Can you try with the attached patch?
> > >
> > > I'm a bit uncomfortable with EFI code that is obviously broken and
> > > untested being queued for the next merge window in another tree.
> > 
> > The patchset was Cc:ed to linux-efi@. Is there anything else I should
> > have done to ensure you picked it up rather than Jarkko?
> 
> That's not the workflow rule the Linux kernel is using, if Cc:-ing a 
> patchset was the only condition for upstream inclusion then we'd have a 
> *LOT* of crap in the Linux kernel.
> 
> Just applying those EFI changes without even as much as an Acked-by from 
> the EFI maintainers is a *totally* unacceptable workflow.
> 
> Please revert/rebase and re-try this on the proper submission channels.
> 
> Meanwhile the broken code is NAK-ed by me:
> 
>    Nacked-by: Ingo Molnar <mingo@kernel.org>

There must be some kind of misconception here. None of the changes have
been submitted so far. They are only in my master branch. They briefly
went to linux-next through my next branch but as soon as issues were
reported I wiped them off from there (which happened like 2-3 weeks
ago). They haven't been part off any of my PR's.

There is nothing to revert.

/Jarkko

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Bartosz Szczepanek]

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

Nope, it doesn't work. It compiled (after correcting one more leftover
mapping), but panicked the same way.

I've came up with a set of changes that make it working in my setup,
see attached patch. There was a problem with passing already remapped
address to tpm2_calc_event_log_size(), which tried to remap it for
second time. Few more adjustments were needed in remap-as-you-go code
so that new address is used consequently. Also, I've removed remapping
digest itself because it was never used.

I'm not certain these changes make it 100% correct, but it worked on
35 events I had in final log. Its ending seems fine now:
> [root@localhost ~]# hexdump -C /sys/kernel/security/tpm0/binary_bios_measurements | tail
> 00003720  42 6f 6f 74 20 53 65 72  76 69 63 65 73 20 49 6e  |Boot Services In|
> 00003730  76 6f 63 61 74 69 6f 6e  05 00 00 00 07 00 00 80  |vocation........|
> 00003740  02 00 00 00 04 00 47 55  45 dd c9 78 d7 bf d0 36  |......GUE..x...6|
> 00003750  fa cc 7e 2e 98 7f 48 18  9f 0d 0b 00 b5 4f 75 42  |..~...H......OuB|
> 00003760  cb d8 72 a8 1a 9d 9d ea  83 9b 2b 8d 74 7c 7e bd  |..r.......+.t|~.|
> 00003770  5e a6 61 5c 40 f4 2f 44  a6 db eb a0 28 00 00 00  |^.a\@./D....(...|
> 00003780  45 78 69 74 20 42 6f 6f  74 20 53 65 72 76 69 63  |Exit Boot Servic|
> 00003790  65 73 20 52 65 74 75 72  6e 65 64 20 77 69 74 68  |es Returned with|
> 000037a0  20 53 75 63 63 65 73 73                           | Success|
> 000037a8

Still, some refactoring could help here as __calc_tpm2_event_size has
grown and its logic became hard to follow. IMO it's far too complex
for inline function.

Attached patch should be applied on top of jjs/master.

Bartosz

[-- Attachment #2: eventlog_fix.diff --]
[-- Type: text/x-patch, Size: 4227 bytes --]

diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index fe48150f06d1..2c912ea08166 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -28,6 +28,7 @@ static int tpm2_calc_event_log_size(void *data, int count, void *size_info)
 		if (event_size == 0)
 			return -1;
 		size += event_size;
+		count--;
 	}
 
 	return size;
@@ -41,6 +42,7 @@ int __init efi_tpm_eventlog_init(void)
 	struct linux_efi_tpm_eventlog *log_tbl;
 	struct efi_tcg2_final_events_table *final_tbl;
 	unsigned int tbl_size;
+	int ret = 0;
 
 	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
 		/*
@@ -60,10 +62,9 @@ int __init efi_tpm_eventlog_init(void)
 
 	tbl_size = sizeof(*log_tbl) + log_tbl->size;
 	memblock_reserve(efi.tpm_log, tbl_size);
-	early_memunmap(log_tbl, sizeof(*log_tbl));
 
 	if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
-		return 0;
+		goto out;
 
 	final_tbl = early_memremap(efi.tpm_final_log, sizeof(*final_tbl));
 
@@ -71,17 +72,22 @@ int __init efi_tpm_eventlog_init(void)
 		pr_err("Failed to map TPM Final Event Log table @ 0x%lx\n",
 		       efi.tpm_final_log);
 		efi.tpm_final_log = EFI_INVALID_TABLE_ADDR;
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 
-	tbl_size = tpm2_calc_event_log_size(final_tbl->events,
+	tbl_size = tpm2_calc_event_log_size(efi.tpm_final_log
+					    + sizeof(final_tbl->version)
+					    + sizeof(final_tbl->nr_events),
 					    final_tbl->nr_events,
-					    (void *)efi.tpm_log);
+					    log_tbl->log);
 	memblock_reserve((unsigned long)final_tbl,
 			 tbl_size + sizeof(*final_tbl));
 	early_memunmap(final_tbl, sizeof(*final_tbl));
 	efi_tpm_final_log_size = tbl_size;
 
-	return 0;
+out:
+	early_memunmap(log_tbl, sizeof(*log_tbl));
+	return ret;
 }
 
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 0ca27bc053af..63238c84dc0b 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -185,8 +185,12 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 			size = 0;
 			goto out;
 		}
+	} else {
+		mapping = marker_start;
 	}
 
+	event = (struct tcg_pcr_event2_head *)mapping;
+
 	efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
 
 	/* Check if event is malformed. */
@@ -201,34 +205,24 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 		/* Map the digest's algorithm identifier */
 		if (do_mapping) {
 			TPM_MEMUNMAP(mapping, mapping_size);
-			mapping_size = marker - marker_start + halg_size;
-			mapping = TPM_MEMREMAP((unsigned long)marker_start,
-					       mapping_size);
+			mapping_size = halg_size;
+			mapping = TPM_MEMREMAP((unsigned long)marker,
+					     mapping_size);
 			if (!mapping) {
 				size = 0;
 				goto out;
 			}
+		} else {
+			mapping = marker;
 		}
 
-		memcpy(&halg, marker, halg_size);
+		memcpy(&halg, mapping, halg_size);
 		marker = marker + halg_size;
 
 		for (j = 0; j < efispecid->num_algs; j++) {
 			if (halg == efispecid->digest_sizes[j].alg_id) {
 				marker +=
 					efispecid->digest_sizes[j].digest_size;
-
-				/* Map the digest content itself */
-				if (do_mapping) {
-					TPM_MEMUNMAP(mapping, mapping_size);
-					mapping_size = marker - marker_start;
-					mapping = TPM_MEMREMAP((unsigned long)marker_start,
-							       mapping_size);
-					if (!mapping) {
-						size = 0;
-						goto out;
-					}
-				}
 				break;
 			}
 		}
@@ -239,23 +233,25 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 		}
 	}
 
-	event_field = (struct tcg_event_field *)marker;
-
 	/*
 	 * Map the event size - we don't read from the event itself, so
 	 * we don't need to map it
 	 */
 	if (do_mapping) {
-		TPM_MEMUNMAP(marker_start, mapping_size);
+		TPM_MEMUNMAP(mapping, mapping_size);
 		mapping_size += sizeof(event_field->event_size);
-		mapping = TPM_MEMREMAP((unsigned long)marker_start,
+		mapping = TPM_MEMREMAP((unsigned long)marker,
 				       mapping_size);
 		if (!mapping) {
 			size = 0;
 			goto out;
 		}
+	} else {
+		mapping = marker;
 	}
 
+	event_field = (struct tcg_event_field *)mapping;
+
 	marker = marker + sizeof(event_field->event_size)
 		+ event_field->event_size;
 	size = marker - marker_start;