On Wed, 24 Nov 2021 at 09:54, Heinrich Schuchardt <heinrich.schucha...@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.schucha...@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.apalodi...@linaro.org>