diff mbox series

[2/4] efivarfs: always register filesystem

Message ID 20230119164255.28091-3-johan+linaro@kernel.org
State New
Headers show
Series efi: verify that variable services are supported | expand

Commit Message

Johan Hovold Jan. 19, 2023, 4:42 p.m. UTC
The efivar ops are typically registered at subsys init time so that
they are available when efivarfs is registered at module init time.

Other efivars implementations, such as Google SMI, exists and can
currently be build as modules which means that efivar may not be
available when efivarfs is initialised.

Move the efivar availability check from module init to when the
filesystem is mounted to allow late registration of efivars.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 fs/efivarfs/super.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Ard Biesheuvel Jan. 20, 2023, 9:23 a.m. UTC | #1
(cc Peter, Heinrich)

On Thu, 19 Jan 2023 at 17:45, Johan Hovold <johan+linaro@kernel.org> wrote:
>
> The efivar ops are typically registered at subsys init time so that
> they are available when efivarfs is registered at module init time.
>
> Other efivars implementations, such as Google SMI, exists and can
> currently be build as modules which means that efivar may not be
> available when efivarfs is initialised.
>
> Move the efivar availability check from module init to when the
> filesystem is mounted to allow late registration of efivars.
>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

I think this change is fine in principle, but I 'm not sure if there
is user space code that the distros are carrying that might get
confused by this: beforehand, efivarfs would not exist in
/proc/filesystems and now, it will but trying to mount it might fail.


> ---
>  fs/efivarfs/super.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index f72c529c8ec3..b67d431c861a 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -194,6 +194,9 @@ static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
>         struct dentry *root;
>         int err;
>
> +       if (!efivar_is_available())
> +               return -EOPNOTSUPP;
> +
>         sb->s_maxbytes          = MAX_LFS_FILESIZE;
>         sb->s_blocksize         = PAGE_SIZE;
>         sb->s_blocksize_bits    = PAGE_SHIFT;
> @@ -256,9 +259,6 @@ static struct file_system_type efivarfs_type = {
>
>  static __init int efivarfs_init(void)
>  {
> -       if (!efivar_is_available())
> -               return -ENODEV;
> -
>         return register_filesystem(&efivarfs_type);
>  }
>
> --
> 2.38.2
>
Ard Biesheuvel Jan. 23, 2023, 11:32 a.m. UTC | #2
On Fri, 20 Jan 2023 at 17:04, Johan Hovold <johan@kernel.org> wrote:
>
> On Fri, Jan 20, 2023 at 10:23:18AM +0100, Ard Biesheuvel wrote:
> > (cc Peter, Heinrich)
> >
> > On Thu, 19 Jan 2023 at 17:45, Johan Hovold <johan+linaro@kernel.org> wrote:
> > >
> > > The efivar ops are typically registered at subsys init time so that
> > > they are available when efivarfs is registered at module init time.
> > >
> > > Other efivars implementations, such as Google SMI, exists and can
> > > currently be build as modules which means that efivar may not be
> > > available when efivarfs is initialised.
> > >
> > > Move the efivar availability check from module init to when the
> > > filesystem is mounted to allow late registration of efivars.
> > >
> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> >
> > I think this change is fine in principle, but I 'm not sure if there
> > is user space code that the distros are carrying that might get
> > confused by this: beforehand, efivarfs would not exist in
> > /proc/filesystems and now, it will but trying to mount it might fail.
>
> User space must already handle mount failing since commit 483028edacab
> ("efivars: respect EFI_UNSUPPORTED return from firmware") so that should
> not be an issue.
>

Fair enough
diff mbox series

Patch

diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index f72c529c8ec3..b67d431c861a 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -194,6 +194,9 @@  static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	struct dentry *root;
 	int err;
 
+	if (!efivar_is_available())
+		return -EOPNOTSUPP;
+
 	sb->s_maxbytes          = MAX_LFS_FILESIZE;
 	sb->s_blocksize         = PAGE_SIZE;
 	sb->s_blocksize_bits    = PAGE_SHIFT;
@@ -256,9 +259,6 @@  static struct file_system_type efivarfs_type = {
 
 static __init int efivarfs_init(void)
 {
-	if (!efivar_is_available())
-		return -ENODEV;
-
 	return register_filesystem(&efivarfs_type);
 }