On Wed, 24 Nov 2021 at 09:54, Heinrich Schuchardt heinrich.schuchardt@canonical.com 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.

Move the logic to clear EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED to the rest of the capsule code.

If CONFIG_EFI_IGNORE_OSINDICATIONS=y, we should still clear the flag. If OsIndications does not exist, we should not create it as it is owned by the operating system.

Fixes: 149108a3eb59 ("efi_loader: clear OsIndications") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

---

v2: move the logic to the rest of the capsule code

---

lib/efi_loader/efi_capsule.c | 45 ++++++++++++++++++++++++------------ lib/efi_loader/efi_setup.c | 36 +---------------------------- 2 files changed, 31 insertions(+), 50 deletions(-)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 502bcfca6e..8301eed631 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1037,30 +1037,45 @@ efi_status_t __weak efi_load_capsule_drivers(void) }

/**

• • check_run_capsules - Check whether capsule update should run

• • check_run_capsules() - check whether capsule update should run
  • The spec says OsIndications must be set in order to run the capsule update
  • on-disk. Since U-Boot doesn't support runtime SetVariable, allow capsules to
  • run explicitly if CONFIG_EFI_IGNORE_OSINDICATIONS is selected
• • Return: EFI_SUCCESS if update to run, EFI_NOT_FOUND otherwise
  */

-static bool check_run_capsules(void) +static efi_status_t check_run_capsules(void) { u64 os_indications; efi_uintn_t size;

•   efi_status_t ret;
  

•   if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS))
  

•           return true;
  

•   efi_status_t r;
  
    size = sizeof(os_indications);
  

•   ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
  

•                              NULL, &size, &os_indications, NULL);
  

•   if (ret == EFI_SUCCESS &&
  

•       (os_indications
  

•         & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
  

•           return true;
  

•   return false;
  

•   r = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
  

•                            NULL, &size, &os_indications, NULL);
  

•   if (r != EFI_SUCCESS || size != sizeof(os_indications))
  

•           return EFI_NOT_FOUND;
  

•   if (os_indications &
  

•       EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED) {
  

•           os_indications &=
  

•                   ~EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED;
  

•           r = efi_set_variable_int(L"OsIndications",
  

•                                    &efi_global_variable_guid,
  

•                                    EFI_VARIABLE_NON_VOLATILE |
  

•                                    EFI_VARIABLE_BOOTSERVICE_ACCESS |
  

•                                    EFI_VARIABLE_RUNTIME_ACCESS,
  

•                                    sizeof(os_indications),
  

•                                    &os_indications, false);
  

•           if (r != EFI_SUCCESS)
  

•                   log_err("Setting %ls failed\n", L"OsIndications");
  

•           return EFI_SUCCESS;
  

•   } else if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS)) {
  

•           return EFI_SUCCESS;
  

•   } else  {
  

•           return EFI_NOT_FOUND;
  

•   }
  

}

/** @@ -1078,7 +1093,7 @@ efi_status_t efi_launch_capsules(void) unsigned int nfiles, index, i; efi_status_t ret;

•   if (!check_run_capsules())
  

•   if (check_run_capsules() != EFI_SUCCESS)
            return EFI_SUCCESS;
  
    index = get_last_capsule();
  

diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index a2338d74af..1aba71cd96 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -175,36 +175,6 @@ static efi_status_t efi_init_os_indications(void) }

-/**

• • efi_clear_os_indications() - clear OsIndications
• • Clear EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED
• */

-static efi_status_t efi_clear_os_indications(void) -{

•   efi_uintn_t size;
  

•   u64 os_indications;
  

•   efi_status_t ret;
  

•   size = sizeof(os_indications);
  

•   ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid,
  

•                              NULL, &size, &os_indications, NULL);
  

•   if (ret != EFI_SUCCESS)
  

•           os_indications = 0;
  

•   else
  

•           os_indications &=
  

•                   ~EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED;
  

•   ret = efi_set_variable_int(L"OsIndications", &efi_global_variable_guid,
  

•                              EFI_VARIABLE_NON_VOLATILE |
  

•                              EFI_VARIABLE_BOOTSERVICE_ACCESS |
  

•                              EFI_VARIABLE_RUNTIME_ACCESS,
  

•                              sizeof(os_indications), &os_indications,
  

•                              false);
  

•   if (ret != EFI_SUCCESS)
  

•           log_err("Setting %ls failed\n", L"OsIndications");
  

•   return ret;
  

-}

/**

• efi_init_obj_list() - Initialize and populate EFI object list

@@ -212,7 +182,7 @@ static efi_status_t efi_clear_os_indications(void) */ efi_status_t efi_init_obj_list(void) {

•   efi_status_t r, ret = EFI_SUCCESS;
  

•   efi_status_t ret = EFI_SUCCESS;
  
    /* Initialize once only */
    if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
  

@@ -331,11 +301,7 @@ 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;
    efi_obj_list_initialized = ret;
    return ret;
  

}

2.32.0

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