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; }