[PATCH 1/2] efi_loader: bootbin: split initrd registration and installation
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu May 1 09:15:49 CEST 2025
On 5/1/25 03:15, Adriano Cordova wrote:
> Current support for initrd in EFI booting has two flaws:
>
> 1. Installs a NULL initrd via LoadFile2 protocol if `efi_binary_run_dp` does
> not provide an initrd. In this case, a LoadFile2 should not be installed
> at all.
> 2. Initrd is not properly uninstalled if booting fails at some point between
> initrd installation and the call to `do_bootefi_exec`. This affects both
> booting via `efi_bootbin` and via `efi_bootmgr`.
>
> This commit splits the process of setting an initrd into two steps:
>
> First, `efi_initrd_set_from_mem` creates a memory-backed device path for the
> initrd. (This step is not necessary when using `efi_bootmgr`, and for now is
> only used when booting FIT images, but the `bootefi` command could now be
> expanded to receive an initrd argument.)
>
> Second, `efi_initrd_install` creates an initrd handle and installs the
> LoadFile2 protocol exposing the initrd. This is needed both when booting with
> `efi_bootbin` and with `efi_bootmgr`, so it is called in `do_bootefi_exec`,
> which is the first common point between the two bootflows.
>
> Also, as `do_bootefi_exec` is the only place where `efi_initrd_install` is
> called, the cleanup function `efi_initrd_uninstall` can be called before
> exiting `do_bootefi_exec` instead of using the event
> `efi_guid_event_group_return_to_efibootmgr`. (This also makes more sense as
> now initrd is also used by `efi_bootbin`.)
>
> Fixes: 36835a9105c ("efi_loader: binary_run: register an initrd")
> Reported-by: Weizhao Ouyang <o451686892 at gmail.com>
> Reported-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Signed-off-by: Adriano Cordova <adriano.cordova at canonical.com>
> ---
> include/efi_loader.h | 8 ++-
> lib/efi_loader/efi_bootbin.c | 14 +++---
> lib/efi_loader/efi_bootmgr.c | 7 ---
> lib/efi_loader/efi_helper.c | 11 +++++
> lib/efi_loader/efi_load_initrd.c | 84 +++++++++++++++++---------------
> 5 files changed, 68 insertions(+), 56 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 144b749278a..0a3ca799287 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -597,6 +597,12 @@ efi_status_t efi_env_set_load_options(efi_handle_t handle, const char *env_var,
> void *efi_get_configuration_table(const efi_guid_t *guid);
> /* Install device tree */
> efi_status_t efi_install_fdt(void *fdt);
> +/* Install initrd LoadFile2 protocol */
> +efi_status_t efi_initrd_install(void);
> +/* Uninstall initrd LoadFile2 protocol */
> +efi_status_t efi_initrd_uninstall(void);
> +/* Set an initrd */
> +efi_status_t efi_initrd_set_from_mem(void *initrd, size_t initd_sz);
> /* Execute loaded UEFI image */
> efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options);
> /* Run loaded UEFI image with given fdt */
> @@ -667,8 +673,6 @@ efi_status_t efi_http_register(const efi_handle_t handle,
> struct efi_service_binding_protocol *http_service_binding);
> /* Called by bootefi to make the watchdog available */
> efi_status_t efi_watchdog_register(void);
> -efi_status_t efi_initrd_register(struct efi_device_path *dp_initrd);
> -efi_status_t efi_initrd_deregister(void);
> /* Called by bootefi to make SMBIOS tables available */
> /**
> * efi_acpi_register() - write out ACPI tables
> diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c
> index d0f7da309ce..637f2c0d195 100644
> --- a/lib/efi_loader/efi_bootbin.c
> +++ b/lib/efi_loader/efi_bootbin.c
> @@ -220,7 +220,6 @@ static efi_status_t efi_binary_run_dp(void *image, size_t size, void *fdt,
> struct efi_device_path *dp_img)
> {
> efi_status_t ret;
> - struct efi_device_path *dp_initrd;
>
> /* Initialize EFI drivers */
> ret = efi_init_obj_list();
> @@ -234,13 +233,12 @@ static efi_status_t efi_binary_run_dp(void *image, size_t size, void *fdt,
> if (ret != EFI_SUCCESS)
> return ret;
>
> - dp_initrd = efi_dp_from_mem(EFI_LOADER_DATA, (uintptr_t)initrd, initd_sz);
> - if (!dp_initrd)
> - return EFI_OUT_OF_RESOURCES;
> -
> - ret = efi_initrd_register(dp_initrd);
> - if (ret != EFI_SUCCESS)
> - return ret;
> + /* Set initrd if provided */
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> + ret = efi_initrd_set_from_mem(initrd, initd_sz);
> + if (ret != EFI_SUCCESS)
> + return ret;
> + }
>
> return efi_run_image(image, size, dp_dev, dp_img);
> }
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index c0df5cb9acd..19c7e16a25b 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -683,13 +683,6 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
> if (ret != EFI_SUCCESS)
> goto error;
>
> - /* try to register load file2 for initrd's */
> - if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> - ret = efi_initrd_register(NULL);
> - if (ret != EFI_SUCCESS)
> - goto error;
> - }
> -
> log_info("Booting: Label: %ls Device path: %pD\n", lo.label, lo.file_path);
>
> /* Ignore the optional data in auto-generated boot options */
> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
> index 3936139ca41..b4c207ea33f 100644
> --- a/lib/efi_loader/efi_helper.c
> +++ b/lib/efi_loader/efi_helper.c
> @@ -658,6 +658,13 @@ efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
> goto out;
> }
>
> + /* Create initrd handle and install LoadFile2 protocol */
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> + ret = efi_initrd_install();
> + if (ret != EFI_SUCCESS)
> + goto out;
> + }
> +
> /* Call our payload! */
> ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
> if (ret != EFI_SUCCESS) {
> @@ -683,6 +690,10 @@ out:
> }
> }
>
> + /* Destroy initrd handle with LoadFile2 protocol */
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD))
> + ret = efi_initrd_uninstall();
> +
> /* Control is returned to U-Boot, disable EFI watchdog */
> efi_set_watchdog(0);
>
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index b5d58943a80..7210bf16d09 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -74,7 +74,40 @@ static efi_status_t get_initrd_fp(struct efi_device_path **initrd_fp)
> }
>
> /**
> - * efi_initrd_from_mem() - load initial RAM disk from memory
> + * efi_initrd_set_from_mem() - set initial RAM disk from memory
> + *
> + * Set efi_initrd_dp to a memory mapped device path pointing
> + * to @initrd. If @initrd is NULL then efi_initrd_dp gets cleared.
> + *
> + *
> + * @initrd: address of initrd or NULL if none is provided
> + * @initrd_sz: size of initrd
> + * Return: status code
> + */
> +efi_status_t efi_initrd_set_from_mem(void *initrd, size_t initd_sz)
> +{
> + struct efi_device_path *old_initrd_dp;
> +
> + old_initrd_dp = efi_initrd_dp;
In efi_initrd_uninstall() we free the device-path and set efi_initrd_dp
= NULL.
It seems you are handling the case of efi_initrd_install() failing here.
To me this looks a bit convoluted. Calling efi_initrd_register() as in
your v3 series would allow handling errors when they occur.
The static variable efi_initrd_dp should receive some documentation in
the code explaining how it is used. This could be in a separate patch.
I will merge your v3 series as it fixes the case of no initrd.
We can handle the uninstallation of the LoadFile2 protocol in a patch on
top of that.
Best regards
Heinrich
> +
> + if (!initrd) {
> + efi_initrd_dp = NULL;
> + goto out;
> + }
> +
> + efi_initrd_dp = efi_dp_from_mem(EFI_LOADER_DATA, (uintptr_t)initrd, initd_sz);
> + if (!efi_initrd_dp) {
> + efi_initrd_dp = old_initrd_dp;
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> +out:
> + efi_free_pool(old_initrd_dp);
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + * efi_initrd_get_from_mem() - get initial RAM disk from memory
> *
> * This function copies the initrd from the memory mapped device
> * path pointed to by efi_initrd_dp
> @@ -84,7 +117,7 @@ static efi_status_t get_initrd_fp(struct efi_device_path **initrd_fp)
> *
> * Return: status code
> */
> -static efi_status_t efi_initrd_from_mem(efi_uintn_t *buffer_size, void *buffer)
> +static efi_status_t efi_initrd_get_from_mem(efi_uintn_t *buffer_size, void *buffer)
> {
> efi_status_t ret = EFI_NOT_FOUND;
> efi_uintn_t bs;
> @@ -155,7 +188,7 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
> }
>
> if (efi_initrd_dp)
> - return EFI_EXIT(efi_initrd_from_mem(buffer_size, buffer));
> + return EFI_EXIT(efi_initrd_get_from_mem(buffer_size, buffer));
>
> ret = get_initrd_fp(&initrd_fp);
> if (ret != EFI_SUCCESS)
> @@ -224,14 +257,14 @@ out:
> }
>
> /**
> - * efi_initrd_deregister() - delete the handle for loading initial RAM disk
> + * efi_initrd_uninstall() - delete the handle for loading initial RAM disk
> *
> * This will delete the handle containing the Linux specific vendor device
> - * path and EFI_LOAD_FILE2_PROTOCOL for loading an initrd
> + * path and the installed EFI_LOAD_FILE2_PROTOCOL for loading an initrd
> *
> * Return: status code
> */
> -efi_status_t efi_initrd_deregister(void)
> +efi_status_t efi_initrd_uninstall(void)
> {
> efi_status_t ret;
>
> @@ -255,42 +288,22 @@ efi_status_t efi_initrd_deregister(void)
> }
>
> /**
> - * efi_initrd_return_notify() - return to efibootmgr callback
> - *
> - * @event: the event for which this notification function is registered
> - * @context: event context
> - */
> -static void EFIAPI efi_initrd_return_notify(struct efi_event *event,
> - void *context)
> -{
> - efi_status_t ret;
> -
> - EFI_ENTRY("%p, %p", event, context);
> - ret = efi_initrd_deregister();
> - EFI_EXIT(ret);
> -}
> -
> -/**
> - * efi_initrd_register() - create handle for loading initial RAM disk
> + * efi_initrd_install() - create handle for loading initial RAM disk
> *
> * This function creates a new handle and installs a Linux specific vendor
> * device path and an EFI_LOAD_FILE2_PROTOCOL. Linux uses the device path
> * to identify the handle and then calls the LoadFile service of the
> - * EFI_LOAD_FILE2_PROTOCOL to read the initial RAM disk. If dp_initrd is
> - * not provided, the initrd will be taken from the BootCurrent variable
> - *
> - * @dp_initrd: optional device path containing an initrd
> + * EFI_LOAD_FILE2_PROTOCOL to read the initial RAM disk. If efi_initrd_dp
> + * is not set the initrd will be taken from the BootCurrent variable. If
> + * an initrd is not found the handle is not created.
> *
> * Return: status code
> */
> -efi_status_t efi_initrd_register(struct efi_device_path *dp_initrd)
> +efi_status_t efi_initrd_install(void)
> {
> efi_status_t ret;
> - struct efi_event *event;
>
> - if (dp_initrd) {
> - efi_initrd_dp = dp_initrd;
> - } else {
> + if (!efi_initrd_dp) {
> /*
> * Allow the user to continue if Boot#### file path is not set for
> * an initrd
> @@ -314,12 +327,5 @@ efi_status_t efi_initrd_register(struct efi_device_path *dp_initrd)
> return ret;
> }
>
> - ret = efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> - efi_initrd_return_notify, NULL,
> - &efi_guid_event_group_return_to_efibootmgr,
> - &event);
> - if (ret != EFI_SUCCESS)
> - log_err("Creating event failed\n");
> -
> return ret;
> }
More information about the U-Boot
mailing list