linux-hardening - Re: [PATCH 1/3] efi: pstore: Request at most 512 bytes for variable names

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: <43bd1a71-8b4a-4edc-827e-6845e173e82d@gmx.de>
Date: Fri, 15 Mar 2024 20:02:09 +0100
From: Tim Schumacher <timschumi@....de>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: linux-efi@...r.kernel.org, Kees Cook <keescook@...omium.org>,
 Tony Luck <tony.luck@...el.com>, "Guilherme G. Piccoli"
 <gpiccoli@...lia.com>, linux-hardening@...r.kernel.org
Subject: Re: [PATCH 1/3] efi: pstore: Request at most 512 bytes for variable
 names

On 15.03.24 10:16, Ard Biesheuvel wrote:
> Hi Tim,
>
> On Fri, 15 Mar 2024 at 01:27, Tim Schumacher <timschumi@....de> wrote:
>>
>> 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.
>>
>> This was already done to efivarfs in f45812cc23fb ("efivarfs: Request at
>> most 512 bytes for variable names"), but the second copy of the variable
>> iteration implementation was overlooked.
>>
>> Signed-off-by: Tim Schumacher <timschumi@....de>
>
> Thanks for the patch. I'll take it as a fix.
>
> As an aside, you really want to avoid EFI pstore in general, and
> specifically on such old systems with quirky UEFI implementations.
>
I found this by chance while looking for appearances of the magic value of
1024, and decided to split it out because this would have been the only change
that modifies behavior.

I didn't intend to actually use it after fixing it up, although I did make sure
that it now does more than it did previously. If we can save someone a confused
"Why is this done differently here?" (and have a reason to boil down the code to
a single implementation in the future), then that is probably good enough on its
own.