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

* [PATCH v2 1/1] efi_loader: segfault in efi_clear_os_indications()
@ 2021-11-24  7:54 Heinrich Schuchardt
  2021-11-25 10:53 ` Ilias Apalodimas
  0 siblings, 1 reply; 2+ messages in thread
From: Heinrich Schuchardt @ 2021-11-24  7:54 UTC (permalink / raw)
  To: u-boot
  Cc: Alexander Graf, AKASHI Takahiro, Ilias Apalodimas,
	Masami Hiramatsu, 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.

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread