On 6/6/23 21:43, Raymond Mao wrote:
Hi Ilias,
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -663,11 +663,13 @@ efi_status_t
efi_bootmgr_update_media_device_boot_option(void)
NULL, &count,
(efi_handle_t **)&volume_handles);
if (ret != EFI_SUCCESS)
- return ret;
+ goto out;
I missed that in my original review, but I think this change is wrong.
If you follow the goto out tag now instead of returning directly
there's a chance efi_locate_handle_buffer_int() will return
EFI_NOT_FOUND. The goto out tag will convert that to EFI_SUCCESS
although it's an actual error.
This is what we intend to do. When there are no removable medias plug-in,
efi_locate_handle_buffer_int() will return EFI_NOT_FOUND.
And if we don't convert it to EFI_SUCCESS here,
efi_bootmgr_update_media_device_boot_option()
will return EFI_NOT_FOUND and then efi_init_obj_list() fails.
This leads to the efi init failure and it will just prompt 'Error:
Cannot initialize UEFI sub-system'
when you run any efidebug commands.
Regards,
Raymond
On Tue, 6 Jun 2023 at 15:24, Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
Hi Raymond
On Tue, 6 Jun 2023 at 19:37, Raymond Mao <raymond....@linaro.org> wrote:
Correct the return code for out-of-memory and no boot option found
Signed-off-by: Raymond Mao <raymond....@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
---
Changes in v7
- new patch file created
cmd/bootmenu.c | 2 +-
cmd/eficonfig.c | 2 +-
lib/efi_loader/efi_bootmgr.c | 8 ++++++--
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 01daddca7b..987b16889f 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -352,7 +352,7 @@ static struct bootmenu_data *bootmenu_create(int delay)
* a architecture-specific default image name such as
BOOTAA64.EFI.
*/
efi_ret = efi_bootmgr_update_media_device_boot_option();
- if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND)
+ if (efi_ret != EFI_SUCCESS)
goto cleanup;
ret = prepare_uefi_bootorder_entry(menu, &iter, &i);
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 82a80306f4..e6e8a0a488 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -2314,7 +2314,7 @@ static int do_eficonfig(struct cmd_tbl *cmdtp, int flag,
int argc, char *const a
return CMD_RET_FAILURE;
ret = efi_bootmgr_update_media_device_boot_option();
- if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND)
+ if (ret != EFI_SUCCESS)
return ret;
while (1) {
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 97d5b8dc2b..28e800499c 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -663,11 +663,13 @@ efi_status_t
efi_bootmgr_update_media_device_boot_option(void)
NULL, &count,
(efi_handle_t **)&volume_handles);
if (ret != EFI_SUCCESS)
- return ret;
+ goto out;
I missed that in my original review, but I think this change is wrong.
If you follow the goto out tag now instead of returning directly
there's a chance efi_locate_handle_buffer_int() will return
EFI_NOT_FOUND. The goto out tag will convert that to EFI_SUCCESS
although it's an actual error.
It was due to my reply
https://lore.kernel.org/u-boot/e951264b-f952-3e83-dacc-bbe2fa377...@gmx.de/
that Raymond moved the EFI_NOT_FOUND handling into this function. As
said non-availability of a file system on removable media should not
stop the initialization of the EFI sub-system or the probing of block
devices.
Why is efi_bootmgr_update_media_device_boot_option() not invoked when
block devices are removed?
Best regards
Heinrich
Thanks
/Ilias
opt = calloc(count, sizeof(struct eficonfig_media_boot_option));
- if (!opt)
+ if (!opt) {
+ ret = EFI_OUT_OF_RESOURCES;
goto out;
+ }
/* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */
ret = efi_bootmgr_enumerate_boot_option(opt, volume_handles, count);
@@ -720,5 +722,7 @@ out:
free(opt);
efi_free_pool(volume_handles);
+ if (ret == EFI_NOT_FOUND)
+ return EFI_SUCCESS;
return ret;
}
--
2.25.1
