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: <b58a112f-767f-4918-8262-63ac1dbfebbf@gmx.de>
Date: Tue, 23 Jan 2024 18:33:00 +0100
From: Tim Schumacher <timschumi@....de>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: Matthew Garrett <mjg59@...f.ucam.org>, Peter Jones <pjones@...hat.com>,
 linux-efi@...r.kernel.org, Jeremy Kerr <jk@...abs.org>,
 linux-kernel@...r.kernel.org, timschumi@....de
Subject: Re: [PATCH] efivarfs: Iterate variables with increasing name buffer
 sizes

On 23.01.24 15:09, Ard Biesheuvel wrote:
> On Tue, 23 Jan 2024 at 14:55, Tim Schumacher <timschumi@....de> wrote:
>>
>> I'd rather avoid introducing deviations from the specifications on the
>> kernel side as well.
>
> Which specification would this deviate from?

The preexisting comment claims "Per EFI spec", and it appears that I got
mislead by that. Neither the UEFI specification, nor the newest revision
of the EFI specification (which I guess is what would have been current
back in 2004, when this comment was introduced) seem to make any mention
of a maximum length for the variable name.

I'll look into updating it appropriately when doing my changes (or remove
it entirely, since it seems to serve no other purpose than justifying the
starting buffer size).

>> 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.
>
> Yes, I would prefer to keep this as simple as possible.

I'll prepare a v2 with those changes then. The 1024 bytes may not be
an actual limit, but I'll keep it as the default size for the second
revision to ensure that we don't break any existing setups.

If this is still considered not simple enough, we can go back to looking
at just doing s/1024/512/ for the static buffer size.

Thank you for the feedback, I greatly appreciate it.

Tim

PS: Apologies in case my previous message ended up with a messed up line
wrap. Thunderbird apparently shows the message with automatic line wrap
while composing, but doesn't actually send it like that (or lines are
magically unwrapped again when displaying the message afterwards).