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

From: Matthew Garrett
Date: Thu May 02 2019 - 14:08:19 EST


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 :(
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);