linux-kernel - [PATCH] efivarfs: Iterate variables with increasing name buffer sizes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240122231507.1307793-1-timschumi@gmx.de>
Date: Tue, 23 Jan 2024 00:15:05 +0100
From: Tim Schumacher <timschumi@....de>
To: linux-efi@...r.kernel.org
Cc: Tim Schumacher <timschumi@....de>,
	Jeremy Kerr <jk@...abs.org>,
	Ard Biesheuvel <ardb@...nel.org>,
	linux-kernel@...r.kernel.org
Subject: [PATCH] efivarfs: Iterate variables with increasing name buffer sizes

This sidesteps a quirk in a few old (2011-ish) UEFI implementations,
where a call to `GetNextVariableName` with a buffer size larger than 512
bytes will always return `EFI_INVALID_PARAMETER`.

It is currently unknown whether this is just a botched check or if the
length is interpreted differently, so the underlying buffer is still
sized for 1024 bytes, even if we communicate a smaller size to the
runtime service.

Cc: stable@...r.kernel.org # 6.1+
Signed-off-by: Tim Schumacher <timschumi@....de>
---
linux-stable is CC'd because this issue (together with a few other
unfortunate non-kernel edge-cases) resulted in semi-bricked machines
when installing various Linux distributions across the last 10+ years.

The short explanation is that efibootmgr created an entirely new list
of boot options when not being able to read the existing one, which
wiped the device-based entries from `BootOrder` and overwrote the "BIOS
Setup" entry that was stored in `Boot0000`, making the setup menu
completely inaccessible on the hardware that I'm testing on.

Matching bug reports exist for Ubuntu [1] and Debian [2], they just
don't mention the root issue because I finished tracking this down only
yesterday.

As mentioned in the commit message and comment, the reason for rejecting
buffers larger than 512 bytes is still unknown, but I plan to continue
looking at the UEFI binary once I have a bit more time. Depending on the
results of that, the accommodations we make here could be toned down
again.

[1] https://bugs.launchpad.net/ubuntu/+source/efibootmgr/+bug/1082418
[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=887139
---
 fs/efivarfs/vars.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/fs/efivarfs/vars.c b/fs/efivarfs/vars.c
index 9e4f47808bd5..55a1468af3fa 100644
--- a/fs/efivarfs/vars.c
+++ b/fs/efivarfs/vars.c
@@ -372,13 +372,15 @@ static void dup_variable_bug(efi_char16_t *str16, efi_guid_t *vendor_guid,
 int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
 		void *data, bool duplicates, struct list_head *head)
 {
-	unsigned long variable_name_size = 1024;
+	unsigned long variable_name_allocation_size = 1024;
+	unsigned long variable_name_advertised_size = 512;
 	efi_char16_t *variable_name;
+	efi_char16_t *variable_name_tmp;
 	efi_status_t status;
 	efi_guid_t vendor_guid;
 	int err = 0;

-	variable_name = kzalloc(variable_name_size, GFP_KERNEL);
+	variable_name = kzalloc(variable_name_allocation_size, GFP_KERNEL);
 	if (!variable_name) {
 		printk(KERN_ERR "efivars: Memory allocation failed.\n");
 		return -ENOMEM;
@@ -391,10 +393,16 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
 	/*
 	 * Per EFI spec, the maximum storage allocated for both
 	 * the variable name and variable data is 1024 bytes.
+	 *
+	 * However, a small set of old UEFI implementations
+	 * reject large sizes, so we start off with advertising
+	 * something that is more digestible. The underlying
+	 * buffer is still 1024 bytes in size, in case the implementation
+	 * interprets the size differently.
 	 */

 	do {
-		variable_name_size = 1024;
+		unsigned long variable_name_size = variable_name_advertised_size;

 		status = efivar_get_next_variable(&variable_name_size,
 						  variable_name,
@@ -431,6 +439,22 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
 			break;
 		case EFI_NOT_FOUND:
 			break;
+		case EFI_BUFFER_TOO_SMALL:
+			variable_name_advertised_size = variable_name_size;
+			if (variable_name_size <= variable_name_allocation_size)
+				break;
+
+			variable_name_tmp = krealloc(variable_name, variable_name_size, GFP_KERNEL);
+
+			if (!variable_name_tmp) {
+				printk(KERN_ERR "efivars: Memory reallocation failed.\n");
+				err = -ENOMEM;
+				status = EFI_NOT_FOUND;
+				break;
+			}
+			variable_name = variable_name_tmp;
+			variable_name_allocation_size = variable_name_size;
+			break;
 		default:
 			printk(KERN_WARNING "efivars: get_next_variable: status=%lx\n",
 				status);
--
2.43.0