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

[PATCH V2 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     | 50 ++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 46 deletions(-)

diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
index d8b77133a83a..89a8b1c10939 100644
--- a/drivers/char/tpm/eventlog/tpm2.c
+++ b/drivers/char/tpm/eventlog/tpm2.c
@@ -40,52 +40,7 @@
 static 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;
+	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..b1b8350c238f 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -112,4 +112,54 @@ struct tcg_pcr_event2_head {
 	struct tpm_digest digests[];
 } __packed;
 
+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.20.1.611.gfbb209baf1-goog

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

[Jarkko Sakkinen]

On Mon, Feb 04, 2019 at 01:33:00PM -0800, Matthew Garrett wrote:
> +static inline int _calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
> +					struct tcg_pcr_event *event_header)

Maybe __calc_tpm2_event_size().

/Jarkko

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

[Jarkko Sakkinen]

On Mon, Feb 11, 2019 at 04:28:00PM +0200, Jarkko Sakkinen wrote:
> On Mon, Feb 04, 2019 at 01:33:00PM -0800, Matthew Garrett wrote:
> > +static inline int _calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
> > +					struct tcg_pcr_event *event_header)
> 
> Maybe __calc_tpm2_event_size().

Nice to have: add a kdoc since the function is now public e.g. a brief
description how the event size is calculated.

/Jarkko

[PATCH V2 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       | 67 ++++++++++++++++++----
 include/linux/efi.h              |  9 +++
 include/linux/tpm_eventlog.h     | 98 +++++++++++++++++++++++++++++---
 5 files changed, 160 insertions(+), 18 deletions(-)

diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
index 89a8b1c10939..54976643ea2d 100644
--- a/drivers/char/tpm/eventlog/tpm2.c
+++ b/drivers/char/tpm/eventlog/tpm2.c
@@ -40,7 +40,7 @@
 static int 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, NULL, NULL);
 }
 
 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..19f742cf0d33 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -10,31 +10,78 @@
 #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);
+
+int tpm2_event_log_length(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,
+						   early_memremap,
+						   early_memunmap);
+		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) {
+		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_INVALID_TABLE_ADDR;
+			return -ENOMEM;
+		}
+
+		tbl_size = sizeof(*log_tbl) + log_tbl->size;
+		memblock_reserve(efi.tpm_log, tbl_size);
+		early_memunmap(log_tbl, sizeof(*log_tbl));
+	} else {
+		/*
+		 * We can't calculate the size of the final events without the
+		 * first entry in the TPM log, so bail here.
+		 */
 		return 0;
+	}
+
+	if (efi.tpm_final_log != EFI_INVALID_TABLE_ADDR) {
+		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;
+		}
 
-	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_INVALID_TABLE_ADDR;
-		return -ENOMEM;
+		tbl_size = tpm2_event_log_length(final_tbl->events,
+						 final_tbl->number_of_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;
 	}
 
-	tbl_size = sizeof(*log_tbl) + log_tbl->size;
-	memblock_reserve(efi.tpm_log, tbl_size);
-	early_memunmap(log_tbl, sizeof(*log_tbl));
 	return 0;
 }
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 45ff763fba76..a6134c301fa7 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 number_of_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 b1b8350c238f..72e995bfc3e7 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -112,11 +112,37 @@ 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[];
+};
+
+/*
+ * This can be called in two contexts - when the event is already mapped,
+ * and when it isn't. In the latter case we don't know how much space we
+ * need to map in advance so need to jump through this repeated map/unmap
+ * dance as we learn more about the size of the event.
+ */
 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,
+				  void *(*map)(resource_size_t, unsigned long),
+				  void (*unmap)(void *, unsigned long))
 {
 	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;
@@ -130,36 +156,94 @@ 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 (map) {
+		mapping_size = marker - marker_start;
+		mapping = map((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 (map && unmap) {
+			unmap(mapping, mapping_size);
+			mapping_size = marker - marker_start + halg_size;
+			mapping = map((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 (map && unmap) {
+					unmap(mapping, mapping_size);
+					mapping_size = marker - marker_start;
+					mapping = map((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 (map && unmap) {
+		unmap(marker_start, mapping_size);
+		mapping_size += sizeof(event_field->event_size);
+		mapping = map((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 (unmap)
+		unmap(mapping, mapping_size);
 	return size;
 }
+
 #endif
-- 
2.20.1.611.gfbb209baf1-goog

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

[Jarkko Sakkinen]

On Mon, Feb 04, 2019 at 01:33:01PM -0800, Matthew Garrett wrote:
> 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       | 67 ++++++++++++++++++----
>  include/linux/efi.h              |  9 +++
>  include/linux/tpm_eventlog.h     | 98 +++++++++++++++++++++++++++++---
>  5 files changed, 160 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
> index 89a8b1c10939..54976643ea2d 100644
> --- a/drivers/char/tpm/eventlog/tpm2.c
> +++ b/drivers/char/tpm/eventlog/tpm2.c
> @@ -40,7 +40,7 @@
>  static int 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, NULL, NULL);
>  }
>  
>  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..19f742cf0d33 100644
> --- a/drivers/firmware/efi/tpm.c
> +++ b/drivers/firmware/efi/tpm.c
> @@ -10,31 +10,78 @@
>  #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);
> +
> +int tpm2_event_log_length(void *data, int count, void *size_info)

Should be a static function and for consistency's sake the name should
be tpm2_calc_event_log_size().

> +{
> +	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,
> +						   early_memremap,
> +						   early_memunmap);
> +		if (event_size == 0)
> +			return -1;
> +		size += event_size;
> +	}

Would add a newline here.

> +	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) {
> +		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_INVALID_TABLE_ADDR;
> +			return -ENOMEM;
> +		}
> +
> +		tbl_size = sizeof(*log_tbl) + log_tbl->size;
> +		memblock_reserve(efi.tpm_log, tbl_size);
> +		early_memunmap(log_tbl, sizeof(*log_tbl));
> +	} else {
> +		/*
> +		 * We can't calculate the size of the final events without the
> +		 * first entry in the TPM log, so bail here.
> +		 */
>  		return 0;
> +	}

Not really sure why this change to branching in the first part of this
function. Would reduce the diff and review the actually relevant parts
if you didn't do this.

> +
> +	if (efi.tpm_final_log != EFI_INVALID_TABLE_ADDR) {

Wondering that should this function instead do right `in the beginning:

if (efi.tpm_log == EFI_INVALID_TABLE_ADDR &&
    efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
	return 0;

Feels odd condition that the log would not be invalid but the post log
(using post would be more self-describing than final imho) would be. Can
that legitly happen?


> +		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;
> +		}
>  
> -	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_INVALID_TABLE_ADDR;
> -		return -ENOMEM;
> +		tbl_size = tpm2_event_log_length(final_tbl->events,
> +						 final_tbl->number_of_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;
>  	}
>  
> -	tbl_size = sizeof(*log_tbl) + log_tbl->size;
> -	memblock_reserve(efi.tpm_log, tbl_size);
> -	early_memunmap(log_tbl, sizeof(*log_tbl));
>  	return 0;
>  }
>  
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 45ff763fba76..a6134c301fa7 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 number_of_events;

Would name this simply as nr_events as nr_ is so commonly used prefix in
the kernel.

> +	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 b1b8350c238f..72e995bfc3e7 100644
> --- a/include/linux/tpm_eventlog.h
> +++ b/include/linux/tpm_eventlog.h
> @@ -112,11 +112,37 @@ 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[];
> +};
> +
> +/*
> + * This can be called in two contexts - when the event is already mapped,
> + * and when it isn't. In the latter case we don't know how much space we
> + * need to map in advance so need to jump through this repeated map/unmap
> + * dance as we learn more about the size of the event.
> + */
>  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,
> +				  void *(*map)(resource_size_t, unsigned long),
> +				  void (*unmap)(void *, unsigned long))

Should replace with bool do_mapping instead of doing indirect calls for
on reason (and thus introducing retpoline's). I don't think call sites
should be enumerated. A proper kdoc would just describe parameters and
their purpose.

>  {
>  	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;
> @@ -130,36 +156,94 @@ 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 (map) {
> +		mapping_size = marker - marker_start;
> +		mapping = map((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 (map && unmap) {
> +			unmap(mapping, mapping_size);
> +			mapping_size = marker - marker_start + halg_size;
> +			mapping = map((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 (map && unmap) {
> +					unmap(mapping, mapping_size);
> +					mapping_size = marker - marker_start;
> +					mapping = map((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 (map && unmap) {
> +		unmap(marker_start, mapping_size);
> +		mapping_size += sizeof(event_field->event_size);
> +		mapping = map((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 (unmap)
> +		unmap(mapping, mapping_size);
>  	return size;
>  }
> +
>  #endif
> -- 
> 2.20.1.611.gfbb209baf1-goog
> 

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

[Matthew Garrett]

On Mon, Feb 11, 2019 at 8:39 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
> On Mon, Feb 04, 2019 at 01:33:01PM -0800, Matthew Garrett wrote:
> > +int tpm2_event_log_length(void *data, int count, void *size_info)
>
> Should be a static function and for consistency's sake the name should
> be tpm2_calc_event_log_size().

Done.

> > +             size += event_size;
> > +     }
>
> Would add a newline here.

Done.

> Not really sure why this change to branching in the first part of this
> function. Would reduce the diff and review the actually relevant parts
> if you didn't do this.

Done.

> > +
> > +     if (efi.tpm_final_log != EFI_INVALID_TABLE_ADDR) {
>
> Wondering that should this function instead do right `in the beginning:
>
> if (efi.tpm_log == EFI_INVALID_TABLE_ADDR &&
>     efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
>         return 0;
>
> Feels odd condition that the log would not be invalid but the post log
> (using post would be more self-describing than final imho) would be. Can
> that legitly happen?

The spec name is final, so I kept it that way for consistency. Keeping
a separate check for the final event log is partly out of
defensiveness against firmware implementations getting this wrong -
I've definitely found implementations that just don't produce any
final events, so it wouldn't surprise me if there are some that don't
install the table.

> > +struct efi_tcg2_final_events_table {
> > +     u64 version;
> > +     u64 number_of_events;
>
> Would name this simply as nr_events as nr_ is so commonly used prefix in
> the kernel.

Done.

> >  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,
> > +                               void *(*map)(resource_size_t, unsigned long),
> > +                               void (*unmap)(void *, unsigned long))
>
> Should replace with bool do_mapping instead of doing indirect calls for
> on reason (and thus introducing retpoline's). I don't think call sites
> should be enumerated. A proper kdoc would just describe parameters and
> their purpose.

Done. I'll send an updated patchset.

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

[Jarkko Sakkinen]

On Mon, Feb 11, 2019 at 12:58:21PM -0800, Matthew Garrett wrote:
> > Wondering that should this function instead do right `in the beginning:
> >
> > if (efi.tpm_log == EFI_INVALID_TABLE_ADDR &&
> >     efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
> >         return 0;
> >
> > Feels odd condition that the log would not be invalid but the post log
> > (using post would be more self-describing than final imho) would be. Can
> > that legitly happen?
> 
> The spec name is final, so I kept it that way for consistency. Keeping
> a separate check for the final event log is partly out of
> defensiveness against firmware implementations getting this wrong -
> I've definitely found implementations that just don't produce any
> final events, so it wouldn't surprise me if there are some that don't
> install the table.

OK, sounds very reasonable.

> Done. I'll send an updated patchset.

Awesome, thank you.

/Jarkko

[PATCH V2 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 | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/eventlog/efi.c b/drivers/char/tpm/eventlog/efi.c
index 3e673ab22cb4..80e9ec28a9be 100644
--- a/drivers/char/tpm/eventlog/efi.c
+++ b/drivers/char/tpm/eventlog/efi.c
@@ -21,10 +21,12 @@
 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;
 
 	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
 		return -ENODEV;
@@ -55,12 +57,41 @@ int tpm_read_log_efi(struct tpm_chip *chip)
 	if (!log->bios_event_log)
 		goto err_memunmap;
 	log->bios_event_log_end = log->bios_event_log + log_size;
-
 	tpm_log_version = log_tbl->version;
+
+	if (efi.tpm_final_log != EFI_INVALID_TABLE_ADDR &&
+	    efi_tpm_final_log_size != 0) {
+		if (tpm_log_version == EFI_TCG2_EVENT_LOG_FORMAT_TCG_2) {
+			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);
+				goto err_memunmap;
+			}
+
+			tmp = krealloc(log->bios_event_log,
+				       log_size + efi_tpm_final_log_size,
+				       GFP_KERNEL);
+			if (!tmp) {
+				kfree(log->bios_event_log);
+				goto err_memunmap;
+			}
+
+			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;
+		}
+	}
+	memunmap(final_tbl);
 	memunmap(log_tbl);
 	return tpm_log_version;
 
 err_memunmap:
+	memunmap(final_tbl);
 	memunmap(log_tbl);
 	return -ENOMEM;
 }
-- 
2.20.1.611.gfbb209baf1-goog

[PATCH V2 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..7d4a045aa2c4 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,
+						      NULL, NULL);
+		} 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.20.1.611.gfbb209baf1-goog

Re: your mail

[Jarkko Sakkinen]

On Mon, Feb 04, 2019 at 01:32:59PM -0800, Matthew Garrett wrote:
> V2: Rebased on top of Roberto's patches, no other changes. Tested on an
> Intel NUC using PTT-based TPM2, binary_bios_measurements contains boot
> events.

Thank you. Looking forward to review this. I'll land Roberto's patches
and then this would be next in the queue.

/Jarkko