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-next>] [day] [month] [year] [list]

Message-ID: <1bc137b6-6006-42cd-9f6d-c523fc753d63@canonical.com>
Date:   Sun, 10 Sep 2023 22:43:50 +0200
From:   Heinrich Schuchardt <heinrich.schuchardt@...onical.com>
To:     Anisse Astier <anisse@...ier.eu>
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,
        Christian Brauner <brauner@...nel.org>
Subject: Re: [PATCH v2 1/1] efivarfs: fix statfs() on efivarfs

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@mail.gmail.com/

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