[PATCH v8 2/4] Fix incorrect return code of boot option update
Raymond Mao
raymond.mao at linaro.org
Tue Jun 6 21:43:22 CEST 2023
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.apalodimas at linaro.org> wrote:
>
> Hi Raymond
>
> On Tue, 6 Jun 2023 at 19:37, Raymond Mao <raymond.mao at linaro.org> wrote:
> >
> > Correct the return code for out-of-memory and no boot option found
> >
> > Signed-off-by: Raymond Mao <raymond.mao at linaro.org>
> > Reviewed-by: Ilias Apalodimas <ilias.apalodimas at 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.
>
> 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
> >
More information about the U-Boot
mailing list