[PATCH 1/1] efi_loader: segfault in efi_clear_os_indications()

[PATCH 1/1] efi_loader: segfault in efi_clear_os_indications()

[Heinrich Schuchardt]

If we call efi_clear_os_indications() before initializing the memory store
for UEFI variables a NULL pointer dereference occurs.

The error was observed on the sandbox with:

    usb start
    host bind 0 sandbox.img
    load host 0:1 $kernel_addr_r helloworld.efi
    bootefi $kernel_addr_r

Here efi_resister_disk() failed due to an error in the BTRFS implementation.

It is enough to clear EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED
if we have successfully initialized the UEFI sub-system.

Fixes: 149108a3eb59 ("efi_loader: clear OsIndications")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/efi_loader/efi_setup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index a2338d74af..661868811f 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -331,11 +331,10 @@ efi_status_t efi_init_obj_list(void)
 	if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
 	    !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
 		ret = efi_launch_capsules();
-
-out:
 	r = efi_clear_os_indications();
 	if (ret == EFI_SUCCESS)
 		ret = r;
+out:
 	efi_obj_list_initialized = ret;
 	return ret;
 }
-- 
2.32.0

Re: [PATCH 1/1] efi_loader: segfault in efi_clear_os_indications()

[AKASHI Takahiro]

On Sat, Nov 20, 2021 at 12:02:25PM +0100, Heinrich Schuchardt wrote:
> If we call efi_clear_os_indications() before initializing the memory store
> for UEFI variables a NULL pointer dereference occurs.
> 
> The error was observed on the sandbox with:
> 
>     usb start
>     host bind 0 sandbox.img
>     load host 0:1 $kernel_addr_r helloworld.efi
>     bootefi $kernel_addr_r
> 
> Here efi_resister_disk() failed due to an error in the BTRFS implementation.
> 
> It is enough to clear EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED
> if we have successfully initialized the UEFI sub-system.

I think that it would be better move this function into
efi_launch_capsules() as we should defer to a responsible sub-component,
capsule support in this case, on whether a particular bit should be cleared.

-Takahiro Akashi

> Fixes: 149108a3eb59 ("efi_loader: clear OsIndications")
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  lib/efi_loader/efi_setup.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index a2338d74af..661868811f 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -331,11 +331,10 @@ efi_status_t efi_init_obj_list(void)
>  	if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
>  	    !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
>  		ret = efi_launch_capsules();
> -
> -out:
>  	r = efi_clear_os_indications();
>  	if (ret == EFI_SUCCESS)
>  		ret = r;
> +out:
>  	efi_obj_list_initialized = ret;
>  	return ret;
>  }
> -- 
> 2.32.0
>