[v2][PATCH 1/3] efi_loader: Add check for event log passed from firmware

[Ruchika Gupta]

Hi Ilias,

On Wed, 24 Nov 2021 at 12:34, Ilias Apalodimas <ilias.apalodimas at linaro.org>
wrote:

> Hi Ruchika,
> > +
>
> [...]
>
> > +     ret = platform_get_eventlog(dev, &base, &sz);
> > +     if (ret == EFI_SUCCESS) {
>
> Can we invert the logic here?
> if (ret != EFI_SUCCESS)
>         return ret;
>
> etc...
>
Change posted in v3.


> > +             void *buffer = (void *)base;
> > +
> > +             if (sz > TPM2_EVENT_LOG_SIZE)
> > +                     return EFI_VOLUME_FULL;
> > +
> > +             pos = 0;
> > +             /* Parse the eventlog to check for its validity */
> > +             ret = parse_event_log_header(buffer, sz, &pos);
> > +             if (ret || pos > sz)
> > +                     return EFI_COMPROMISED_DATA;
> > +
> > +             ret = parse_specid_event(dev, buffer, sz, &pos,
> &digest_list);
> > +             if (ret || pos > sz) {
> > +                     log_err("Error parsing SPEC ID Event\n");
> > +                     return EFI_COMPROMISED_DATA;
> > +             }
> > +
> > +             while (pos < sz) {
> > +                     ret = tcg2_parse_event(dev, buffer, sz, &pos,
> > +                                            &digest_list, &pcr);
> > +                     if (ret) {
> > +                             log_err("Error parsing event\n");
> > +                             return ret;
> > +                     }
> > +             }
> > +
> > +             memcpy(log_buffer, buffer, sz);
> > +             *log_sz = sz;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> >  /**
> >   * create_specid_event() - Create the first event in the eventlog
> >   *
> > @@ -1340,6 +1622,12 @@ static efi_status_t efi_init_event_log(void)
> >        * last log entry
> >        */
> >       memset(event_log.buffer, 0xff, TPM2_EVENT_LOG_SIZE);
> > +
> > +     /*
> > +      * The log header is defined to be in SHA1 event log entry format.
> > +      * Setup event header
> > +      */
> > +     event_header =  (struct tcg_pcr_event *)event_log.buffer;
> >       event_log.pos = 0;
> >       event_log.last_event_size = 0;
> >       event_log.get_event_called = false;
> > @@ -1347,22 +1635,28 @@ static efi_status_t efi_init_event_log(void)
> >       event_log.truncated = false;
> >
> >       /*
> > -      * The log header is defined to be in SHA1 event log entry format.
> > -      * Setup event header
> > +      * Check if earlier firmware have passed any eventlog. Different
> > +      * platforms can use different ways to do so
> >        */
> > -     event_header =  (struct tcg_pcr_event *)event_log.buffer;
> > -     put_unaligned_le32(0, &event_header->pcr_index);
> > -     put_unaligned_le32(EV_NO_ACTION, &event_header->event_type);
> > -     memset(&event_header->digest, 0, sizeof(event_header->digest));
> > -     ret = create_specid_event(dev, (void
> *)((uintptr_t)event_log.buffer + sizeof(*event_header)),
> > -                               &spec_event_size);
> > -     if (ret != EFI_SUCCESS)
> > -             goto free_pool;
> > -     put_unaligned_le32(spec_event_size, &event_header->event_size);
> > -     event_log.pos = spec_event_size + sizeof(*event_header);
> > -     event_log.last_event_size = event_log.pos;
> > +     ret = tcg2_get_fw_eventlog(dev, event_log.buffer, &event_log.pos);
> > +     if (ret == EFI_NOT_FOUND) {
> > +             put_unaligned_le32(0, &event_header->pcr_index);
> > +             put_unaligned_le32(EV_NO_ACTION,
> &event_header->event_type);
> > +             memset(&event_header->digest, 0,
> sizeof(event_header->digest));
> > +             ret = create_specid_event(dev,
> > +                                       (void
> *)((uintptr_t)event_log.buffer +
> > +                                                sizeof(*event_header)),
> > +                                       &spec_event_size);
> > +             if (ret != EFI_SUCCESS)
> > +                     goto free_pool;
> > +             put_unaligned_le32(spec_event_size,
> &event_header->event_size);
> > +             event_log.pos = spec_event_size + sizeof(*event_header);
> > +             event_log.last_event_size = event_log.pos;
> > +     }
> > +
> > +     if (ret == EFI_SUCCESS)
> > +             ret = create_final_event();
>
> Same here please.  Check for != EFI_SUCCESS and exit before creating the
> final eventlog config table.
>

Changes done in v3

Regards,
Ruchika


> >
> > -     ret = create_final_event();
> >       if (ret != EFI_SUCCESS)
> >               goto free_pool;
> >
> > --
> > 2.25.1
> >
>
> Thanks
> /lias
>