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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9dec892136b296cf680a79fe71a602de@matoro.tk>
Date:   Fri, 15 Sep 2023 00:43:55 -0400
From:   matoro <matoro_mailinglist_kernel@...oro.tk>
To:     Heinrich Schuchardt <heinrich.schuchardt@...onical.com>
Cc:     Jeremy Kerr <jk@...abs.org>, Ard Biesheuvel <ardb@...nel.org>,
        Anisse Astier <an.astier@...teo.com>,
        linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
        Linux Ia64 <linux-ia64@...r.kernel.org>
Subject: 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 <heinrich.schuchardt@...onical.com>
> ---
> 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.