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

From: Tim Schumacher
Date: Mon Jan 22 2024 - 18:15:29 EST


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@xxxxxxxxxxxxxxx # 6.1+
Signed-off-by: Tim Schumacher <timschumi@xxxxxx>
---
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