From: Masahisa Kojima <masahisa.kojima@linaro.org>
Date: Mon, 19 Dec 2022 02:33:13 +0000 (+0900)
Subject: eficonfig: avoid SetVariable between GetNextVariableName calls
X-Git-Tag: v2023.07~225^2~3
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=645502743ac10ffaa5abf944bf0b751b32b47220;p=platform%2Fkernel%2Fu-boot.git

eficonfig: avoid SetVariable between GetNextVariableName calls

The current code calls efi_set_variable_int() to delete the
invalid boot option between calls to efi_get_next_variable_name_int(),
it may produce unpredictable results.

This commit moves removal of the invalid boot option outside
of the efi_get_next_variable_name_int() calls.
EFI_NOT_FOUND returned from efi_get_next_variable_name_int()
indicates we retrieved all EFI variables, it should be treated
as EFI_SUCEESS.

To address the checkpatch warning of too many leading tabs,
combine two if statement into one.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 0b07dfc958..ce7175a566 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -2310,13 +2310,14 @@ out:
 efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_option *opt,
 						  efi_status_t count)
 {
-	u32 i;
 	efi_uintn_t size;
 	void *load_option;
+	u32 i, list_size = 0;
 	struct efi_load_option lo;
 	u16 *var_name16 = NULL;
 	u16 varname[] = u"Boot####";
 	efi_status_t ret = EFI_SUCCESS;
+	u16 *delete_index_list = NULL, *p;
 	efi_uintn_t buf_size;
 
 	buf_size = 128;
@@ -2331,8 +2332,14 @@ efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_op
 		efi_uintn_t tmp;
 
 		ret = efi_next_variable_name(&buf_size, &var_name16, &guid);
-		if (ret == EFI_NOT_FOUND)
+		if (ret == EFI_NOT_FOUND) {
+			/*
+			 * EFI_NOT_FOUND indicates we retrieved all EFI variables.
+			 * This should be treated as success.
+			 */
+			ret = EFI_SUCCESS;
 			break;
+		}
 		if (ret != EFI_SUCCESS)
 			goto out;
 
@@ -2349,31 +2356,46 @@ efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_op
 		if (ret != EFI_SUCCESS)
 			goto next;
 
-		if (size >= sizeof(efi_guid_bootmenu_auto_generated)) {
-			if (guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated) == 0) {
-				for (i = 0; i < count; i++) {
-					if (opt[i].size == tmp &&
-					    memcmp(opt[i].lo, load_option, tmp) == 0) {
-						opt[i].exist = true;
-						break;
-					}
+		if (size >= sizeof(efi_guid_bootmenu_auto_generated) &&
+		    !guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated)) {
+			for (i = 0; i < count; i++) {
+				if (opt[i].size == tmp &&
+				    memcmp(opt[i].lo, load_option, tmp) == 0) {
+					opt[i].exist = true;
+					break;
 				}
+			}
 
-				if (i == count) {
-					ret = delete_boot_option(i);
-					if (ret != EFI_SUCCESS) {
-						free(load_option);
-						goto out;
-					}
+			/*
+			 * The entire list of variables must be retrieved by
+			 * efi_get_next_variable_name_int() before deleting the invalid
+			 * boot option, just save the index here.
+			 */
+			if (i == count) {
+				p = realloc(delete_index_list, sizeof(u32) *
+					    (list_size + 1));
+				if (!p) {
+					ret = EFI_OUT_OF_RESOURCES;
+					goto out;
 				}
+				delete_index_list = p;
+				delete_index_list[list_size++] = index;
 			}
 		}
 next:
 		free(load_option);
 	}
 
+	/* delete all invalid boot options */
+	for (i = 0; i < list_size; i++) {
+		ret = delete_boot_option(delete_index_list[i]);
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
 out:
 	free(var_name16);
+	free(delete_index_list);
 
 	return ret;
 }