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

[Ard Biesheuvel <ardb@kernel.org>]

Hi Tim,

On Fri, 15 Mar 2024 at 01:27, Tim Schumacher <timschumi@gmx.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@gmx.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 CC'd the pstore people and linux-hardening mailing list because
> get_maintainer.pl suggested to do so. Apologies in case this was the
> incorrect decision, this is a very non-pstore-specific patch after all.
>

If any of the linux-hardening/pstore people give you grief, just send
them to me :-)

(I am part of the linux-hardening group myself, and work closely with Kees)


> I have taken the liberty of adding a TODO for the future, the actual
> refactor can follow at some point down the line.
> ---
>  drivers/firmware/efi/efi-pstore.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
> index e7b9ec6f8a86..f0ceb5702d21 100644
> --- a/drivers/firmware/efi/efi-pstore.c
> +++ b/drivers/firmware/efi/efi-pstore.c
> @@ -135,7 +135,15 @@ static ssize_t efi_pstore_read(struct pstore_record *record)
>         efi_status_t status;
>
>         for (;;) {
> -               varname_size = 1024;
> +               /*
> +                * A small set of old UEFI implementations reject sizes
> +                * above a certain threshold, the lowest seen in the wild
> +                * is 512.
> +                *
> +                * TODO: Commonize with the iteration implementation in
> +                *       fs/efivarfs to keep all the quirks in one place.
> +                */
> +               varname_size = 512;
>
>                 /*
>                  * If this is the first read() call in the pstore enumeration,
> --
> 2.44.0
>