linux-kernel - Re: [PATCH] efivarfs: Iterate variables with increasing name buffer sizes

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: <223a4e75-2e06-4bc6-a70c-823f0eb308d7@gmx.de>
Date: Tue, 23 Jan 2024 14:55:27 +0100
From: Tim Schumacher <timschumi@....de>
To: Ard Biesheuvel <ardb@...nel.org>, Matthew Garrett <mjg59@...f.ucam.org>,
 Peter Jones <pjones@...hat.com>
Cc: linux-efi@...r.kernel.org, Jeremy Kerr <jk@...abs.org>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] efivarfs: Iterate variables with increasing name buffer
 sizes

On 23.01.24 12:24, Ard Biesheuvel wrote:
> On Tue, 23 Jan 2024 at 00:15, Tim Schumacher <timschumi@....de> wrote:
>>
>> 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`.
>
> I wonder if we might just reduce this to 512 and be done with it.
> Presumably, Windows boots fine in UEFI mode on these machines, which
> suggests that it passes a value <= 512 too, and I don't recall ever
> encountering systems using extremely long variable names (i.e., longer
> than 512 byte)

I'd rather avoid introducing deviations from the specifications on the
kernel side as well. Someone or something might legitimately set a large
variable name, so we'd have to have resize logic anyways (to resize from
512 to 512+). Also, as mentioned on the patch, I'm entirely unsure what
the size ends up being used for, so I'd rather err on the side of
caution (most importantly in regards to the buffer size).

Windows _does_ boot fine (and is able to read all the variables), so
they presumably start off with 512 or smaller. FreeBSD definitely starts
from 512, but they also implement additional resize logic.

In regards to complexity of the proposed solution, how about we approach
this from the other side? Start off with advertising 1024 bytes of
buffer storage, and cut that value in half (without actually resizing
the buffer) as long as we get `EFI_INVALID_PARAMETER` while on the first
run.

If we ever get `EFI_BUFFER_TOO_SMALL`, we know that something is wrong
with the UEFI implementation (because that either means that something
claims to be larger than 1024 bytes, or that our assumptions about the
quirk don't hold up) and can bail out and log as appropriate. That would
limit the complexity to the machines that need it, completely omit the
need for resize logic, and would still be specification compliant.