| 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
| ||
|
Message-ID: <20200925005049.GD31226@linux-l9pv.suse>
Date: Fri, 25 Sep 2020 08:50:49 +0800
From: joeyli <jlee@...e.com>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: "Lee, Chun-Yi" <joeyli.kernel@...il.com>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
linux-efi <linux-efi@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Matthias Brugger <mbrugger@...e.com>,
Fabian Vogt <fvogt@...e.com>,
Ilias Apalodimas <ilias.apalodimas@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Arthur Heymans <arthur@...ymans.xyz>,
Patrick Rudolph <patrick.rudolph@...ements.com>
Subject: Re: [PATCH] efi/efivars: Create efivars mount point in the
registration of efivars abstraction
Hi Ard,
On Thu, Sep 24, 2020 at 12:47:46PM +0200, Ard Biesheuvel wrote:
> On Thu, 24 Sep 2020 at 10:28, Lee, Chun-Yi <joeyli.kernel@...il.com> wrote:
> >
> > This patch moved the logic of creating efivars mount point to the
> > registration of efivars abstraction. It's useful for userland to
> > determine the availability of efivars filesystem by checking the
> > existence of mount point.
> >
> > The 'efivars' platform device be created on generic EFI runtime services
> > platform, so it can be used to determine the availability of efivarfs.
> > But this approach is not available for google gsmi efivars abstraction.
> >
> > This patch be tested on Here on qemu-OVMF and qemu-uboot.
> >
> > Cc: Ard Biesheuvel <ardb@...nel.org>
> > Cc: Matthias Brugger <mbrugger@...e.com>
> > Cc: Fabian Vogt <fvogt@...e.com>
> > Cc: Ilias Apalodimas <ilias.apalodimas@...aro.org>
> > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > Cc: Arthur Heymans <arthur@...ymans.xyz>
> > Cc: Patrick Rudolph <patrick.rudolph@...ements.com>
> > Signed-off-by: "Lee, Chun-Yi" <jlee@...e.com>
> > ---
>
> I take it this is v3 of [0]? If so, please explain how it deviates
> from v2. If it doesn't deviate from v2, it is better to continue the
> discussion in the other thread.
>
> For the sake of discussion, it helps to clarify the confusing nomenclature:
>
> a) 'efivars abstraction' - an internal kernel API that exposes EFI
> variables, and can potentially be backed by an implementation that is
> not EFI based (i.e., Google gsmi)
>
> b) efivars.ko module, built on top of the efivars abstraction, which
> exposes EFI variables (real ones or gsmi ones) via the deprecated
> sysfs interface
>
> c) efivarfs filesystem, also built on top of the efivars abstraction,
> which exposes EFI variables (real ones or gsmi ones) via a special
> filesystem independently of sysfs.
>
> Of course, the sysfs mount point we create for efivarfs is not called
> 'efivarfs' but 'efivars'. The sysfs subdirectory we create for
> efivars.ko is called 'vars'. Sigh.
>
Thanks for your clarification. It's useful to me!
>
> In this patch, you create the mount point for c) based on whether a)
> gets registered (which occurs on systems with EFI Get/SetVariable
> support or GSMI), right? So, to Greg's point, wouldn't it be easier to
> simply check whether efivarfs is listed in /proc/filesystems?
>
Yes, I think that Greg's suggestion is good enough for a userland tool
to detect the availability of efivarfs. You can ignore my patch.
Thanks for your help!
Joey Lee
> It also helps if you could clarify what the actual use case is, rather
> than saying that it is generally useful.
>
>
>
>
>
> [0] https://lore.kernel.org/linux-efi/20200825160719.7188-1-jlee@suse.com/
>
> > drivers/firmware/efi/efi.c | 7 -------
> > drivers/firmware/efi/vars.c | 17 +++++++++++++++++
> > 2 files changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > index 3aa07c3b5136..23c11a2a3f4d 100644
> > --- a/drivers/firmware/efi/efi.c
> > +++ b/drivers/firmware/efi/efi.c
> > @@ -405,13 +405,6 @@ 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 (efi_enabled(EFI_DBG) && efi_enabled(EFI_PRESERVE_BS_REGIONS))
> > efi_debugfs_init();
> >
> > diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
> > index 973eef234b36..6fa7f288d635 100644
> > --- a/drivers/firmware/efi/vars.c
> > +++ b/drivers/firmware/efi/vars.c
> > @@ -1179,6 +1179,8 @@ int efivars_register(struct efivars *efivars,
> > const struct efivar_operations *ops,
> > struct kobject *kobject)
> > {
> > + int error;
> > +
> > if (down_interruptible(&efivars_lock))
> > return -EINTR;
> >
> > @@ -1191,6 +1193,19 @@ int efivars_register(struct efivars *efivars,
> >
> > up(&efivars_lock);
> >
> > + /* and the standard mountpoint for efivarfs */
> > + if (efi_kobj) {
> > + error = sysfs_create_mount_point(efi_kobj, "efivars");
> > + if (error) {
> > + if (down_interruptible(&efivars_lock))
> > + return -EINTR;
> > + __efivars = NULL;
> > + up(&efivars_lock);
> > + pr_err("efivars: Subsystem registration failed.\n");
> > + return error;
> > + }
> > + }
> > +
> > return 0;
> > }
> > EXPORT_SYMBOL_GPL(efivars_register);
> > @@ -1222,6 +1237,8 @@ int efivars_unregister(struct efivars *efivars)
> >
> > pr_info("Unregistered efivars operations\n");
> > __efivars = NULL;
> > + if (efi_kobj)
> > + sysfs_remove_mount_point(efi_kobj, "efivars");
> >
> > rv = 0;
> > out:
> > --
> > 2.16.4
> >
Powered by blists - more mailing lists