fs/efivarfs/inode.c | 4 ++++ fs/efivarfs/internal.h | 9 +++++++ fs/efivarfs/super.c | 54 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+)
Add uid and gid in efivarfs's mount option, so that
we can mount the file system with ownership. This approach
is used by a number of other filesystems that don't have
native support for ownership
Signed-off-by: Jiao Zhou <jiaozhou@google.com>
---
fs/efivarfs/inode.c | 4 ++++
fs/efivarfs/internal.h | 9 +++++++
fs/efivarfs/super.c | 54 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+)
diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
index b973a2c03dde..86175e229b0f 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->i_ctime = current_time(inode);
diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
index 8ebf3a6a8aa2..2c7b6b24df19 100644
--- a/fs/efivarfs/internal.h
+++ b/fs/efivarfs/internal.h
@@ -48,6 +48,15 @@ bool efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data,
bool efivar_variable_is_removable(efi_guid_t vendor, const char *name,
size_t len);
+struct efivarfs_mount_opts {
+ kuid_t uid;
+ kgid_t gid;
+};
+
+struct efivarfs_fs_info {
+ struct efivarfs_mount_opts mount_opts;
+};
+
extern const struct file_operations efivarfs_file_operations;
extern const struct inode_operations efivarfs_dir_inode_operations;
extern bool efivarfs_valid_name(const char *str, int len);
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index e028fafa04f3..e3c81fac8208 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>
@@ -60,10 +61,27 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
return 0;
}
+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 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,40 @@ 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);
+ break;
+ case Opt_gid:
+ opts->gid = make_kgid(current_user_ns(), result.uint_32);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
{
struct inode *inode = NULL;
@@ -271,6 +323,7 @@ static int efivarfs_get_tree(struct fs_context *fc)
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)
@@ -295,6 +348,7 @@ static struct file_system_type efivarfs_type = {
.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.rc1.204.g551eb34607-googOn Tue, 22 Aug 2023 at 18:24, Jiao Zhou <jiaozhou@google.com> wrote:
>
> Add uid and gid in efivarfs's mount option, so that
> we can mount the file system with ownership. This approach
> is used by a number of other filesystems that don't have
> native support for ownership
>
> Signed-off-by: Jiao Zhou <jiaozhou@google.com>
> ---
>
> fs/efivarfs/inode.c | 4 ++++
> fs/efivarfs/internal.h | 9 +++++++
> fs/efivarfs/super.c | 54 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 67 insertions(+)
>
> diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
> index b973a2c03dde..86175e229b0f 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->i_ctime = current_time(inode);
> diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
> index 8ebf3a6a8aa2..2c7b6b24df19 100644
> --- a/fs/efivarfs/internal.h
> +++ b/fs/efivarfs/internal.h
> @@ -48,6 +48,15 @@ bool efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data,
> bool efivar_variable_is_removable(efi_guid_t vendor, const char *name,
> size_t len);
>
> +struct efivarfs_mount_opts {
> + kuid_t uid;
> + kgid_t gid;
> +};
> +
> +struct efivarfs_fs_info {
> + struct efivarfs_mount_opts mount_opts;
> +};
> +
> extern const struct file_operations efivarfs_file_operations;
> extern const struct inode_operations efivarfs_dir_inode_operations;
> extern bool efivarfs_valid_name(const char *str, int len);
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index e028fafa04f3..e3c81fac8208 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>
> @@ -60,10 +61,27 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>
> return 0;
> }
> +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 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,40 @@ 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),
> +};
> +
The kasan report seems to suggest that this array needs a trailing
empty entry {}
> +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);
> + break;
> + case Opt_gid:
> + opts->gid = make_kgid(current_user_ns(), result.uint_32);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
> {
> struct inode *inode = NULL;
> @@ -271,6 +323,7 @@ static int efivarfs_get_tree(struct fs_context *fc)
>
> 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)
> @@ -295,6 +348,7 @@ static struct file_system_type efivarfs_type = {
> .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.rc1.204.g551eb34607-goog
>
Hello,
kernel test robot noticed "BUG:KASAN:global-out-of-bounds_in_fs_validate_description" on:
commit: 635056da10724c7af59483a8251f8e6432b50faa ("[PATCH] kernel: Add Mount Option For Efivarfs")
url: https://github.com/intel-lab-lkp/linux/commits/Jiao-Zhou/kernel-Add-Mount-Option-For-Efivarfs/20230823-002613
base: https://git.kernel.org/cgit/linux/kernel/git/efi/efi.git next
patch link: https://lore.kernel.org/all/20230822162350.1.I96423a31e88428004c2f4a28ccad13828adf433e@changeid/
patch subject: [PATCH] kernel: Add Mount Option For Efivarfs
in testcase: boot
compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202308291443.ea96ac66-oliver.sang@intel.com
[ 17.276007][ T1] ==================================================================
[ 17.277001][ T1] BUG: KASAN: global-out-of-bounds in fs_validate_description+0x1c2/0x1e0
[ 17.278097][ T1] Read of size 8 at addr ffffffff84f20b80 by task swapper/1
[ 17.278821][ T1]
[ 17.278821][ T1] CPU: 0 PID: 1 Comm: swapper Not tainted 6.5.0-rc1-00012-g635056da1072 #1
[ 17.278821][ T1] Call Trace:
[ 17.278821][ T1] <TASK>
[ 17.278821][ T1] dump_stack_lvl+0x32/0xa0
[ 17.278821][ T1] print_address_description+0x33/0x3c0
[ 17.278821][ T1] ? fs_validate_description+0x1c2/0x1e0
[ 17.278821][ T1] print_report+0xc1/0x280
[ 17.278821][ T1] ? kasan_addr_to_slab+0x11/0x80
[ 17.278821][ T1] ? fs_validate_description+0x1c2/0x1e0
[ 17.278821][ T1] kasan_report+0x154/0x190
[ 17.278821][ T1] ? fs_validate_description+0x1c2/0x1e0
[ 17.278821][ T1] __asan_report_load8_noabort+0x18/0x20
[ 17.278821][ T1] fs_validate_description+0x1c2/0x1e0
[ 17.278821][ T1] register_filesystem+0x73/0x2b0
[ 17.278821][ T1] ? pstore_blk_init+0x720/0x720
[ 17.278821][ T1] efivarfs_init+0x24/0x50
[ 17.278821][ T1] do_one_initcall+0xfc/0x480
[ 17.278821][ T1] ? trace_event_raw_event_initcall_level+0x1c0/0x1c0
[ 17.278821][ T1] ? __kasan_kmalloc+0x96/0xa0
[ 17.278821][ T1] ? do_initcalls+0x47/0x5b0
[ 17.278821][ T1] do_initcalls+0x2ae/0x5b0
[ 17.278821][ T1] kernel_init_freeable+0x313/0x570
[ 17.278821][ T1] ? rest_init+0x260/0x260
[ 17.278821][ T1] kernel_init+0x23/0x240
[ 17.278821][ T1] ? rest_init+0x260/0x260
[ 17.278821][ T1] ret_from_fork+0x22/0x30
[ 17.278821][ T1] </TASK>
[ 17.278821][ T1]
[ 17.278821][ T1] The buggy address belongs to the variable:
[ 17.278821][ T1] efivarfs_parameters+0x40/0x80
[ 17.278821][ T1]
[ 17.278821][ T1] The buggy address belongs to the physical page:
[ 17.278821][ T1] page:(____ptrval____) refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x4f20
[ 17.278821][ T1] flags: 0x4000000000001000(reserved|zone=1)
[ 17.278821][ T1] page_type: 0xffffffff()
[ 17.278821][ T1] raw: 4000000000001000 ffffea000013c808 ffffea000013c808 0000000000000000
[ 17.278821][ T1] raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
[ 17.278821][ T1] page dumped because: kasan: bad access detected
[ 17.278821][ T1] page_owner info is not present (never set?)
[ 17.278821][ T1]
[ 17.278821][ T1] Memory state around the buggy address:
[ 17.278821][ T1] ffffffff84f20a80: 00 00 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
[ 17.278821][ T1] ffffffff84f20b00: 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
[ 17.278821][ T1] >ffffffff84f20b80: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
[ 17.278821][ T1] ^
[ 17.278821][ T1] ffffffff84f20c00: 00 00 00 00 00 00 00 00 f9 f9 f9 f9 00 00 00 00
[ 17.278821][ T1] ffffffff84f20c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 17.278821][ T1] ==================================================================
[ 17.311721][ T1] Disabling lock debugging due to kernel taint
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20230829/202308291443.ea96ac66-oliver.sang@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
(cc Peter and Matthew)
On Tue, 22 Aug 2023 at 18:24, Jiao Zhou <jiaozhou@google.com> wrote:
>
> Add uid and gid in efivarfs's mount option, so that
> we can mount the file system with ownership. This approach
> is used by a number of other filesystems that don't have
> native support for ownership
>
> Signed-off-by: Jiao Zhou <jiaozhou@google.com>
> ---
>
> fs/efivarfs/inode.c | 4 ++++
> fs/efivarfs/internal.h | 9 +++++++
> fs/efivarfs/super.c | 54 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 67 insertions(+)
>
> diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
> index b973a2c03dde..86175e229b0f 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->i_ctime = current_time(inode);
> diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
> index 8ebf3a6a8aa2..2c7b6b24df19 100644
> --- a/fs/efivarfs/internal.h
> +++ b/fs/efivarfs/internal.h
> @@ -48,6 +48,15 @@ bool efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data,
> bool efivar_variable_is_removable(efi_guid_t vendor, const char *name,
> size_t len);
>
> +struct efivarfs_mount_opts {
> + kuid_t uid;
> + kgid_t gid;
> +};
> +
> +struct efivarfs_fs_info {
> + struct efivarfs_mount_opts mount_opts;
> +};
> +
> extern const struct file_operations efivarfs_file_operations;
> extern const struct inode_operations efivarfs_dir_inode_operations;
> extern bool efivarfs_valid_name(const char *str, int len);
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index e028fafa04f3..e3c81fac8208 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>
> @@ -60,10 +61,27 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>
> return 0;
> }
> +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 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,40 @@ 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);
> + break;
> + case Opt_gid:
> + opts->gid = make_kgid(current_user_ns(), result.uint_32);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
> {
> struct inode *inode = NULL;
> @@ -271,6 +323,7 @@ static int efivarfs_get_tree(struct fs_context *fc)
>
> 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)
> @@ -295,6 +348,7 @@ static struct file_system_type efivarfs_type = {
> .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.rc1.204.g551eb34607-goog
>
On Wed, Aug 23, 2023 at 06:30:12PM +0200, Ard Biesheuvel wrote: > (cc Peter and Matthew) > > On Tue, 22 Aug 2023 at 18:24, Jiao Zhou <jiaozhou@google.com> wrote: > > > > Add uid and gid in efivarfs's mount option, so that > > we can mount the file system with ownership. This approach > > is used by a number of other filesystems that don't have > > native support for ownership > > > > Signed-off-by: Jiao Zhou <jiaozhou@google.com> No inherent objection, but what's the use case?
© 2016 - 2025 Red Hat, Inc.