Re: [PATCH] efi/libstub: measure initrd to PCR9 independent of source

[Ilias Apalodimas]

Thanks, Ard

On Tue, 1 Oct 2024 at 08:59, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> (cc Ilias)
>
> On Tue, 1 Oct 2024 at 05:20, Jeremy Linton <jeremy.linton@xxxxxxx> wrote:
> >
> > Currently the initrd is only measured if it can be loaded using the
> > INITRD_MEDIA_GUID, if we are loading it from a path provided via the
> > command line it is never measured. Lets move the check down a couple
> > lines so the measurement happens independent of the source.
> >
> > Signed-off-by: Jeremy Linton <jeremy.linton@xxxxxxx>
> > ---
> >  drivers/firmware/efi/libstub/efi-stub-helper.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > index de659f6a815f..555f84287f0b 100644
> > --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > @@ -621,10 +621,6 @@ efi_status_t efi_load_initrd(efi_loaded_image_t *image,
> >         status = efi_load_initrd_dev_path(&initrd, hard_limit);
> >         if (status == EFI_SUCCESS) {
> >                 efi_info("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
> > -               if (initrd.size > 0 &&
> > -                   efi_measure_tagged_event(initrd.base, initrd.size,
> > -                                            EFISTUB_EVT_INITRD) == EFI_SUCCESS)
> > -                       efi_info("Measured initrd data into PCR 9\n");
> >         } else if (status == EFI_NOT_FOUND) {
> >                 status = efi_load_initrd_cmdline(image, &initrd, soft_limit,
> >                                                  hard_limit);
> > @@ -637,6 +633,11 @@ efi_status_t efi_load_initrd(efi_loaded_image_t *image,
> >         if (status != EFI_SUCCESS)
> >                 goto failed;
> >
> > +       if (initrd.size > 0 &&
> > +           efi_measure_tagged_event(initrd.base, initrd.size,
> > +                                    EFISTUB_EVT_INITRD) == EFI_SUCCESS)
> > +               efi_info("Measured initrd data into PCR 9\n");

Back when we added this we intentionally left loading an initramfs
loaded via the command line out.
We wanted people to start using the LoadFile2 protocol instead of the
command line option, which suffered from various issues  -- e.g could
only be loaded if it resided in the same filesystem as the kernel and
the bootloader had to reason about the kernel memory layout.
I don't think measuring the command line option as well is going to
cause any problems, but isn't it a step backward?

Thanks
/Ilias
> > +
> >         status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, sizeof(initrd),
> >                              (void **)&tbl);
> >         if (status != EFI_SUCCESS)
> > --
> > 2.46.1
> >