Re: [PATCH] efi_loader: fix use of uninit ialized guid in variable enumeration loops

Am 28. Juli 2026 18:49:48 MESZ schrieb Scott Moser <[email protected]>:
>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 = {};

Thank you for addressing this issue.

According to the UEFI specification:

"When VariableName is a pointer to a Null character, VendorGuid is ignored."

Initialization of the GUID is not needed and should be avoided to reduce the code size.

> 
> 	/* 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 = {};

ditto

Best regards

Heinrich


> 
> 	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);