All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] efivarfs: Cache occupied and available space
@ 2026-08-01 14:42 Ard Biesheuvel
  2026-08-04 10:02 ` Ard Biesheuvel
  2026-08-04 16:01 ` Anisse Astier
  0 siblings, 2 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2026-08-01 14:42 UTC (permalink / raw)
  To: linux-efi; +Cc: x86, Anisse Astier, Ard Biesheuvel, stable, Ravi Bangoria

Ravi reports that statfs() may be called by unprivileged users on the
efivarfs mount point, resulting in repeated calls to QueryVariableInfo()
which are disproportionately costly on x86 systems where the variable
store is backed by SMM, as each SMM entry requires a rendez-vous of all
the CPUs.

So cache the output of QueryVariableInfo(), and invalidate the cached
values when an entry is added or deleted, or the variable store is
synchronized after a suspend/resume sequence.

Cc: <stable@vger.kernel.org>
Reported-by: Ravi Bangoria <ravi.bangoria@amd.com>
Fixes: d86ff3333cb1 ("efivarfs: expose used and total size")
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 fs/efivarfs/internal.h |  2 ++
 fs/efivarfs/super.c    | 21 +++++++++++---------
 fs/efivarfs/vars.c     |  9 +++++++++
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
index f913b6824289..d43789bc7839 100644
--- a/fs/efivarfs/internal.h
+++ b/fs/efivarfs/internal.h
@@ -31,6 +31,8 @@ struct efivar_entry {
 	bool removed;
 };
 
+extern u64 efivar_storage_space, efivar_remaining_space;
+
 static inline struct efivar_entry *efivar_entry(struct inode *inode)
 {
 	return container_of(inode, struct efivar_entry, vfs_inode);
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index 733c19571f1c..3691a2a087a0 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -23,6 +23,8 @@
 #include "internal.h"
 #include "../internal.h"
 
+u64 efivar_storage_space, efivar_remaining_space;
+
 static int efivarfs_ops_notifier(struct notifier_block *nb, unsigned long event,
 				 void *data)
 {
@@ -82,15 +84,16 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	const u32 attr = EFI_VARIABLE_NON_VOLATILE |
 			 EFI_VARIABLE_BOOTSERVICE_ACCESS |
 			 EFI_VARIABLE_RUNTIME_ACCESS;
-	u64 storage_space, remaining_space, max_variable_size;
 	u64 id = huge_encode_dev(dentry->d_sb->s_dev);
+	u64 max_variable_size;
 	efi_status_t status;
 
 	/* Some UEFI firmware does not implement QueryVariableInfo() */
-	storage_space = remaining_space = 0;
-	if (efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)) {
-		status = efivar_query_variable_info(attr, &storage_space,
-						    &remaining_space,
+	if (efivar_storage_space == 0 &&
+	    efivar_remaining_space == 0 &&
+	    efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)) {
+		status = efivar_query_variable_info(attr, &efivar_storage_space,
+						    &efivar_remaining_space,
 						    &max_variable_size);
 		if (status != EFI_SUCCESS && status != EFI_UNSUPPORTED)
 			pr_warn_ratelimited("query_variable_info() failed: 0x%lx\n",
@@ -104,8 +107,8 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	 */
 	buf->f_bsize	= 1;
 	buf->f_namelen	= NAME_MAX;
-	buf->f_blocks	= storage_space;
-	buf->f_bfree	= remaining_space;
+	buf->f_blocks	= efivar_storage_space;
+	buf->f_bfree	= efivar_remaining_space;
 	buf->f_type	= dentry->d_sb->s_magic;
 	buf->f_fsid	= u64_to_fsid(id);
 
@@ -114,8 +117,8 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	 * when the storage_paranoia x86 quirk is active. To use more, users
 	 * should boot the kernel with efi_no_storage_paranoia.
 	 */
-	if (remaining_space > efivar_reserved_space())
-		buf->f_bavail = remaining_space - efivar_reserved_space();
+	if (efivar_remaining_space > efivar_reserved_space())
+		buf->f_bavail = efivar_remaining_space - efivar_reserved_space();
 	else
 		buf->f_bavail = 0;
 
diff --git a/fs/efivarfs/vars.c b/fs/efivarfs/vars.c
index 6833c3d24b54..4196963695bd 100644
--- a/fs/efivarfs/vars.c
+++ b/fs/efivarfs/vars.c
@@ -361,6 +361,11 @@ static void dup_variable_bug(efi_char16_t *str16, efi_guid_t *vendor_guid,
 	kfree(str8);
 }
 
+static void efivar_invalidate_cached_storage_space(void)
+{
+	efivar_storage_space = efivar_remaining_space = 0;
+}
+
 /**
  * efivar_init - build the initial list of EFI variables
  * @func: callback function to invoke for every variable
@@ -450,6 +455,7 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
 	} while (status != EFI_NOT_FOUND);
 
 	efivar_unlock();
+	efivar_invalidate_cached_storage_space();
 free:
 	kfree(variable_name);
 
@@ -480,6 +486,8 @@ int efivar_entry_delete(struct efivar_entry *entry)
 					    &entry->var.VendorGuid,
 					    0, 0, NULL, false);
 	efivar_unlock();
+	efivar_invalidate_cached_storage_space();
+
 	if (!(status == EFI_SUCCESS || status == EFI_NOT_FOUND))
 		return efi_status_to_err(status);
 
@@ -620,6 +628,7 @@ int efivar_entry_set_get_size(struct efivar_entry *entry, u32 attributes,
 				    NULL, size, NULL);
 
 	efivar_unlock();
+	efivar_invalidate_cached_storage_space();
 
 	if (status && status != EFI_BUFFER_TOO_SMALL)
 		return efi_status_to_err(status);
-- 
2.55.0.508.g3f0d502094-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] efivarfs: Cache occupied and available space
  2026-08-01 14:42 [PATCH] efivarfs: Cache occupied and available space Ard Biesheuvel
@ 2026-08-04 10:02 ` Ard Biesheuvel
  2026-08-04 16:01 ` Anisse Astier
  1 sibling, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2026-08-04 10:02 UTC (permalink / raw)
  To: linux-efi; +Cc: x86, Anisse Astier, stable, Ravi Bangoria



On Sat, 1 Aug 2026, at 17:42, Ard Biesheuvel wrote:
> Ravi reports that statfs() may be called by unprivileged users on the
> efivarfs mount point, resulting in repeated calls to QueryVariableInfo()
> which are disproportionately costly on x86 systems where the variable
> store is backed by SMM, as each SMM entry requires a rendez-vous of all
> the CPUs.
>
> So cache the output of QueryVariableInfo(), and invalidate the cached
> values when an entry is added or deleted, or the variable store is
> synchronized after a suspend/resume sequence.
>
> Cc: <stable@vger.kernel.org>
> Reported-by: Ravi Bangoria <ravi.bangoria@amd.com>
> Fixes: d86ff3333cb1 ("efivarfs: expose used and total size")
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  fs/efivarfs/internal.h |  2 ++
>  fs/efivarfs/super.c    | 21 +++++++++++---------
>  fs/efivarfs/vars.c     |  9 +++++++++
>  3 files changed, 23 insertions(+), 9 deletions(-)
>

Applied to efi/next

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] efivarfs: Cache occupied and available space
  2026-08-01 14:42 [PATCH] efivarfs: Cache occupied and available space Ard Biesheuvel
  2026-08-04 10:02 ` Ard Biesheuvel
@ 2026-08-04 16:01 ` Anisse Astier
  1 sibling, 0 replies; 3+ messages in thread
From: Anisse Astier @ 2026-08-04 16:01 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, x86, stable, Ravi Bangoria

Hi Ard,

Please find a few comments below,

On Sat, Aug 01, 2026 at 04:42:59PM +0200, Ard Biesheuvel wrote:
> Ravi reports that statfs() may be called by unprivileged users on the
> efivarfs mount point, resulting in repeated calls to QueryVariableInfo()
> which are disproportionately costly on x86 systems where the variable
> store is backed by SMM, as each SMM entry requires a rendez-vous of all
> the CPUs.
> 
> So cache the output of QueryVariableInfo(), and invalidate the cached
> values when an entry is added or deleted, or the variable store is
> synchronized after a suspend/resume sequence.
> 
> Cc: <stable@vger.kernel.org>
> Reported-by: Ravi Bangoria <ravi.bangoria@amd.com>
> Fixes: d86ff3333cb1 ("efivarfs: expose used and total size")
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  fs/efivarfs/internal.h |  2 ++
>  fs/efivarfs/super.c    | 21 +++++++++++---------
>  fs/efivarfs/vars.c     |  9 +++++++++
>  3 files changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
> index f913b6824289..d43789bc7839 100644
> --- a/fs/efivarfs/internal.h
> +++ b/fs/efivarfs/internal.h
> @@ -31,6 +31,8 @@ struct efivar_entry {
>  	bool removed;
>  };
>  
> +extern u64 efivar_storage_space, efivar_remaining_space;
> +
>  static inline struct efivar_entry *efivar_entry(struct inode *inode)
>  {
>  	return container_of(inode, struct efivar_entry, vfs_inode);
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index 733c19571f1c..3691a2a087a0 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -23,6 +23,8 @@
>  #include "internal.h"
>  #include "../internal.h"
>  
> +u64 efivar_storage_space, efivar_remaining_space;
> +
Since those are globals (before they were only local to the function),
I'd put them behind a lock now, probably efivar_lock, because we need
something that's globally available.

statfs could be raced from userspace vs the rest of the code; and
efivar_invalidate_cached_storage_space is only called outside the lock.

>  static int efivarfs_ops_notifier(struct notifier_block *nb, unsigned long event,
>  				 void *data)
>  {
> @@ -82,15 +84,16 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	const u32 attr = EFI_VARIABLE_NON_VOLATILE |
>  			 EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  			 EFI_VARIABLE_RUNTIME_ACCESS;
> -	u64 storage_space, remaining_space, max_variable_size;
>  	u64 id = huge_encode_dev(dentry->d_sb->s_dev);
> +	u64 max_variable_size;
>  	efi_status_t status;
>  
>  	/* Some UEFI firmware does not implement QueryVariableInfo() */
> -	storage_space = remaining_space = 0;
> -	if (efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)) {
> -		status = efivar_query_variable_info(attr, &storage_space,
> -						    &remaining_space,
> +	if (efivar_storage_space == 0 &&
> +	    efivar_remaining_space == 0 &&
> +	    efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)) {
> +		status = efivar_query_variable_info(attr, &efivar_storage_space,
> +						    &efivar_remaining_space,
>  						    &max_variable_size);
>  		if (status != EFI_SUCCESS && status != EFI_UNSUPPORTED)
>  			pr_warn_ratelimited("query_variable_info() failed: 0x%lx\n",

We've had issues with firmware in the past, shouldn't we be a bit more
defensive here in case of errors? I'd reset both variables on error in
case an incomplete write to one of the variables gives us an invalid state.

> @@ -104,8 +107,8 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	 */
>  	buf->f_bsize	= 1;
>  	buf->f_namelen	= NAME_MAX;
> -	buf->f_blocks	= storage_space;
> -	buf->f_bfree	= remaining_space;
> +	buf->f_blocks	= efivar_storage_space;
> +	buf->f_bfree	= efivar_remaining_space;
>  	buf->f_type	= dentry->d_sb->s_magic;
>  	buf->f_fsid	= u64_to_fsid(id);
>  
> @@ -114,8 +117,8 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	 * when the storage_paranoia x86 quirk is active. To use more, users
>  	 * should boot the kernel with efi_no_storage_paranoia.
>  	 */
> -	if (remaining_space > efivar_reserved_space())
> -		buf->f_bavail = remaining_space - efivar_reserved_space();
> +	if (efivar_remaining_space > efivar_reserved_space())
> +		buf->f_bavail = efivar_remaining_space - efivar_reserved_space();
>  	else
>  		buf->f_bavail = 0;
>  
> diff --git a/fs/efivarfs/vars.c b/fs/efivarfs/vars.c
> index 6833c3d24b54..4196963695bd 100644
> --- a/fs/efivarfs/vars.c
> +++ b/fs/efivarfs/vars.c
> @@ -361,6 +361,11 @@ static void dup_variable_bug(efi_char16_t *str16, efi_guid_t *vendor_guid,
>  	kfree(str8);
>  }
>  
> +static void efivar_invalidate_cached_storage_space(void)
> +{
> +	efivar_storage_space = efivar_remaining_space = 0;
> +}
> +
>  /**
>   * efivar_init - build the initial list of EFI variables
>   * @func: callback function to invoke for every variable
> @@ -450,6 +455,7 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
>  	} while (status != EFI_NOT_FOUND);
>  
>  	efivar_unlock();
> +	efivar_invalidate_cached_storage_space();
>  free:
>  	kfree(variable_name);
>  
> @@ -480,6 +486,8 @@ int efivar_entry_delete(struct efivar_entry *entry)
>  					    &entry->var.VendorGuid,
>  					    0, 0, NULL, false);
>  	efivar_unlock();
> +	efivar_invalidate_cached_storage_space();
> +
>  	if (!(status == EFI_SUCCESS || status == EFI_NOT_FOUND))
>  		return efi_status_to_err(status);
>  
> @@ -620,6 +628,7 @@ int efivar_entry_set_get_size(struct efivar_entry *entry, u32 attributes,
>  				    NULL, size, NULL);
>  
>  	efivar_unlock();
> +	efivar_invalidate_cached_storage_space();

In this function, the error path for efi_set_variable_locked (that could
have triggered an EFI_OUT_OF_RESOURCES) is not covered by the
invalidation.

>  
>  	if (status && status != EFI_BUFFER_TOO_SMALL)
>  		return efi_status_to_err(status);
> -- 

Couldn't the variables be written outside of efivarfs as well?
efivar_set_variable can be called from other code? pstore could maybe be
ignored if it's only used during crashes (I'm not sure), but I see at
least one other driver calling it as well. Wouldn't that change the
available/free size?

Regards,

Anisse

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-04 16:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-01 14:42 [PATCH] efivarfs: Cache occupied and available space Ard Biesheuvel
2026-08-04 10:02 ` Ard Biesheuvel
2026-08-04 16:01 ` Anisse Astier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.