[PATCH] bootmenu: fix leak of entries for missing or inactive boot options

[PATCH] bootmenu: fix leak of entries for missing or inactive boot options

[Yuho Choi]

prepare_uefi_bootorder_entry() allocates a menu entry for each index in
BootOrder before it reads the Boot#### variable. The entry is only
linked into the menu if the variable exists and the boot option has
LOAD_OPTION_ACTIVE set, and bootmenu_destroy() only frees linked
entries. When the variable is missing or the option is inactive, the
entry is neither linked nor freed.

With CONFIG_CMD_BOOTEFI_BOOTMGR=y and CONFIG_CMD_EFICONFIG=y, each time
the menu is created with UEFI entries ('bootmenu -e' or
CONFIG_AUTOBOOT_MENU_SHOW), one entry is therefore leaked for every
such BootOrder index.

Allocate the entry only for an active boot option. If this allocation
fails, free load_option and bootorder before returning, as the error
path for the title buffer already does; bootorder used to be leaked
there.

Fixes: c606c0059f7f ("bootmenu: add UEFI boot entry into bootmenu")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
Tested on sandbox_defconfig (LTO disabled for gdb) with Boot0001 active,
Boot0002 missing and Boot0003 inactive in BootOrder, running
'bootmenu -e' with bootmenu_delay=1 several times and reading
dlmallinfo().uordblks before each run. Without this patch each run used
128 bytes more heap than a control run with only Boot0001 in BootOrder,
which matches the two 48-byte entries; with it there is no difference.
The menu still lists only Boot0001.

 cmd/bootmenu.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index b55f5579409..39fc16f8c27 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -304,10 +304,6 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu,
 
 	num = size / sizeof(u16);
 	for (j = 0; j < num; j++) {
-		entry = malloc(sizeof(struct bootmenu_entry));
-		if (!entry)
-			return -ENOMEM;
-
 		efi_create_indexed_name(varname, sizeof(varname),
 					"Boot", bootorder[j]);
 		load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
@@ -318,13 +314,18 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu,
 		if (ret != EFI_SUCCESS) {
 			log_warning("Invalid load option for %ls\n", varname);
 			free(load_option);
-			free(entry);
 			continue;
 		}
 
 		if (lo.attributes & LOAD_OPTION_ACTIVE) {
 			char *buf;
 
+			entry = malloc(sizeof(struct bootmenu_entry));
+			if (!entry) {
+				free(load_option);
+				free(bootorder);
+				return -ENOMEM;
+			}
 			buf = calloc(1, utf16_utf8_strlen(lo.label) + 1);
 			if (!buf) {
 				free(load_option);
-- 
2.43.0