linux-kernel - [PATCH v2] efivarfs: Halve name buffer size until first successful response

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240123202743.1591165-1-timschumi@gmx.de>
Date: Tue, 23 Jan 2024 21:27:42 +0100
From: Tim Schumacher <timschumi@....de>
To: linux-efi@...r.kernel.org
Cc: Tim Schumacher <timschumi@....de>,
	jk@...abs.org,
	ardb@...nel.org,
	mjg59@...f.ucam.org,
	pjones@...hat.com,
	linux-kernel@...r.kernel.org
Subject: [PATCH v2] efivarfs: Halve name buffer size until first successful response

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`.

Additionally, remove a related but outdated comment that claims the
maximum variable name size to be 1024. The default buffer size of 1024
is kept to ensure that existing setups keep working.

Cc: stable@...r.kernel.org # 6.1+
Signed-off-by: Tim Schumacher <timschumi@....de>
---
Changes since v1 ("efivarfs: Iterate variables with increasing name buffer sizes"):

- Redid the logic to start with the current limit of 1024 and continuously
  halve it until we get a successful result.
- Added a warning line in case we find anything that is bigger than the limit.
- Removed an outdated (or never accurate?) comment about a specification-imposed
  length limit.
---
 fs/efivarfs/vars.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/fs/efivarfs/vars.c b/fs/efivarfs/vars.c
index 9e4f47808bd5..26a10ed4a116 100644
--- a/fs/efivarfs/vars.c
+++ b/fs/efivarfs/vars.c
@@ -372,13 +372,14 @@ 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 maximum_variable_name_size = 1024;
 	efi_char16_t *variable_name;
 	efi_status_t status;
 	efi_guid_t vendor_guid;
 	int err = 0;
+	bool successful_once = false;

-	variable_name = kzalloc(variable_name_size, GFP_KERNEL);
+	variable_name = kzalloc(maximum_variable_name_size, GFP_KERNEL);
 	if (!variable_name) {
 		printk(KERN_ERR "efivars: Memory allocation failed.\n");
 		return -ENOMEM;
@@ -388,19 +389,16 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
 	if (err)
 		goto free;

-	/*
-	 * Per EFI spec, the maximum storage allocated for both
-	 * the variable name and variable data is 1024 bytes.
-	 */
-
 	do {
-		variable_name_size = 1024;
+		unsigned long variable_name_size = maximum_variable_name_size;

 		status = efivar_get_next_variable(&variable_name_size,
 						  variable_name,
 						  &vendor_guid);
 		switch (status) {
 		case EFI_SUCCESS:
+			successful_once = true;
+
 			variable_name_size = var_name_strnsize(variable_name,
 							       variable_name_size);

@@ -431,6 +429,23 @@ 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:
+			printk(KERN_WARNING "efivars: Assumed maximum name size to be 0x%lx, got name of size 0x%lx\n",
+			       maximum_variable_name_size, variable_name_size);
+			status = EFI_NOT_FOUND;
+			break;
+		case EFI_INVALID_PARAMETER:
+			if (!successful_once && maximum_variable_name_size > 1) {
+				/*
+				 * A small set of old UEFI implementations reject sizes
+				 * above a certain threshold. Halve the advertised size
+				 * until we get the first successful response.
+				 */
+				maximum_variable_name_size /= 2;
+				break;
+			}
+
+			fallthrough;
 		default:
 			printk(KERN_WARNING "efivars: get_next_variable: status=%lx\n",
 				status);
--
2.43.0