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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAC_iWjJH8JwdPbL9Et6xNLf4vV1AQDm8ZZh8zYVkb+VFLXedTg@mail.gmail.com>
Date: Tue, 1 Oct 2024 10:19:59 +0300
From: Ilias Apalodimas <ilias.apalodimas@...aro.org>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: linux-efi@...r.kernel.org, bp@...en8.de, 
	sathyanarayanan.kuppuswamy@...ux.intel.com, linux-kernel@...r.kernel.org, 
	Jeremy Linton <jeremy.linton@....com>
Subject: Re: [PATCH] efi/libstub: measure initrd to PCR9 independent of source

Thanks, Ard

On Tue, 1 Oct 2024 at 08:59, Ard Biesheuvel <ardb@...nel.org> wrote:
>
> (cc Ilias)
>
> On Tue, 1 Oct 2024 at 05:20, Jeremy Linton <jeremy.linton@....com> 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@....com>
> > ---
> >  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
> >