Re: [PATCH V2 2/2] tpm: Don't duplicate events from the final event log in the TCG2 log

2019-06-13 Thread Jarkko Sakkinen
On Fri, Jun 07, 2019 at 01:51:47PM -0700, Matthew Garrett wrote:
> After the first call to GetEventLog() on UEFI systems using the TCG2
> crypto agile log format, any further log events (other than those
> triggered by ExitBootServices()) will be logged in both the main log and
> also in the Final Events Log. While the kernel only calls GetEventLog()
> immediately before ExitBootServices(), we can't control whether earlier
> parts of the boot process have done so. This will result in log entries
> that exist in both logs, and so the current approach of simply appending
> the Final Event Log to the main log will result in events being
> duplicated.
> 
> We can avoid this problem by looking at the size of the Final Event Log
> just before we call ExitBootServices() and exporting this to the main
> kernel. The kernel can then skip over all events that occured before
> ExitBootServices() and only append events that were not also logged to
> the main log.
> 
> Signed-off-by: Matthew Garrett 
> Reported-by: Joe Richey 
> Suggested-by: Joe Richey 

Reviewed-by: Jarkko Sakkinen 

/Jarkko


Re: [PATCH V2 2/2] tpm: Don't duplicate events from the final event log in the TCG2 log

2019-06-13 Thread Jarkko Sakkinen
On Fri, Jun 07, 2019 at 11:11:21PM +0200, Ard Biesheuvel wrote:
> Acked-by: Ard Biesheuvel 

Ard, is it cool if I include these to my next TPM PR along with the
other Matthew's changes? Just sanity checking given that crossing
subsystems...

/Jarkko


Re: [PATCH V2 2/2] tpm: Don't duplicate events from the final event log in the TCG2 log

2019-06-13 Thread Ard Biesheuvel
On Thu, 13 Jun 2019 at 16:06, Jarkko Sakkinen
 wrote:
>
> On Fri, Jun 07, 2019 at 11:11:21PM +0200, Ard Biesheuvel wrote:
> > Acked-by: Ard Biesheuvel 
>
> Ard, is it cool if I include these to my next TPM PR along with the
> other Matthew's changes? Just sanity checking given that crossing
> subsystems...
>

Yes, that is fine.


Re: [PATCH V2 2/2] tpm: Don't duplicate events from the final event log in the TCG2 log

2019-06-07 Thread Ard Biesheuvel
On Fri, 7 Jun 2019 at 22:52, Matthew Garrett  wrote:
>
> After the first call to GetEventLog() on UEFI systems using the TCG2
> crypto agile log format, any further log events (other than those
> triggered by ExitBootServices()) will be logged in both the main log and
> also in the Final Events Log. While the kernel only calls GetEventLog()
> immediately before ExitBootServices(), we can't control whether earlier
> parts of the boot process have done so. This will result in log entries
> that exist in both logs, and so the current approach of simply appending
> the Final Event Log to the main log will result in events being
> duplicated.
>
> We can avoid this problem by looking at the size of the Final Event Log
> just before we call ExitBootServices() and exporting this to the main
> kernel. The kernel can then skip over all events that occured before
> ExitBootServices() and only append events that were not also logged to
> the main log.
>
> Signed-off-by: Matthew Garrett 
> Reported-by: Joe Richey 
> Suggested-by: Joe Richey 
> ---
>  drivers/char/tpm/eventlog/efi.c| 11 ++-
>  drivers/firmware/efi/libstub/tpm.c | 30 ++
>  drivers/firmware/efi/tpm.c |  2 +-
>  include/linux/efi.h|  1 +
>  4 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/eventlog/efi.c b/drivers/char/tpm/eventlog/efi.c
> index 9179cf6bdee9..be6540f2cb3d 100644
> --- a/drivers/char/tpm/eventlog/efi.c
> +++ b/drivers/char/tpm/eventlog/efi.c
> @@ -80,6 +80,8 @@ int tpm_read_log_efi(struct tpm_chip *chip)
> goto out;
> }
>
> +   efi_tpm_final_log_size -= log_tbl->final_events_preboot_size;
> +
> tmp = krealloc(log->bios_event_log,
>log_size + efi_tpm_final_log_size,
>GFP_KERNEL);
> @@ -90,8 +92,15 @@ int tpm_read_log_efi(struct tpm_chip *chip)
> }
>
> log->bios_event_log = tmp;
> +
> +   /*
> +* Copy any of the final events log that didn't also end up in the
> +* main log. Events can be logged in both if events are generated
> +* between GetEventLog() and ExitBootServices().
> +*/
> memcpy((void *)log->bios_event_log + log_size,
> -  final_tbl->events, efi_tpm_final_log_size);
> +  final_tbl->events + log_tbl->final_events_preboot_size,
> +  efi_tpm_final_log_size);
> log->bios_event_log_end = log->bios_event_log +
> log_size + efi_tpm_final_log_size;
>
> diff --git a/drivers/firmware/efi/libstub/tpm.c 
> b/drivers/firmware/efi/libstub/tpm.c
> index 6b3b507a54eb..eb9af83e4d59 100644
> --- a/drivers/firmware/efi/libstub/tpm.c
> +++ b/drivers/firmware/efi/libstub/tpm.c
> @@ -64,11 +64,13 @@ void efi_retrieve_tpm2_eventlog(efi_system_table_t 
> *sys_table_arg)
> efi_status_t status;
> efi_physical_addr_t log_location = 0, log_last_entry = 0;
> struct linux_efi_tpm_eventlog *log_tbl = NULL;
> +   struct efi_tcg2_final_events_table *final_events_table;
> 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;
> +   int final_events_size = 0;
>
> status = efi_call_early(locate_protocol, _guid, NULL,
> _protocol);
> @@ -134,8 +136,36 @@ void efi_retrieve_tpm2_eventlog(efi_system_table_t 
> *sys_table_arg)
> return;
> }
>
> +   /*
> +* Figure out whether any events have already been logged to the
> +* final events structure, and if so how much space they take up
> +*/
> +   final_events_table = get_efi_config_table(sys_table_arg,
> +   LINUX_EFI_TPM_FINAL_LOG_GUID);
> +   if (final_events_table && final_events_table->nr_events) {
> +   struct tcg_pcr_event2_head *header;
> +   int offset;
> +   void *data;
> +   int event_size;
> +   int i = final_events_table->nr_events;
> +
> +   data = (void *)final_events_table;
> +   offset = sizeof(final_events_table->version) +
> +   sizeof(final_events_table->nr_events);
> +
> +   while (i > 0) {
> +   header = data + offset + final_events_size;
> +   event_size = __calc_tpm2_event_size(header,
> +  (void *)(long)log_location,
> +  false);
> +   final_events_size += event_size;
> +   i--;
> +   }
> +   }
> +
> memset(log_tbl, 0, sizeof(*log_tbl) + log_size);
> log_tbl->size = log_size;
> +   log_tbl->final_events_preboot_size = final_events_size;
>