Re: [PATCH v2 1/1] efivarfs: fix statfs() on efivarfs

On 2023-09-10 00:54, Heinrich Schuchardt wrote:
> Some firmware (notably U-Boot) provides GetVariable() and
> GetNextVariableName() but not QueryVariableInfo().
> 
> With commit d86ff3333cb1 ("efivarfs: expose used and total size") the
> statfs syscall was broken for such firmware.
> 
> If QueryVariableInfo() does not exist or returns an error, just report 
> the
> file-system size as 0 as statfs_simple() previously did.
> 
> Fixes: d86ff3333cb1 ("efivarfs: expose used and total size")
> Signed-off-by: Heinrich Schuchardt <[email protected]>
> ---
> v2:
> 	initialize remaining_space to 0
> ---
>  fs/efivarfs/super.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index e028fafa04f3..3893aae6a9be 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -29,14 +29,9 @@ 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 storage_space, remaining_space = 0, max_variable_size;
>  	efi_status_t status;
> 
> -	status = efivar_query_variable_info(attr, &storage_space, 
> &remaining_space,
> -					    &max_variable_size);
> -	if (status != EFI_SUCCESS)
> -		return efi_status_to_err(status);
> -
>  	/*
>  	 * This is not a normal filesystem, so no point in pretending it has 
> a block
>  	 * size; we declare f_bsize to 1, so that we can then report the 
> exact value
> @@ -44,10 +39,19 @@ 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_type	= dentry->d_sb->s_magic;
> 
> +	/* Some UEFI firmware does not implement QueryVariable() */
> +	if (efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)) 
> {
> +		status = efivar_query_variable_info(attr, &storage_space,
> +						    &remaining_space,
> +						    &max_variable_size);
> +		if (status == EFI_SUCCESS) {
> +			buf->f_blocks	= storage_space;
> +			buf->f_bfree	= remaining_space;
> +		}
> +	}
> +
>  	/*
>  	 * In f_bavail we declare the free space that the kernel will allow 
> writing
>  	 * when the storage_paranoia x86 quirk is active. To use more, users

FYI, this issue/fix affects ia64 EFI implementation as well, so adding a 
CC.