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

[Jeremy Linton <jeremy.linton@arm.com>]

Hi,

On 10/1/24 2:19 AM, Ilias Apalodimas wrote:
> Thanks, Ard
> 
> On Tue, 1 Oct 2024 at 08:59, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> (cc Ilias)
>>
>> On Tue, 1 Oct 2024 at 05:20, Jeremy Linton <jeremy.linton@arm.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@arm.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 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
>>>