Re: [PATCH 1/1] efi_loader: fix efi_initrd_deregister()

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Fri, Sep 30, 2022 at 01:57:48AM +0200, Heinrich Schuchardt wrote:
> Don't try to delete a non-existent handle.

It is okay as a safe guard, but it doesn't fix underlying issues.

efi_initrd_register() is called only in efi_bootmgr_load(), and so
efi_initrd_deregister() should be called only at the paired location.

- Remove efi_initrd_deregister() from do_bootefi_exec()
- do_efibootmgr() should look like
      efi_bootmgr_load()
      do_bootefi_exec()
      efi_initrd_deregister()
Otherwise, do_bootefi_exec() always tries to free a handle in
the other cases (i.e. bootefi <addr>).

Another issue is there in try_load_entry() called by efi_bootmgr_load().
     (after efi_initrd_register())
     if (size) {
                *load_options = malloc(size);
                if (!*load_options) {
                        ret = EFI_OUT_OF_RESOURCES;
                        goto error;
                }
		...

If malloc() fails, we should call efi_initrd_deregister() within
try_load_entry().

Should I submit a patch?

-Takahiro Akashi

> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  lib/efi_loader/efi_load_initrd.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index c5e6652e66..3d6044f760 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -230,6 +230,9 @@ efi_status_t efi_initrd_register(void)
>   */
>  void efi_initrd_deregister(void)
>  {
> +	if (!efi_initrd_handle)
> +		return;
> +
>  	efi_delete_handle(efi_initrd_handle);
>  	efi_initrd_handle = NULL;
>  }
> -- 
> 2.37.2
>