Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 19 Aug 2022 at 07:29, Heinrich Schuchardt
<heinrich.schuchardt@xxxxxxxxxxxxx> wrote:
>
>
>
> On 8/18/22 19:10, Ard Biesheuvel wrote:
> > On Thu, 18 Aug 2022 at 18:42, Heinrich Schuchardt
> > <heinrich.schuchardt@xxxxxxxxxxxxx> wrote:
> >>
> >> On 8/17/22 13:03, Ard Biesheuvel wrote:
> >>> Implement a minimal EFI app that decompresses the real kernel image and
> >>> launches it using the firmware's LoadImage and StartImage boot services.
> >>> This removes the need for any arch-specific hacks.
> >>>
> >>> Note that on systems that have UEFI secure boot policies enabled,
> >>> LoadImage/StartImage require images to be signed, or their hashes known
> >>> a priori, in order to be permitted to boot.
> >>>
> >>> There are various possible strategies to work around this requirement,
> >>> but they all rely either on overriding internal PI/DXE protocols (which
> >>> are not part of the EFI spec) or omitting the firmware provided
> >>> LoadImage() and StartImage() boot services, which is also undesirable,
> >>> given that they encapsulate platform specific policies related to secure
> >>> boot and measured boot, but also related to memory permissions (whether
> >>> or not and which types of heap allocations have both write and execute
> >>> permissions.)
> >>>
> >>> The only generic and truly portable way around this is to simply sign
> >>> both the inner and the outer image with the same key/cert pair, so this
> >>> is what is implemented here.
> >>>
> >>> BZIP2 has been omitted from the set of supported compression algorithms,
> >>> given that its performance is mediocre both in speed and size, and it
> >>> uses a disproportionate amount of memory. For optimal compression, use
> >>> LZMA. For the fastest boot speed, use LZO.
> >>>
> >>> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> >>> ---
> >>>    drivers/firmware/efi/Kconfig                |  31 ++++-
> >>>    drivers/firmware/efi/libstub/Makefile       |   8 +-
> >>>    drivers/firmware/efi/libstub/Makefile.zboot |  69 ++++++++++
> >>>    drivers/firmware/efi/libstub/zboot-header.S | 139 ++++++++++++++++++++
> >>>    drivers/firmware/efi/libstub/zboot.c        | 101 ++++++++++++++
> >>>    drivers/firmware/efi/libstub/zboot.lds      |  39 ++++++
> >>>    6 files changed, 382 insertions(+), 5 deletions(-)
> >>>
> > ...
> >>> diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
> >>> new file mode 100644
> >>> index 000000000000..9cf968e90775
> >>> --- /dev/null
> >>> +++ b/drivers/firmware/efi/libstub/zboot.c
> > ...
> >>> +efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
> >>> +                                   efi_system_table_t *systab)
> >>> +{
> >>> +     static efi_guid_t loaded_image = LOADED_IMAGE_PROTOCOL_GUID;
> >>> +     efi_loaded_image_t *parent, *child;
> >>> +     unsigned long image_buffer;
> >>> +     efi_handle_t child_handle;
> >>> +     efi_status_t status;
> >>> +     int ret;
> >>> +
> >>> +     WRITE_ONCE(efi_system_table, systab);
> >>> +
> >>> +     free_mem_ptr = (unsigned long)&zboot_heap;
> >>> +     free_mem_end_ptr = free_mem_ptr + sizeof(zboot_heap);
> >>> +
> >>> +     status = efi_bs_call(handle_protocol, handle, &loaded_image,
> >>> +                          (void **)&parent);
> >>> +     if (status != EFI_SUCCESS) {
> >>> +             log(L"Failed to locate parent's loaded image protocol\n");
> >>> +             return status;
> >>> +     }
> >>> +
> >>> +     status = efi_allocate_pages(uncompressed_size, &image_buffer, ULONG_MAX);
> >>> +     if (status != EFI_SUCCESS) {
> >>> +             log(L"Failed to allocate memory\n");
> >>> +             return status;
> >>> +     }
> >>> +
> >>> +     ret = __decompress(_gzdata_start, _gzdata_end - _gzdata_start, NULL,
> >>> +                        NULL, (unsigned char *)image_buffer, 0, NULL,
> >>> +                        error);
> >>> +     if (ret < 0) {
> >>> +             log(L"Decompression failed\n");
> >>> +             return EFI_LOAD_ERROR;
> >>> +     }
> >>> +
> >>> +     status = efi_bs_call(load_image, false, handle, NULL,
> >>
> >> I would prefer to pass the device path of the compressed image instead
> >> of NULL. This way information is not lost.
> >>
> >
> > That way, we will have two loaded images with different handles
> > claiming to be loaded from the same device path - I don't think that
> > is appropriate tbh.
>
> They both are the product of the same file on disk.
>

But they are not the same. When re-loading the device path (as you
suggest below) you will get a completely different file, and the only
way to get at the payload is to execute it.

So using the same device path is out of the question imo.

> >
> > What we could do is define a vendor GUID for the decompressed kernel,
> > and create a device path for it. That way, you can grab the
> > loaded_image of the parent to obtain this information.
> >
> > What did you have in mind as a use case?
>
> The device-path could be used in the kernel log.
>
> It can be used to find the device or folder with initrd where we use
> initrd= on the command line.
>
> You could use the device path to access the original file, e.g. to read
> additional information.
>
> For all use cases you would want to have the original device path.
>

What we could do is:

- define a device path in the decompressor, e.g.,

<original device path>/Offset(<start>, <end>)/VendorMedia(xxx-xxx-xxx,
<compression type>)

where start, end and compression type describe the compressed payload
inside the decompressor executable. (The compression type could be
omitted, or could be a separate node.)

- install the LoadFile2 protocol and the device path protocol onto a
handle, and move the decompression logic into the LoadFile2
implementation

- drop the SourceBuffer and SourceSize arguments to LoadImage(), and
pass the device path instead, so that LoadFile2 will be invoked by
LoadImage directly to perform the decompression.

That way, we retain the information about the outer file, and each
piece is described in detail in device path notation. As a bonus, we
could easily expose the compressed part separately, if there is a need
for that.

This doesn't cover the initrd= issue you raised, but that is something
we could address later in the stub if we wanted to (but I don't think
initrd= is something we should care too much about)



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux