On 7/28/26 21:33, Scott Moser wrote:
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]>
Reviewed-by: Heinrich Schuchardt <[email protected]>
None of the loops handles EFI_BUFFER_TOO_SMALL. Just create a variable
called fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210
to break the code.
We should move all calls of GetNextVariable() to a new function in
lib/efi_loader/efi_var_common.c with proper handling of the variable
name buffer.
But let's merge the current patch first.
Best regards
Heinrich
---
Changes for v2:
- Drop the "= {}" initializer on the hoisted guid: per the UEFI spec,
VendorGuid is ignored when VariableName is an empty string, which is
the state var_name16 is in on the first call, so the initial value is
never read and zero-initializing it only costs code size.
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..d8e7ed6666a 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..8938b214ceb 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);
