Re: [PATCH v2] efi: vars: commonize the 512-byte name buffer quirk

["Ard Biesheuvel" <ardb@kernel.org>]

Thanks for respinning this.

Some additional thoughts below.

On Thu, 3 Sep 2026, at 13:55, Jonggeun Park wrote:
> Some old UEFI implementations reject GetNextVariableName() calls with
> a name buffer size larger than 512 bytes. Both efivar_init() and
> efi_pstore_read() open-code the same workaround of resetting the size
> to 512 on every iteration.
>
> Move this workaround into efivar_get_next_variable(), which keeps this
> quirk in one place, resolving the TODO in efi-pstore.c.
>
> No functional change intended.
>
> Signed-off-by: Jonggeun Park <jakejgpark@gmail.com>
> ---
> v2:
> - Move the workaround into the existing efivar_get_next_variable().
> ---
>  drivers/firmware/efi/efi-pstore.c | 10 ----------
>  drivers/firmware/efi/vars.c       |  7 +++++++
>  fs/efivarfs/vars.c                | 12 +-----------
>  3 files changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/firmware/efi/efi-pstore.c 
> b/drivers/firmware/efi/efi-pstore.c
> index a5db3534f..4f2594d08 100644
> --- a/drivers/firmware/efi/efi-pstore.c
> +++ b/drivers/firmware/efi/efi-pstore.c
> @@ -164,16 +164,6 @@ static ssize_t efi_pstore_read(struct 
> pstore_record *record)
>  	efi_status_t status;
> 
>  	for (;;) {
> -		/*
> -		 * 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,
>  		 * varname will be the empty string, and the GetNextVariable()
> diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
> index 3700e9869..0f95fc60d 100644
> --- a/drivers/firmware/efi/vars.c
> +++ b/drivers/firmware/efi/vars.c
> @@ -191,11 +191,18 @@ EXPORT_SYMBOL_NS_GPL(efivar_get_variable, 
> "EFIVAR");
>  /*
>   * efivar_get_next_variable() - enumerate the next name/vendor pair
>   *
> + * A small set of old UEFI implementations reject sizes above a certain
> + * threshold, the lowest seen in the wild is 512. Set the name buffer 
> size
> + * to 512 on each call.
> + *
>   * Must be called with efivars_lock held.
>   */
>  efi_status_t efivar_get_next_variable(unsigned long *name_size,
>  				      efi_char16_t *name, efi_guid_t *vendor)
>  {
> +	BUILD_BUG_ON(EFI_VAR_NAME_LEN < 512);
> +	*name_size = 512;
> +

This should really be

  *name_size = min(*name_size, 512UL);

so that a smaller buffer size provided by the caller is respected.

That also removes the need for the BUILD_BUG_ON().

No need to send a v3, I can fix that up when applying.

>  	return __efivars->ops->get_next_variable(name_size, name, vendor);
>  }
>  EXPORT_SYMBOL_NS_GPL(efivar_get_next_variable, "EFIVAR");
> diff --git a/fs/efivarfs/vars.c b/fs/efivarfs/vars.c
> index 6833c3d24..5a01833a6 100644
> --- a/fs/efivarfs/vars.c
> +++ b/fs/efivarfs/vars.c
> @@ -391,18 +391,8 @@ int efivar_init(int (*func)(efi_char16_t *, 
> efi_guid_t, unsigned long, void *),
>  	if (err)
>  		goto free;
> 
> -	/*
> -	 * A small set of old UEFI implementations reject sizes
> -	 * above a certain threshold, the lowest seen in the wild
> -	 * is 512.
> -	 */
> -
>  	do {
> -		variable_name_size = 512;
> -		BUILD_BUG_ON(EFI_VAR_NAME_LEN < 512);
> -
> -		status = efivar_get_next_variable(&variable_name_size,
> -						  variable_name,
> +		status = efivar_get_next_variable(&variable_name_size, variable_name,
>  						  &vendor_guid);
>  		switch (status) {
>  		case EFI_SUCCESS:
> -- 
> 2.43.0