On 02/17/20 10:23, Ard Biesheuvel wrote: > On Mon, 17 Feb 2020 at 10:15, Laszlo Ersek <lersek@xxxxxxxxxx> wrote: >> >> On 02/16/20 15:11, Ard Biesheuvel wrote: >>> There are currently two ways to specify the initrd to be passed to the >>> Linux kernel when booting via the EFI stub: >>> - it can be passed as a initrd= command line option when doing a pure PE >>> boot (as opposed to the EFI handover protocol that exists for x86) >>> - otherwise, the bootloader or firmware can load the initrd into memory, >>> and pass the address and size via the bootparams struct (x86) or >>> device tree (ARM) >>> >>> In the first case, we are limited to loading from the same file system >>> that the kernel was loaded from, and it is also problematic in a trusted >>> boot context, given that we cannot easily protect the command line from >>> tampering without either adding complicated white/blacklisting of boot >>> arguments or locking down the command line altogether. >>> >>> In the second case, we force the bootloader to duplicate knowledge about >>> the boot protocol which is already encoded in the stub, and which may be >>> subject to change over time, e.g., bootparams struct definitions, memory >>> allocation/alignment requirements for the placement of the initrd etc etc. >>> In the ARM case, it also requires the bootloader to modify the hardware >>> description provided by the firmware, as it is passed in the same file. >>> On systems where the initrd is measured after loading, it creates a time >>> window where the initrd contents might be manipulated in memory before >>> handing over to the kernel. >>> >>> Address these concerns by adding support for loading the initrd into >>> memory by invoking the EFI LoadFile2 protocol installed on a vendor >>> GUIDed device path that specifically designates a Linux initrd. >>> This addresses the above concerns, by putting the EFI stub in charge of >>> placement in memory and of passing the base and size to the kernel proper >>> (via whatever means it desires) while still leaving it up to the firmware >>> or bootloader to obtain the file contents, potentially from other file >>> systems than the one the kernel itself was loaded from. On platforms that >>> implement measured boot, it permits the firmware to take the measurement >>> right before the kernel actually consumes the contents. >>> >>> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx> >>> --- >>> drivers/firmware/efi/libstub/arm-stub.c | 15 +++- >>> drivers/firmware/efi/libstub/efi-stub-helper.c | 82 ++++++++++++++++++++ >>> drivers/firmware/efi/libstub/efistub.h | 4 + >>> drivers/firmware/efi/libstub/x86-stub.c | 23 ++++++ >>> include/linux/efi.h | 1 + >>> 5 files changed, 122 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c >>> index 2edc673ea06c..4bae620b95b9 100644 >>> --- a/drivers/firmware/efi/libstub/arm-stub.c >>> +++ b/drivers/firmware/efi/libstub/arm-stub.c >>> @@ -160,6 +160,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg, >>> enum efi_secureboot_mode secure_boot; >>> struct screen_info *si; >>> efi_properties_table_t *prop_tbl; >>> + unsigned long max_addr; >>> >>> sys_table = sys_table_arg; >>> >>> @@ -258,10 +259,18 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg, >>> if (!fdt_addr) >>> pr_efi("Generating empty DTB\n"); >>> >>> - status = efi_load_initrd(image, &initrd_addr, &initrd_size, ULONG_MAX, >>> - efi_get_max_initrd_addr(dram_base, *image_addr)); >>> + max_addr = efi_get_max_initrd_addr(dram_base, *image_addr); >>> + status = efi_load_initrd_dev_path(&initrd_addr, &initrd_size, max_addr); >>> + if (status == EFI_SUCCESS) { >>> + pr_efi("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n"); >>> + } else if (status == EFI_NOT_FOUND) { >>> + status = efi_load_initrd(image, &initrd_addr, &initrd_size, >>> + ULONG_MAX, max_addr); >>> + if (status == EFI_SUCCESS) >>> + pr_efi("Loaded initrd from command line option\n"); >>> + } >>> if (status != EFI_SUCCESS) >>> - pr_efi_err("Failed initrd from command line!\n"); >>> + pr_efi_err("Failed to load initrd!\n"); >>> >>> efi_random_get_seed(); >>> >>> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c >>> index 49008ac88b63..e37afe2c752e 100644 >>> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c >>> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c >>> @@ -299,3 +299,85 @@ void efi_char16_printk(efi_char16_t *str) >>> efi_call_proto(efi_table_attr(efi_system_table(), con_out), >>> output_string, str); >>> } >>> + >>> +/* >>> + * The LINUX_EFI_INITRD_MEDIA_GUID vendor media device path below provides a way >>> + * for the firmware or bootloader to expose the initrd data directly to the stub >>> + * via the trivial LoadFile2 protocol, which is defined in the UEFI spec, and is >>> + * very easy to implement. It is a simple Linux initrd specific conduit between >>> + * kernel and firmware, allowing us to put the EFI stub (being part of the >>> + * kernel) in charge of where and when to load the initrd, while leaving it up >>> + * to the firmware to decide whether it needs to expose its filesystem hierarchy >>> + * via EFI protocols. >>> + */ >>> +static const struct { >>> + struct efi_vendor_dev_path vendor; >>> + struct efi_generic_dev_path end; >>> +} __packed initrd_dev_path = { >>> + { >>> + EFI_DEV_MEDIA, >>> + EFI_DEV_MEDIA_VENDOR, >>> + sizeof(struct efi_vendor_dev_path), >>> + LINUX_EFI_INITRD_MEDIA_GUID >>> + }, { >>> + EFI_DEV_END_PATH, >>> + EFI_DEV_END_ENTIRE, >>> + sizeof(struct efi_generic_dev_path) >>> + } >>> +}; >>> + >>> +/** >>> + * efi_load_initrd_dev_path - load the initrd from the Linux initrd device path >>> + * @load_addr: pointer to store the address where the initrd was loaded >>> + * @load_size: pointer to store the size of the loaded initrd >>> + * @max: upper limit for the initrd memory allocation >>> + * @return: %EFI_SUCCESS if the initrd was loaded successfully, in which case >>> + * @load_addr and @load_size are assigned accordingly >>> + * %EFI_NOT_FOUND if no LoadFile2 protocol exists on the initrd >>> + * device path >>> + * %EFI_LOAD_ERROR in all other cases >> >> [*] >> >>> + */ >>> +efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr, >>> + unsigned long *load_size, >>> + unsigned long max) >>> +{ >>> + efi_guid_t lf2_proto_guid = EFI_LOAD_FILE2_PROTOCOL_GUID; >>> + efi_device_path_protocol_t *dp; >>> + efi_load_file2_protocol_t *lf2; >>> + unsigned long initrd_addr; >>> + unsigned long initrd_size; >>> + efi_handle_t handle; >>> + efi_status_t status; >>> + >>> + if (!load_addr || !load_size) >>> + return EFI_INVALID_PARAMETER; >> >> Doesn't return EFI_LOAD_ERROR. >> >>> + >>> + dp = (efi_device_path_protocol_t *)&initrd_dev_path; >>> + status = efi_bs_call(locate_device_path, &lf2_proto_guid, &dp, &handle); >>> + if (status != EFI_SUCCESS) >>> + return status; >> >> Seems safe (the only plausible error could be EFI_NOT_FOUND). >> >>> + >>> + status = efi_bs_call(handle_protocol, handle, &lf2_proto_guid, >>> + (void **)&lf2); >>> + if (status != EFI_SUCCESS) >>> + return status; >> >> Interesting case; this should never fail... but note, if it does, it >> returns EFI_UNSUPPORTED, not EFI_NOT_FOUND (if the protocol is missing >> from the handle). >> >>> + >>> + status = efi_call_proto(lf2, load_file, dp, false, &initrd_size, NULL); >>> + if (status != EFI_BUFFER_TOO_SMALL) >>> + return EFI_LOAD_ERROR; >>> + >>> + status = efi_allocate_pages(initrd_size, &initrd_addr, max); >>> + if (status != EFI_SUCCESS) >>> + return status; >> >> Not sure about the efi_allocate_pages() wrapper (?); the UEFI service >> could return EFI_OUT_OF_RESOURCES. >> > > Hmm, guess I was a bit sloppy with the return codes. The important > thing is that EFI_NOT_FOUND is only returned in the one specifically > defined case. > >> Looks OK to me otherwise. >> > > Thanks. > >> (... I'm a bit doubtful of passing and End node to LF2 rather than a >> filepath node with "" for pathname, but it's an LF2 on our own vendor >> path, so I guess we dictate what we accept.) >> > > It seems to me that the whole point of advancing the > pointer-to-pointer-to-device path protocol past the matched part of > the device path is to make it straight-forward for the caller to pass > the remainder into the protocol implementation. So if the matched part > fully defines the target, pointing to an end node is the only correct > thing to do imo, as the empty file did not appear in the device path > that was used to locate the protocol. Fair enough. Thanks Laszlo