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 <o451686...@gmail.com>
Reported-by: Heinrich Schuchardt <xypron.g...@gmx.de>
Signed-off-by: Adriano Cordova <adriano.cord...@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;
}
