diff mbox series

[5.15-] efivars: Request at most 512 bytes for variable names

Message ID 20240317023326.285140-1-timschumi@gmx.de
State New
Headers show
Series [5.15-] efivars: Request at most 512 bytes for variable names | expand

Commit Message

Tim Schumacher March 17, 2024, 2:33 a.m. UTC 
commit f45812cc23fb74bef62d4eb8a69fe7218f4b9f2a upstream.

Work around 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.

There is some lore around EFI variable names being up to 1024 bytes in
size, but this has no basis in the UEFI specification, and the upper
bounds are typically platform specific, and apply to the entire variable
(name plus payload).

Given that Linux does not permit creating files with names longer than
NAME_MAX (255) bytes, 512 bytes (== 256 UTF-16 characters) is a
reasonable limit.

Cc: <stable@vger.kernel.org> # 6.1+
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
[timschumi@gmx.de: adjusted diff for changed context and code move]
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
---
Please apply this patch to stable kernel 5.15, 5.10, 5.4, and 4.19
respectively. Kernel 6.1 and upwards were already handled via CC,
5.15 and below required a separate patch due to a slight refactor of
surrounding code in bbc6d2c6ef22 ("efi: vars: Switch to new wrapper
layer") and a subsequent code move in 2d82e6227ea1 ("efi: vars: Move
efivar caching layer into efivarfs").

Please note that the upper Signed-off-by tags are remnants from the
original patch, I documented my modifications below them and added
another sign-off. As far as I was able to gather, this is the expected
format for diverged stable patches.

I'm not sure on the specifics of manual stable backports, so let me
know in case anything doesn't follow the process. The linux-efi team
and list are on CC both for documentation/review purposes and in case
a new sign-off/ack of theirs is required.
---
 drivers/firmware/efi/vars.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

--
2.44.0

Comments

Greg KH March 29, 2024, 1:13 p.m. UTC | #1 
On Sun, Mar 17, 2024 at 03:33:21AM +0100, Tim Schumacher wrote:
> commit f45812cc23fb74bef62d4eb8a69fe7218f4b9f2a upstream.
> 
> Work around 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.
> 
> There is some lore around EFI variable names being up to 1024 bytes in
> size, but this has no basis in the UEFI specification, and the upper
> bounds are typically platform specific, and apply to the entire variable
> (name plus payload).
> 
> Given that Linux does not permit creating files with names longer than
> NAME_MAX (255) bytes, 512 bytes (== 256 UTF-16 characters) is a
> reasonable limit.
> 
> Cc: <stable@vger.kernel.org> # 6.1+
> Signed-off-by: Tim Schumacher <timschumi@gmx.de>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> [timschumi@gmx.de: adjusted diff for changed context and code move]
> Signed-off-by: Tim Schumacher <timschumi@gmx.de>
> ---
> Please apply this patch to stable kernel 5.15, 5.10, 5.4, and 4.19
> respectively. Kernel 6.1 and upwards were already handled via CC,
> 5.15 and below required a separate patch due to a slight refactor of
> surrounding code in bbc6d2c6ef22 ("efi: vars: Switch to new wrapper
> layer") and a subsequent code move in 2d82e6227ea1 ("efi: vars: Move
> efivar caching layer into efivarfs").
> 
> Please note that the upper Signed-off-by tags are remnants from the
> original patch, I documented my modifications below them and added
> another sign-off. As far as I was able to gather, this is the expected
> format for diverged stable patches.
> 
> I'm not sure on the specifics of manual stable backports, so let me
> know in case anything doesn't follow the process. The linux-efi team
> and list are on CC both for documentation/review purposes and in case
> a new sign-off/ack of theirs is required.

Now queued up, thanks.

greg k-h
diff mbox series

Patch

diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index cae590bd08f2..eaed1ddcc803 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -415,7 +415,7 @@  int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
 		void *data, bool duplicates, struct list_head *head)
 {
 	const struct efivar_operations *ops;
-	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;
@@ -438,12 +438,13 @@  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.
+	 * 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 = ops->get_next_variable(&variable_name_size,
 						variable_name,
@@ -491,9 +492,13 @@  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:
+			pr_warn("efivars: Variable name size exceeds maximum (%lu > 512)\n",
+				variable_name_size);
+			status = EFI_NOT_FOUND;
+			break;
 		default:
-			printk(KERN_WARNING "efivars: get_next_variable: status=%lx\n",
-				status);
+			pr_warn("efivars: get_next_variable: status=%lx\n", status);
 			status = EFI_NOT_FOUND;
 			break;
 		}