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

[Matthew Garrett <mjg59@google.com>]

[-- 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;
 }