[PATCH] efi_loader: fix FinalEvents table if an EFI uses GetEventLog

[Ilias Apalodimas]

Hi Heinrich,

On Wed, Nov 17, 2021 at 11:01:55AM +0100, Heinrich Schuchardt wrote:
> On 11/17/21 10:10, Ilias Apalodimas wrote:
> > As described in the TCG spec [1] in sections 7.1.1 and 7.1.2 the FinalEvent
> > table should include events after GetEventLog has been called.  This
> > currently works for us as long as the kernel is the only EFI application
> > calling that.  Specifically we only implement what's described in 7.1.1.
> > 
> > So refactor the code a bit and support EFI application calling GetEventLog.
> > Events will now be logged in both the EventLog and FinalEvent table as long
> > as ExitBootServices haven't been invoked.
> > 
> > [1] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf
> > 
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > ---
> >   lib/efi_loader/efi_tcg2.c | 90 ++++++++++++++++++++++++++-------------
> >   1 file changed, 61 insertions(+), 29 deletions(-)
> > 
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 189e4a5ba59c..215f4b2b04b8 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -34,6 +34,7 @@ struct event_log_buffer {
> >   	size_t final_pos; /* final events config table position */
> >   	size_t last_event_size;
> >   	bool get_event_called;
> > +	bool ebs_called;
> 
> Please, add documentation for the elements of the structure. Not every
> reader will be aware of ebs_called referring to ExitBootServices().

Sure

> 
> >   	bool truncated;
> >   };
> > 
> > @@ -186,39 +187,29 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
> >   	return EFI_SUCCESS;
> > 

[...]

> >   /**
> > @@ -1303,6 +1333,7 @@ static efi_status_t efi_init_event_log(void)
> >   	event_log.pos = 0;
> >   	event_log.last_event_size = 0;
> >   	event_log.get_event_called = false;
> > +	event_log.ebs_called = false;
> >   	event_log.truncated = false;
> > 
> >   	/*
> > @@ -1792,6 +1823,7 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
> > 
> >   	EFI_ENTRY("%p, %p", event, context);
> 
> This is called in EFI_EVENT_GROUP_EXIT_BOOT_SERVICES.
> 
> This implies that whatever happens in
> EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES is measured normally. Does
> this conform to the TCG2 standard?

Yes I think so.  My understanding of 7.1.2 diagram in the spec is:
- Log all events to the EventLog buffer if GetEventLog() hasn't been called
- Log all events to the EventLog buffer *and* the FinalEvent config table if 
  GetEventLog() has been called
- If you are in EBS(), you don't know if the firmware has cleaned up the
  EventLog buffer, so log these events in the FinalEvent config table only.
> 
> > 
> > +	event_log.ebs_called = true;
> 
> How should a failed call to ExitBootServices() be handled?
> E.g. invalid memory map?

Good question.  We also have efi_tcg2_notify_exit_boot_services_failed().
If the EventLog buffer hasn't been destroyed from memory we can switch the 
ebs_called = false?

Cheers
/Ilias
> 
> Best regards
> 
> Heinrich
> 
> >   	ret = platform_get_tpm2_device(&dev);
> >   	if (ret != EFI_SUCCESS)
> >   		goto out;
> > 
>