EFI applications executed directly from the shell via the binfmt hook, e.g. "/boot/shell.efi -nostartup", currently have their arguments silently dropped: the only load options ever set are the Linux bootargs and only if the image was detected as an EFI-stubbed kernel.
Serialize the arguments following the image path into the load options instead and leave it to the caller of efi_execute_image() to decide what the load options should be. bootm keeps passing the Linux bootargs for kernel images, but executing a kernel image directly from the shell now passes exactly what was typed on the command line, which makes the documentation's claim that only bootm passes the kernel command line true again. The firmware unloads an application as soon as it returns, as does barebox' own loader in efi_exit() and EDK2 in CoreStartImage(), so the load options may only be cleared for a driver that started successfully and is thus still around. The buffer itself belongs to barebox and is freed either way. While at it, print the "Booting ... with options" message for all images, not only for EFI-stubbed kernels, and add the missing space in it. Assisted-by: Claude:fable-5 Signed-off-by: Ahmad Fatoum <[email protected]> --- efi/payload/bootm.c | 9 ++++++-- efi/payload/image.c | 54 ++++++++++++++++++++++++++++++++------------- efi/payload/image.h | 2 +- 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/efi/payload/bootm.c b/efi/payload/bootm.c index 58b7dd5ea779..1eec73a13dd0 100644 --- a/efi/payload/bootm.c +++ b/efi/payload/bootm.c @@ -22,6 +22,7 @@ #include <string.h> #include <linux/err.h> #include <boot.h> +#include <bootargs.h> #include <bootm.h> #include <fs.h> #include <libfile.h> @@ -185,7 +186,9 @@ static int do_bootm_efi_stub(struct image_data *data) if (data->dryrun) goto unload_ramdisk; - ret = efi_execute_image(handle, loaded_image, true, type); + ret = efi_execute_image(handle, loaded_image, true, type, + filetype_is_linux_efi_image(type) ? + linux_bootargs_get() : NULL); /* efi_execute_image takes care to unload the image on error, * so we set image_freed and fall through to freeing ramdisk @@ -225,7 +228,9 @@ static int efi_app_execute(struct image_data *data) return 0; } - return efi_execute_image(handle, loaded_image, true, type); + return efi_execute_image(handle, loaded_image, true, type, + filetype_is_linux_efi_image(type) ? + linux_bootargs_get() : NULL); } static int linux_efi_handover = true; diff --git a/efi/payload/image.c b/efi/payload/image.c index 4e59c7b4ea10..003433ebb4ad 100644 --- a/efi/payload/image.c +++ b/efi/payload/image.c @@ -18,7 +18,6 @@ #include <malloc.h> #include <string.h> #include <linux/err.h> -#include <bootargs.h> #include <bootm.h> #include <fs.h> #include <libfile.h> @@ -101,10 +100,10 @@ int efi_load_image(const char *file, struct efi_loaded_image **loaded_image, int efi_execute_image(efi_handle_t handle, struct efi_loaded_image *loaded_image, bool is_bootm, - enum filetype filetype) + enum filetype filetype, const char *options) { efi_status_t efiret; - const char *options; + wchar_t *load_options = NULL; bool is_driver; bool no_return; @@ -120,18 +119,19 @@ int efi_execute_image(efi_handle_t handle, */ no_return = is_bootm || filetype_is_linux_efi_image(filetype); - if (filetype_is_linux_efi_image(filetype)) { - options = linux_bootargs_get(); - printf("Booting kernel via StartImage"); - if (options) { - printf("with options '%s'", options); - loaded_image->load_options = xstrdup_char_to_wchar(options); - loaded_image->load_options_size = - (strlen(options) + 1) * sizeof(wchar_t); - } - printf("...\n"); + if (options && *options) { + load_options = xstrdup_char_to_wchar(options); + loaded_image->load_options = load_options; + loaded_image->load_options_size = + (strlen(options) + 1) * sizeof(wchar_t); } + printf("Booting %s", filetype_is_linux_efi_image(filetype) ? + "kernel via StartImage" : "EFI image"); + if (load_options) + printf(" with options '%s'", options); + printf("...\n"); + if (no_return) { efi_set_variable_usec("LoaderTimeExecUSec", &efi_systemd_vendor_guid, ktime_to_us(ktime_get())); @@ -152,8 +152,24 @@ int efi_execute_image(efi_handle_t handle, if (EFI_ERROR(efiret)) pr_err("failed to StartImage: %s\n", efi_strerror(efiret)); - if (!is_driver) + /* + * load_options is freed below, so a driver that started successfully + * and thus stays loaded must drop its reference to it. The firmware + * already unloaded an application that returned and a driver that + * failed to start, so their loaded image protocol must not be touched. + * Unloading an application that already returned just fails, but is + * still needed when StartImage failed before running it. + */ + if (is_driver) { + if (!EFI_ERROR(efiret)) { + loaded_image->load_options = NULL; + loaded_image->load_options_size = 0; + } + } else { BS->unload_image(handle); + } + + free(load_options); efi_connect_all(); efi_register_devices(); @@ -165,13 +181,21 @@ static int efi_execute(struct binfmt_hook *b, char *file, int argc, char **argv) { struct efi_loaded_image *loaded_image; efi_handle_t handle; + char *options; int ret; ret = efi_load_image(file, &loaded_image, &handle); if (ret) return ret; - return efi_execute_image(handle, loaded_image, false, b->type); + /* argv[0] is the image itself, the rest become the load options */ + options = strjoin(" ", &argv[1], argc - 1); + + ret = efi_execute_image(handle, loaded_image, false, b->type, options); + + free(options); + + return ret; } static struct binfmt_hook binfmt_efi_hook = { diff --git a/efi/payload/image.h b/efi/payload/image.h index 33f7e1a21b30..54494c71626a 100644 --- a/efi/payload/image.h +++ b/efi/payload/image.h @@ -14,7 +14,7 @@ int efi_load_image(const char *file, struct efi_loaded_image **loaded_image, int efi_execute_image(efi_handle_t handle, struct efi_loaded_image *loaded_image, bool is_bootm, - enum filetype filetype); + enum filetype filetype, const char *options); extern struct image_handler efi_x86_linux_handle_tr; extern struct image_handler efi_x86_linux_handle_handover; -- 2.47.3
