linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tim Schumacher <timschumi@gmx.de>
To: linux-efi@vger.kernel.org
Cc: Tim Schumacher <timschumi@gmx.de>, Jeremy Kerr <jk@ozlabs.org>,
	Ard Biesheuvel <ardb@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] efivarfs: Iterate variables with increasing name buffer sizes
Date: Tue, 23 Jan 2024 00:15:05 +0100	[thread overview]
Message-ID: <20240122231507.1307793-1-timschumi@gmx.de> (raw)

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@vger.kernel.org # 6.1+
Signed-off-by: Tim Schumacher <timschumi@gmx.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


             reply	other threads:[~2024-01-22 23:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-22 23:15 Tim Schumacher [this message]
2024-01-23 11:24 ` [PATCH] efivarfs: Iterate variables with increasing name buffer sizes Ard Biesheuvel
2024-01-23 13:55   ` Tim Schumacher
2024-01-23 14:09     ` Ard Biesheuvel
2024-01-23 17:33       ` Tim Schumacher
2024-01-24 21:25         ` Peter Jones
2024-01-25  8:12           ` Ard Biesheuvel
2024-04-13 10:47           ` Tim Schumacher
2024-01-23 20:27 ` [PATCH v2] efivarfs: Halve name buffer size until first successful response Tim Schumacher
2024-01-26 16:25   ` [PATCH v3] efivarfs: Request at most 512 bytes for variable names Tim Schumacher
2024-01-26 16:35     ` Ard Biesheuvel
2024-01-26 18:02       ` Tim Schumacher
2024-01-30 16:00         ` Tim Schumacher
2024-02-14 15:18           ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240122231507.1307793-1-timschumi@gmx.de \
    --to=timschumi@gmx.de \
    --cc=ardb@kernel.org \
    --cc=jk@ozlabs.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).