fs/efivarfs/inode.c | 4 +++ fs/efivarfs/internal.h | 9 +++++ fs/efivarfs/super.c | 75 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 85 insertions(+), 3 deletions(-)
Allow UEFI variables to be modified by non-root processes
in order to run sandboxed code. This doesn't change the behavior
of mounting efivarfs unless uid/gid are specified;
by default both are set to root.
Signed-off-by: Jiao Zhou <jiaozhou@google.com>
Acked-by: Matthew Garrett <mgarrett@aurora.tech>
---
Changelog since v1:
- Add missing sentinel entry in fs_parameter_spec[] array.
- Fix a NULL pointer dereference.
Changelog since v2:
- Format the patch description.
Changelog since v3:
- Use sizeof(*sfi) to allocate memory to avoids future problems if sfi ever changes type.
- Add gid and uid check to make sure that ids are valid.
- Drop the indentation for one block.
fs/efivarfs/inode.c | 4 +++
fs/efivarfs/internal.h | 9 +++++
fs/efivarfs/super.c | 75 ++++++++++++++++++++++++++++++++++++++++--
3 files changed, 85 insertions(+), 3 deletions(-)
diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
index db9231f0e77b..06dfc73fda04 100644
--- a/fs/efivarfs/inode.c
+++ b/fs/efivarfs/inode.c
@@ -20,9 +20,13 @@ struct inode *efivarfs_get_inode(struct super_block *sb,
const struct inode *dir, int mode,
dev_t dev, bool is_removable)
{
+ struct efivarfs_fs_info *fsi = sb->s_fs_info;
struct inode *inode = new_inode(sb);
+ struct efivarfs_mount_opts *opts = &fsi->mount_opts;
if (inode) {
+ inode->i_uid = opts->uid;
+ inode->i_gid = opts->gid;
inode->i_ino = get_next_ino();
inode->i_mode = mode;
inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
index 8ebf3a6a8aa2..c66647f5c0bd 100644
--- a/fs/efivarfs/internal.h
+++ b/fs/efivarfs/internal.h
@@ -9,6 +9,15 @@
#include <linux/list.h>
#include <linux/efi.h>
+struct efivarfs_mount_opts {
+ kuid_t uid;
+ kgid_t gid;
+};
+
+struct efivarfs_fs_info {
+ struct efivarfs_mount_opts mount_opts;
+};
+
struct efi_variable {
efi_char16_t VariableName[EFI_VAR_NAME_LEN/sizeof(efi_char16_t)];
efi_guid_t VendorGuid;
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index e028fafa04f3..c53cebf45ac5 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>
@@ -24,6 +25,22 @@ static void efivarfs_evict_inode(struct inode *inode)
clear_inode(inode);
}
+static int efivarfs_show_options(struct seq_file *m, struct dentry *root)
+{
+ struct super_block *sb = root->d_sb;
+ struct efivarfs_fs_info *sbi = sb->s_fs_info;
+ struct efivarfs_mount_opts *opts = &sbi->mount_opts;
+
+ /* Show partition info */
+ if (!uid_eq(opts->uid, GLOBAL_ROOT_UID))
+ seq_printf(m, ",uid=%u",
+ from_kuid_munged(&init_user_ns, opts->uid));
+ if (!gid_eq(opts->gid, GLOBAL_ROOT_GID))
+ seq_printf(m, ",gid=%u",
+ from_kgid_munged(&init_user_ns, opts->gid));
+ return 0;
+}
+
static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
{
const u32 attr = EFI_VARIABLE_NON_VOLATILE |
@@ -64,6 +81,7 @@ static const struct super_operations efivarfs_ops = {
.statfs = efivarfs_statfs,
.drop_inode = generic_delete_inode,
.evict_inode = efivarfs_evict_inode,
+ .show_options = efivarfs_show_options,
};
/*
@@ -225,6 +243,45 @@ static int efivarfs_destroy(struct efivar_entry *entry, void *data)
return 0;
}
+enum {
+ Opt_uid, Opt_gid,
+};
+
+static const struct fs_parameter_spec efivarfs_parameters[] = {
+ fsparam_u32("uid", Opt_uid),
+ fsparam_u32("gid", Opt_gid),
+ {},
+};
+
+static int efivarfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
+{
+ struct efivarfs_fs_info *sbi = fc->s_fs_info;
+ struct efivarfs_mount_opts *opts = &sbi->mount_opts;
+ struct fs_parse_result result;
+ int opt;
+
+ opt = fs_parse(fc, efivarfs_parameters, param, &result);
+ if (opt < 0)
+ return opt;
+
+ switch (opt) {
+ case Opt_uid:
+ opts->uid = make_kuid(current_user_ns(), result.uint_32);
+ if (!uid_valid(opts->uid))
+ return -EINVAL;
+ break;
+ case Opt_gid:
+ opts->gid = make_kgid(current_user_ns(), result.uint_32);
+ if (!gid_valid(opts->gid))
+ return -EINVAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
{
struct inode *inode = NULL;
@@ -270,11 +327,22 @@ static int efivarfs_get_tree(struct fs_context *fc)
}
static const struct fs_context_operations efivarfs_context_ops = {
- .get_tree = efivarfs_get_tree,
+ .get_tree = efivarfs_get_tree,
+ .parse_param = efivarfs_parse_param,
};
static int efivarfs_init_fs_context(struct fs_context *fc)
{
+ struct efivarfs_fs_info *sfi;
+
+ sfi = kzalloc(sizeof(struct efivarfs_fs_info), GFP_KERNEL);
+ if (!sizeof(*sfi))
+ return -ENOMEM;
+
+ sfi->mount_opts.uid = GLOBAL_ROOT_UID;
+ sfi->mount_opts.gid = GLOBAL_ROOT_GID;
+
+ fc->s_fs_info = sfi;
fc->ops = &efivarfs_context_ops;
return 0;
}
@@ -291,10 +359,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,
};
static __init int efivarfs_init(void)
--
2.42.0.283.g2d96d420d3-goog(cc Christian)
Hi,
On Thu, 7 Sept 2023 at 18:43, Jiao Zhou <jiaozhou@google.com> wrote:
>
> Allow UEFI variables to be modified by non-root processes
> in order to run sandboxed code. This doesn't change the behavior
> of mounting efivarfs unless uid/gid are specified;
> by default both are set to root.
>
> Signed-off-by: Jiao Zhou <jiaozhou@google.com>
> Acked-by: Matthew Garrett <mgarrett@aurora.tech>
> ---
> Changelog since v1:
> - Add missing sentinel entry in fs_parameter_spec[] array.
> - Fix a NULL pointer dereference.
>
> Changelog since v2:
> - Format the patch description.
>
> Changelog since v3:
> - Use sizeof(*sfi) to allocate memory to avoids future problems if sfi ever changes type.
> - Add gid and uid check to make sure that ids are valid.
> - Drop the indentation for one block.
>
> fs/efivarfs/inode.c | 4 +++
> fs/efivarfs/internal.h | 9 +++++
> fs/efivarfs/super.c | 75 ++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 85 insertions(+), 3 deletions(-)
>
The commit log looks fine now - thanks for taking the time to dive
into the docs.
> diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
> index db9231f0e77b..06dfc73fda04 100644
> --- a/fs/efivarfs/inode.c
> +++ b/fs/efivarfs/inode.c
> @@ -20,9 +20,13 @@ struct inode *efivarfs_get_inode(struct super_block *sb,
> const struct inode *dir, int mode,
> dev_t dev, bool is_removable)
> {
> + struct efivarfs_fs_info *fsi = sb->s_fs_info;
> struct inode *inode = new_inode(sb);
> + struct efivarfs_mount_opts *opts = &fsi->mount_opts;
>
> if (inode) {
> + inode->i_uid = opts->uid;
> + inode->i_gid = opts->gid;
> inode->i_ino = get_next_ino();
> inode->i_mode = mode;
> inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
> diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
> index 8ebf3a6a8aa2..c66647f5c0bd 100644
> --- a/fs/efivarfs/internal.h
> +++ b/fs/efivarfs/internal.h
> @@ -9,6 +9,15 @@
> #include <linux/list.h>
> #include <linux/efi.h>
>
> +struct efivarfs_mount_opts {
> + kuid_t uid;
> + kgid_t gid;
> +};
> +
> +struct efivarfs_fs_info {
> + struct efivarfs_mount_opts mount_opts;
> +};
> +
> struct efi_variable {
> efi_char16_t VariableName[EFI_VAR_NAME_LEN/sizeof(efi_char16_t)];
> efi_guid_t VendorGuid;
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index e028fafa04f3..c53cebf45ac5 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>
> @@ -24,6 +25,22 @@ static void efivarfs_evict_inode(struct inode *inode)
> clear_inode(inode);
> }
>
> +static int efivarfs_show_options(struct seq_file *m, struct dentry *root)
> +{
> + struct super_block *sb = root->d_sb;
> + struct efivarfs_fs_info *sbi = sb->s_fs_info;
> + struct efivarfs_mount_opts *opts = &sbi->mount_opts;
> +
> + /* Show partition info */
> + if (!uid_eq(opts->uid, GLOBAL_ROOT_UID))
> + seq_printf(m, ",uid=%u",
> + from_kuid_munged(&init_user_ns, opts->uid));
> + if (!gid_eq(opts->gid, GLOBAL_ROOT_GID))
> + seq_printf(m, ",gid=%u",
> + from_kgid_munged(&init_user_ns, opts->gid));
> + return 0;
> +}
> +
> static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
> {
> const u32 attr = EFI_VARIABLE_NON_VOLATILE |
> @@ -64,6 +81,7 @@ static const struct super_operations efivarfs_ops = {
> .statfs = efivarfs_statfs,
> .drop_inode = generic_delete_inode,
> .evict_inode = efivarfs_evict_inode,
> + .show_options = efivarfs_show_options,
> };
>
> /*
> @@ -225,6 +243,45 @@ static int efivarfs_destroy(struct efivar_entry *entry, void *data)
> return 0;
> }
>
> +enum {
> + Opt_uid, Opt_gid,
> +};
> +
> +static const struct fs_parameter_spec efivarfs_parameters[] = {
> + fsparam_u32("uid", Opt_uid),
> + fsparam_u32("gid", Opt_gid),
> + {},
> +};
> +
> +static int efivarfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
> +{
> + struct efivarfs_fs_info *sbi = fc->s_fs_info;
> + struct efivarfs_mount_opts *opts = &sbi->mount_opts;
> + struct fs_parse_result result;
> + int opt;
> +
> + opt = fs_parse(fc, efivarfs_parameters, param, &result);
> + if (opt < 0)
> + return opt;
> +
> + switch (opt) {
> + case Opt_uid:
> + opts->uid = make_kuid(current_user_ns(), result.uint_32);
> + if (!uid_valid(opts->uid))
> + return -EINVAL;
> + break;
> + case Opt_gid:
> + opts->gid = make_kgid(current_user_ns(), result.uint_32);
> + if (!gid_valid(opts->gid))
> + return -EINVAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
> {
> struct inode *inode = NULL;
> @@ -270,11 +327,22 @@ static int efivarfs_get_tree(struct fs_context *fc)
> }
>
> static const struct fs_context_operations efivarfs_context_ops = {
> - .get_tree = efivarfs_get_tree,
> + .get_tree = efivarfs_get_tree,
> + .parse_param = efivarfs_parse_param,
> };
>
> static int efivarfs_init_fs_context(struct fs_context *fc)
> {
> + struct efivarfs_fs_info *sfi;
> +
> + sfi = kzalloc(sizeof(struct efivarfs_fs_info), GFP_KERNEL);
> + if (!sizeof(*sfi))
Something went wrong here
> + return -ENOMEM;
> +
> + sfi->mount_opts.uid = GLOBAL_ROOT_UID;
> + sfi->mount_opts.gid = GLOBAL_ROOT_GID;
> +
> + fc->s_fs_info = sfi;
> fc->ops = &efivarfs_context_ops;
> return 0;
> }
> @@ -291,10 +359,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,
> };
>
> static __init int efivarfs_init(void)
> --
> 2.42.0.283.g2d96d420d3-goog
>
© 2016 - 2025 Red Hat, Inc.