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

[Jeremy Linton]

Hi,

On 10/1/24 2:19 AM, Ilias Apalodimas wrote:
> 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 for looking at this. Since no one else seems to have commented, I 
will just express IMHO, that both methods are useful in differing 
circumstances.

For a heavyweight Linux aware bootloader like grub/sd-boot the 
INITRD_MEDIA_GUID is obviously preferred. But, for booting strictly out 
out of a pure UEFI environment or Linux unaware bootloader (ex: UEFI 
shell), the commandline based initrd loader is a useful function. 
Because, the kernel stub should continue to serve as a complete, if 
minimal implementation for booting Linux out of a pure UEFI environment 
without additional support infrastructure (shim/grub/etc). So, it seems 
that unless there is a reason for divergent behavior it shouldn't exist. 
And at the moment, the two primary linux bootloaders grub2 and sdboot 
are both using the INITRD_MEDIA_GUID. Given the battering ram has been 
successful, it isn't 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