[PATCH] efi: vars: allow passing fmode= and dmode= when mounting

From: Jason A. Donenfeld
Date: Thu Nov 10 2022 - 10:46:27 EST


The default wide-open permissions of efivarfs make the prospect of
storing secrets there a bit sketchy. Currently systemd does this, and
then pid 1 takes care of chmodding particular nodes. But this is limited
and error-prone. Rather, allow passing an explicit dmode for directories
and fmode for files.

Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
Cc: Jeremy Kerr <jk@xxxxxxxxxx>
Cc: Matthew Garrett <matthew.garrett@xxxxxxxxxx>
Suggested-by: Lennart Poettering <lennart@xxxxxxxxxxxxxx>
Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
---
fs/efivarfs/internal.h | 5 ++++
fs/efivarfs/super.c | 60 ++++++++++++++++++++++++++++++++++++++----
2 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
index 8ebf3a6a8aa2..ea1ca322d247 100644
--- a/fs/efivarfs/internal.h
+++ b/fs/efivarfs/internal.h
@@ -24,6 +24,11 @@ struct efivar_entry {
struct kobject kobj;
};

+struct efivarfs_sb_info {
+ umode_t dmode;
+ umode_t fmode;
+};
+
int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
void *data, bool duplicates, struct list_head *head);

diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index 6780fc81cc11..45edbb1550e9 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -8,6 +8,7 @@
#include <linux/efi.h>
#include <linux/fs.h>
#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
#include <linux/module.h>
#include <linux/pagemap.h>
#include <linux/ucs2_string.h>
@@ -107,6 +108,7 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor,
unsigned long name_size, void *data)
{
struct super_block *sb = (struct super_block *)data;
+ struct efivarfs_sb_info *sbi = sb->s_fs_info;
struct efivar_entry *entry;
struct inode *inode = NULL;
struct dentry *dentry, *root = sb->s_root;
@@ -144,7 +146,7 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor,
/* replace invalid slashes like kobject_set_name_vargs does for /sys/firmware/efi/vars. */
strreplace(name, '/', '!');

- inode = efivarfs_get_inode(sb, d_inode(root), S_IFREG | 0644, 0,
+ inode = efivarfs_get_inode(sb, d_inode(root), S_IFREG | sbi->fmode, 0,
is_removable);
if (!inode)
goto fail_name;
@@ -187,6 +189,7 @@ static int efivarfs_destroy(struct efivar_entry *entry, void *data)

static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
{
+ struct efivarfs_sb_info *sbi = sb->s_fs_info;
struct inode *inode = NULL;
struct dentry *root;
int err;
@@ -202,7 +205,7 @@ static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
if (!efivar_supports_writes())
sb->s_flags |= SB_RDONLY;

- inode = efivarfs_get_inode(sb, NULL, S_IFDIR | 0755, 0, true);
+ inode = efivarfs_get_inode(sb, NULL, S_IFDIR | sbi->dmode, 0, true);
if (!inode)
return -ENOMEM;
inode->i_op = &efivarfs_dir_inode_operations;
@@ -226,12 +229,58 @@ static int efivarfs_get_tree(struct fs_context *fc)
return get_tree_single(fc, efivarfs_fill_super);
}

+enum {
+ Opt_dmode,
+ Opt_fmode,
+};
+
+static const struct fs_parameter_spec efivarfs_parameters[] = {
+ fsparam_u32oct("dmode", Opt_dmode),
+ fsparam_u32oct("fmode", Opt_fmode),
+ {}
+};
+
+static int efivarfs_parse_param(struct fs_context *fc,
+ struct fs_parameter *param)
+{
+ struct efivarfs_sb_info *sbi = fc->s_fs_info;
+ struct fs_parse_result result;
+ int opt;
+
+ opt = fs_parse(fc, efivarfs_parameters, param, &result);
+ if (opt < 0)
+ return opt;
+
+ switch (opt) {
+ case Opt_dmode:
+ sbi->dmode = result.uint_32;
+ break;
+ case Opt_fmode:
+ sbi->fmode = result.uint_32;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static const struct fs_context_operations efivarfs_context_ops = {
.get_tree = efivarfs_get_tree,
+ .parse_param = efivarfs_parse_param,
};

static int efivarfs_init_fs_context(struct fs_context *fc)
{
+ struct efivarfs_sb_info *sbi;
+
+ sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
+ if (!sbi)
+ return -ENOMEM;
+ sbi->dmode = 0755;
+ sbi->fmode = 0644;
+ fc->s_fs_info = sbi;
+
fc->ops = &efivarfs_context_ops;
return 0;
}
@@ -245,10 +294,11 @@ static void efivarfs_kill_sb(struct super_block *sb)
}

static struct file_system_type efivarfs_type = {
- .owner = THIS_MODULE,
- .name = "efivarfs",
+ .owner = THIS_MODULE,
+ .name = "efivarfs",
.init_fs_context = efivarfs_init_fs_context,
- .kill_sb = efivarfs_kill_sb,
+ .parameters = efivarfs_parameters,
+ .kill_sb = efivarfs_kill_sb,
};

static __init int efivarfs_init(void)
--
2.38.1