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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/10/23 20:53, Anisse Astier wrote:
Hi Heinrich,

On Sun, Sep 10, 2023 at 06:54:45AM +0200, Heinrich Schuchardt wrote:
Some firmware (notably U-Boot) provides GetVariable() and
GetNextVariableName() but not QueryVariableInfo().

 From a quick search, it seems u-boot, does support QueryVariableInfo, is
it on a given version ?

https://elixir.bootlin.com/u-boot/v2023.07.02/source/lib/efi_loader/efi_variable.c#L391

QueryVariableInfo() and SetVariable() are available before ExitBootServices(), i.e. in Linux' EFI stub.

ExitBootServices() results in calling efi_variables_boot_exit_notify() which disables these services during the UEFI runtime.



With commit d86ff3333cb1 ("efivarfs: expose used and total size") the
statfs syscall was broken for such firmware.

Could you be more specific ? What breaks, and what regressed ? I imagine
it could be some scripts running df, but maybe you had something else in
mind ?

Some more details can be found in https://bugs.launchpad.net/ubuntu/+source/linux-meta-riscv/+bug/2034705.

Though EFI variables are exposed via GetVariable() and GetNextVariableName() the efivar command refuses to display variables when statfs() reports an error.



If QueryVariableInfo() does not exist or returns an error, just report the
file-system size as 0 as statfs_simple() previously did.

I considered doing this [2] , but we settled on returning an error
instead for clarity:
https://lore.kernel.org/linux-efi/20230515-vorgaben-portrait-bb1b4255d31a@brauner/

I still think it would be a good idea if necessary.

We should never break user APIs.


On the approach, I prefer what Ard proposed, to fall back to the old
approach. I think the difference in block size could also be a good
marker that something wrong is happening:
https://lore.kernel.org/linux-efi/CAMj1kXEkNSoqG4zWfCZ8Ytte5b2SzwXggZp21Xt17Pszd-q0dg@xxxxxxxxxxxxxx/

This will allow user code making assumptions based on block size:
If block size > 1, assume setting variables is possible.

We should really avoid this.

Best regards

Heinrich




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux