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>,
	jk@ozlabs.org, ardb@kernel.org, mjg59@srcf.ucam.org,
	pjones@redhat.com, linux-kernel@vger.kernel.org
Subject: [PATCH v3] efivarfs: Request at most 512 bytes for variable names
Date: Fri, 26 Jan 2024 17:25:23 +0100	[thread overview]
Message-ID: <20240126162524.52051-1-timschumi@gmx.de> (raw)
In-Reply-To: <20240123202743.1591165-1-timschumi@gmx.de>

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

Cc: stable@vger.kernel.org # 6.1+
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
---
Made a patch with just the size change (and a comment / error message
for good measure) as requested. I just hope that I don't have to come
back in a month to find out that there is a machine that only accepts up
to (e.g.) 256 bytes.

One thing that I just recently noticed is that properly processing
variables above 512 bytes in size is currently meaningless anyways,
since the VFS layer only allows file name sizes of up to 255 bytes,
and 512 bytes of UCS2 will end up being at least 256 bytes of
UTF-8.

If path creation errors are decoupled from the iteration process then
one could at least skip those entries, but the efivarfs implementation
seems to be in no shape to handle that currently.
---
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.

Changes since v2 ("efivarfs: Halve name buffer size until first successful response"):

- Removed all the complicated logic, only keeping the new limit, the
  comment change, and the error message in case a big variable is found.
---
 fs/efivarfs/vars.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/efivarfs/vars.c b/fs/efivarfs/vars.c
index 9e4f47808bd5..3f6385f2a4e5 100644
--- a/fs/efivarfs/vars.c
+++ b/fs/efivarfs/vars.c
@@ -372,7 +372,7 @@ 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_size = 512;
 	efi_char16_t *variable_name;
 	efi_status_t status;
 	efi_guid_t vendor_guid;
@@ -389,12 +389,13 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
 		goto free;

 	/*
-	 * Per EFI spec, the maximum storage allocated for both
-	 * the variable name and variable data is 1024 bytes.
+	 * A small set of old UEFI implementations reject sizes
+	 * above a certain threshold, the lowest seen in the wild
+	 * is 512.
 	 */

 	do {
-		variable_name_size = 1024;
+		variable_name_size = 512;

 		status = efivar_get_next_variable(&variable_name_size,
 						  variable_name,
@@ -431,6 +432,11 @@ 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 512, got name of size %lu\n",
+			       variable_name_size);
+			status = EFI_NOT_FOUND;
+			break;
 		default:
 			printk(KERN_WARNING "efivars: get_next_variable: status=%lx\n",
 				status);
--
2.43.0


  reply	other threads:[~2024-01-26 16:25 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-22 23:15 [PATCH] efivarfs: Iterate variables with increasing name buffer sizes Tim Schumacher
2024-01-23 11:24 ` 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   ` Tim Schumacher [this message]
2024-01-26 16:35     ` [PATCH v3] efivarfs: Request at most 512 bytes for variable names 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=20240126162524.52051-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 \
    --cc=mjg59@srcf.ucam.org \
    --cc=pjones@redhat.com \
    /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).