efi_loader: segfault in efi_clear_os_indications()

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>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 502bcfca6ec9f9230cbb71124dab09477062a00d..8301eed6317db0c3b33ec7588d1f1d931750b90d 100644 (file)
--- 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 a2338d74aface356bc58c7372a0365b34773ca6d..1aba71cd96240b2d6ffdfd98065f3cdde8b966c5 100644 (file)
--- 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;
 }