* [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
* Re: [PATCH v2 1/1] efi_loader: segfault in efi_clear_os_indications()
2021-11-24 7:54 [PATCH v2 1/1] efi_loader: segfault in efi_clear_os_indications() Heinrich Schuchardt
@ 2021-11-25 10:53 ` Ilias Apalodimas
0 siblings, 0 replies; 2+ messages in thread
From: Ilias Apalodimas @ 2021-11-25 10:53 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: u-boot, Alexander Graf, AKASHI Takahiro, Masami Hiramatsu
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>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-11-25 10:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-24 7:54 [PATCH v2 1/1] efi_loader: segfault in efi_clear_os_indications() Heinrich Schuchardt
2021-11-25 10:53 ` Ilias Apalodimas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.