efi_loader: fix use of uninitialized guid in variable enumeration loops

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 <smoser@brickies.net>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 4d060e3007cc696411483f1c48c1d4d6925ffb30..d8e7ed6666a14943a341681e9d009e708dddc243 100644 (file)
--- 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 8c9a9b5eb564ecbd6b4b7ba306147cbc9b2aeb54..8938b214ceb394867698efe5a4b16949a4da3ee9 100644 (file)
--- 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);