linux-kernel - Re: [PATCH v2] efi/efivars: Create efivars mount point via efivars abstraction

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

Date:   Wed, 26 Aug 2020 14:08:18 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     "Lee, Chun-Yi" <joeyli.kernel@...il.com>
Cc:     Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        linux-efi <linux-efi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Lee, Chun-Yi" <jlee@...e.com>,
        Matthias Brugger <mbrugger@...e.com>,
        Fabian Vogt <fvogt@...e.com>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>
Subject: Re: [PATCH v2] efi/efivars: Create efivars mount point via efivars abstraction

On Wed, 26 Aug 2020 at 02:46, Lee, Chun-Yi <joeyli.kernel@...il.com> wrote:
>
> This patch creates efivars mount point when active efivars abstraction
> be set. It is useful for userland to determine the availability of efivars
> filesystem.
>
> Cc: Matthias Brugger <mbrugger@...e.com>
> Cc: Fabian Vogt <fvogt@...e.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@...aro.org>
> Cc: Ard Biesheuvel <ardb@...nel.org>
> Signed-off-by: "Lee, Chun-Yi" <jlee@...e.com>

Apologies for not bringing this up before: while the patch seems fine,
I wonder if we really need this if the purpose is to decide whether
efivars is available or not. We already have the 'efivars' platform
device for this, and so userland can simply check for the existence of

/sys/devices/platform/efivars.0

and so we don't need to make any changes for this.



> ---
>
> v2:
> Using efivars_kobject() helper instead of checking GetVariable or
> GetNextVariable EFI runtime services. Because the efivarfs code could be
> instantiated using a different efivars abstraction.
>
>  drivers/firmware/efi/efi.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 3aa07c3b5136..db483fc68501 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -405,11 +405,13 @@ static int __init efisubsys_init(void)
>         if (error)
>                 goto err_remove_group;
>
> -       /* and the standard mountpoint for efivarfs */
> -       error = sysfs_create_mount_point(efi_kobj, "efivars");
> -       if (error) {
> -               pr_err("efivars: Subsystem registration failed.\n");
> -               goto err_remove_group;
> +       if (efivars_kobject()) {
> +               /* and the standard mountpoint for efivarfs */
> +               error = sysfs_create_mount_point(efi_kobj, "efivars");
> +               if (error) {
> +                       pr_err("efivars: Subsystem registration failed.\n");
> +                       goto err_remove_group;
> +               }
>         }
>
>         if (efi_enabled(EFI_DBG) && efi_enabled(EFI_PRESERVE_BS_REGIONS))
> --
> 2.16.4
>