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

Jonggeun Park posted 1 patch 3 weeks, 1 day ago
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(-)
[PATCH v2] efi: vars: commonize the 512-byte name buffer quirk
Posted by Jonggeun Park 3 weeks, 1 day ago
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;
+
 	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
Re: [PATCH v2] efi: vars: commonize the 512-byte name buffer quirk
Posted by Ard Biesheuvel 3 weeks, 1 day ago
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
Re: [PATCH v2] efi: vars: commonize the 512-byte name buffer quirk
Posted by David Laight 3 weeks, 1 day ago
On Thu, 03 Sep 2026 22:03:18 +0200
"Ard Biesheuvel" <ardb@kernel.org> wrote:

> 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);

You don't need the UL suffix.

> 
> 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  
>