[PATCH] efi_loader: fix FinalEvent table initial position

[Ilias Apalodimas]

On Wed, 17 Nov 2021 at 11:10, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 11/17/21 00:24, Ilias Apalodimas wrote:
> > When initializing the final event table in create_final_event() we are
> > setting the initial buffer position to sizeof(*final_event).  Although
> > we clear the buffer correctly and won't cause any problems,  we should
> > start logging events starting from zero.
> > While at it add a cast when defining the final_event.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > ---
> >   lib/efi_loader/efi_tcg2.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 189e4a5ba59c..634556342a7c 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -1257,10 +1257,11 @@ static efi_status_t create_final_event(void)
> >               goto out;
> >
> >       memset(event_log.final_buffer, 0xff, TPM2_EVENT_LOG_SIZE);
> > -     final_event = event_log.final_buffer;
> > +     final_event =
> > +             (struct efi_tcg2_final_events_table *)event_log.final_buffer;
>
> final_buffer is defined as void *. So this cast is superfluous and
> should be avoided. Cf.
> https://www.kernel.org/doc/html/latest/process/coding-style.html#allocating-memory
>
> There is a further unneeded conversion of the same type in the file.
>
> Why didn't you define component final_buffer as struct
> efi_tcg2_final_events_table * and just added a void ** conversion to the
> efi_allocate_pool() call?
>
> This would allow to eliminate the variable final_event here and in
> tcg2_agile_log_append().

Because both the buffer and the final_buffer describe log events of
different type that need to be appended on the eventlog using the same
functions.  Due to all the ptr arithmetic that takes place,  I though
having them as void would be easier to read.  In any case this patch
shouldn't be applied.

Cheers
/Ilias

> Best regards
>
> Heinrich
>
> >       final_event->number_of_events = 0;
> >       final_event->version = EFI_TCG2_FINAL_EVENTS_TABLE_VERSION;
> > -     event_log.final_pos = sizeof(*final_event);
> > +     event_log.final_pos = 0;
> >       ret = efi_install_configuration_table(&efi_guid_final_events,
> >                                             final_event);
> >       if (ret != EFI_SUCCESS) {
> >
>
>