On Sun, 10 Sept 2023 at 06:53, Heinrich Schuchardt <heinrich.schuchardt@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> > --- > v2: > initialize remaining_space to 0 Thanks for the patch, and apologies for the oversight. > --- > 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; Shouldn't storage_space be set to 0 too? > 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; > + } > + } > + I'd prefer not to ignore the error completely here. How about we do --- a/fs/efivarfs/super.c +++ b/fs/efivarfs/super.c @@ -32,10 +32,15 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf) u64 storage_space, remaining_space, 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); + /* 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, + &max_variable_size); + if (status != EFI_SUCCESS && status != EFI_UNSUPPORTED) + return efi_status_to_err(status); + } /* * This is not a normal filesystem, so no point in pretending it has a block (no need to resend if you agree)