Hi Heinrich

On Sat, 16 Mar 2024 at 11:37, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:

Running an EFI binary loaded via tftp using the bootefi command results in showing the usage help.

The commit message sounds a bit off. The real problem is removing protocols from a non existent handle right? The help message was fixed in a subsequent patch

With that fixed Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

We should not try to remove protocol interfaces from a NULL handle.

Fixes: 6422820ac3e5 ("efi_loader: split unrelated code from efi_bootmgr.c") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

---

lib/efi_loader/efi_bootbin.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index 733cc1a61b5..b7910f78fb6 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -125,7 +125,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) efi_handle_t mem_handle = NULL, handle; struct efi_device_path *file_path = NULL; struct efi_device_path *msg_path;

•   efi_status_t ret, ret2;
  

•   efi_status_t ret;
    u16 *load_options;
  
    if (!bootefi_device_path || !bootefi_image_path) {
  

@@ -172,11 +172,17 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) ret = do_bootefi_exec(handle, load_options);

out:

•   ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
  

•                                                     &efi_guid_device_path,
  

•                                                     file_path, NULL);
  

•   if (mem_handle) {
  

•           efi_status_t r;
  

•           r = efi_uninstall_multiple_protocol_interfaces(
  

•                   mem_handle, &efi_guid_device_path, file_path, NULL);
  

•           if (r != EFI_SUCCESS)
  

•                   log_err("Uninstalling protocol interfaces failed\n");
  

•   }
    efi_free_pool(file_path);
  

•   return (ret != EFI_SUCCESS) ? ret : ret2;
  

•   return ret;
  

}

/**

2.43.0

On Sat, 16 Mar 2024 at 11:37, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:

If an EFI binary returns an error code EFI_INVALID_PARAMETER, we show the usage help for the bootefi command:

Shell> exit 0x8000000000000002
## Application failed, r = 2
bootefi - Boots an EFI payload from memory

Usage:
bootefi <image address>[:<image size>] [<fdt address>]
  - boot EFI payload
bootefi bootmgr [fdt address]
  - load and boot EFI payload based on BootOrder/BootXXXX variables.

    If specified, the device tree located at <fdt address> gets
    exposed as EFI configuration table.

This makes no sense.

Fixes: 296faf4f7ef1 ("cmd: bootefi: re-organize do_bootefi()") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

---

cmd/bootefi.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 9cf9027bf40..b509440cde0 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -154,9 +154,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc, !strcmp(argv[1], "bootmgr")) { ret = efi_bootmgr_run(fdt);

•           if (ret == EFI_INVALID_PARAMETER)
  

•                   return CMD_RET_USAGE;
  

•           else if (ret)
  

•           if (ret != EFI_SUCCESS)
                    return CMD_RET_FAILURE;
  
            return CMD_RET_SUCCESS;
  

@@ -218,9 +216,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,

    ret = efi_binary_run(image_buf, size, fdt);

•   if (ret == EFI_INVALID_PARAMETER)
  

•           return CMD_RET_USAGE;
  

•   else if (ret)
  

•   if (ret != EFI_SUCCESS)
            return CMD_RET_FAILURE;
  
    return CMD_RET_SUCCESS;
  

-- 2.43.0

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

On Sat, 16 Mar 2024 at 11:37, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:

If bootefi selftest is executed and a problem with the device-tree installation occurs, efi_install_fdt() writes sensible error messages. It never returns EFI_INVALID_PARAMETER. It neither makes sense to check for EFI_INVALID_PARAMETER nor to show the usage help for the bootefi command in this case.

Fixes: 296faf4f7ef1 ("cmd: bootefi: re-organize do_bootefi()") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

---

cmd/bootefi.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c index b509440cde0..578dbb19a7e 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -171,9 +171,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc, }

            ret = efi_install_fdt(fdt);

•           if (ret == EFI_INVALID_PARAMETER)
  

•                   return CMD_RET_USAGE;
  

•           else if (ret != EFI_SUCCESS)
  

•           if (ret != EFI_SUCCESS)
                    return CMD_RET_FAILURE;
  
            return do_efi_selftest();
  

-- 2.43.0

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org