efi_bootmgr_delete_invalid_boot_option(), eficonfig_show_boot_selection(),
and eficonfig_create_change_boot_order_entry() each enumerate all EFI
variables by repeatedly calling efi_next_variable_name() in a loop,
passing the same efi_guid_t as both input and output. GetNextVariableName()
needs the vendor GUID returned by the previous call, together with the
variable name it returned, to know where to resume.
In each of these loops the efi_guid_t was declared inside the loop body,
so a new instance comes into scope on every iteration. Relying on it to
still hold the previous iteration's value depends on the compiler reusing
the same stack slot across iterations, which is undefined behavior. With
a compiler that zero-initializes locals by default (e.g. clang, or gcc
configured with -ftrivial-auto-var-init=zero), the GUID is cleared on
every iteration, so the lookup of the variable name returned by the
previous call fails and efi_init_obj_list() aborts:
Cannot initialize UEFI sub-system
** Booting bootflow ... with efi
Boot failed (err=-22)
Move the efi_guid_t declarations out of the loops so the value written
by the previous efi_next_variable_name() call is preserved across
iterations.
Fixes: 140a8959d48f ("eficonfig: use efi_get_next_variable_name_int()")
Signed-off-by: Scott Moser <[email protected]>
---
cmd/eficonfig.c | 4 ++--
lib/efi_loader/efi_bootmgr.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 4d060e3007c..cd66f05fb7a 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -1844,6 +1844,7 @@ static efi_status_t
eficonfig_show_boot_selection(unsigned int *selected)
struct efimenu *efi_menu;
struct list_head *pos, *n;
struct eficonfig_entry *entry;
+ efi_guid_t guid = {};
efi_menu = calloc(1, sizeof(struct efimenu));
if (!efi_menu)
@@ -1872,7 +1873,6 @@ static efi_status_t
eficonfig_show_boot_selection(unsigned int *selected)
var_name16[0] = 0;
for (;;) {
int index;
- efi_guid_t guid;
ret = efi_next_variable_name(&buf_size, &var_name16, &guid);
if (ret == EFI_NOT_FOUND)
@@ -2245,6 +2245,7 @@ static efi_status_t
eficonfig_create_change_boot_order_entry(struct efimenu *efi
u16 *var_name16 = NULL;
efi_uintn_t size, buf_size;
struct eficonfig_save_boot_order_data *save_data;
+ efi_guid_t guid = {};
/* list the load option in the order of BootOrder variable */
for (i = 0; i < num; i++) {
@@ -2265,7 +2266,6 @@ static efi_status_t
eficonfig_create_change_boot_order_entry(struct efimenu *efi
var_name16[0] = 0;
for (;;) {
int index;
- efi_guid_t guid;
if (efi_menu->count >= EFICONFIG_ENTRY_NUM_MAX - 2)
break;
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 8c9a9b5eb56..3ee47000d23 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -934,6 +934,7 @@ static efi_status_t
efi_bootmgr_delete_invalid_boot_option(struct eficonfig_medi
efi_status_t ret = EFI_SUCCESS;
u16 *delete_index_list = NULL, *p;
efi_uintn_t buf_size;
+ efi_guid_t guid = {};
buf_size = 128;
var_name16 = malloc(buf_size);
@@ -943,7 +944,6 @@ static efi_status_t
efi_bootmgr_delete_invalid_boot_option(struct eficonfig_medi
var_name16[0] = 0;
for (;;) {
int index;
- efi_guid_t guid;
efi_uintn_t tmp;
ret = efi_next_variable_name(&buf_size, &var_name16, &guid);
--
2.55.0
