kevin/linux
1
Fork 0
Code

tracefs: Reset permissions on remount if permissions are options

Browse Source 
There's an inconsistency with the way permissions are handled in tracefs.
Because the permissions are generated when accessed, they default to the
root inode's permission if they were never set by the user. If the user
sets the permissions, then a flag is set and the permissions are saved via
the inode (for tracefs files) or an internal attribute field (for
eventfs).

But if a remount happens that specify the permissions, all the files that
were not changed by the user gets updated, but the ones that were are not.
If the user were to remount the file system with a given permission, then
all files and directories within that file system should be updated.

This can cause security issues if a file's permission was updated but the
admin forgot about it. They could incorrectly think that remounting with
permissions set would update all files, but miss some.

For example:

 # cd /sys/kernel/tracing
 # chgrp 1002 current_tracer
 # ls -l
[..]
 -rw-r-----  1 root root 0 May  1 21:25 buffer_size_kb
 -rw-r-----  1 root root 0 May  1 21:25 buffer_subbuf_size_kb
 -r--r-----  1 root root 0 May  1 21:25 buffer_total_size_kb
 -rw-r-----  1 root lkp  0 May  1 21:25 current_tracer
 -rw-r-----  1 root root 0 May  1 21:25 dynamic_events
 -r--r-----  1 root root 0 May  1 21:25 dyn_ftrace_total_info
 -r--r-----  1 root root 0 May  1 21:25 enabled_functions

Where current_tracer now has group "lkp".

 # mount -o remount,gid=1001 .
 # ls -l
 -rw-r-----  1 root tracing 0 May  1 21:25 buffer_size_kb
 -rw-r-----  1 root tracing 0 May  1 21:25 buffer_subbuf_size_kb
 -r--r-----  1 root tracing 0 May  1 21:25 buffer_total_size_kb
 -rw-r-----  1 root lkp     0 May  1 21:25 current_tracer
 -rw-r-----  1 root tracing 0 May  1 21:25 dynamic_events
 -r--r-----  1 root tracing 0 May  1 21:25 dyn_ftrace_total_info
 -r--r-----  1 root tracing 0 May  1 21:25 enabled_functions

Everything changed but the "current_tracer".

Add a new link list that keeps track of all the tracefs_inodes which has
the permission flags that tell if the file/dir should use the root inode's
permission or not. Then on remount, clear all the flags so that the
default behavior of using the root inode's permission is done for all
files and directories.

Link: https://lore.kernel.org/linux-trace-kernel/20240502200905.529542160@goodmis.org

Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Fixes: 8186fff7ab ("tracefs/eventfs: Use root and instance inodes as default ownership")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
This commit is contained in:
Steven Rostedt (Google) 2024-05-02 16:08:23 -04:00
parent ee4e037947
commit baa23a8d43
3 changed files with 99 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
fs/tracefs/event_inode.c
View File

@ -308,6 +308,35 @@ static const struct file_operations eventfs_file_operations = {
.llseek = generic_file_llseek,
};
/*
* On a remount of tracefs, if UID or GID options are set, then
* the mount point inode permissions should be used.
* Reset the saved permission flags appropriately.
*/
void eventfs_remount(struct tracefs_inode *ti, bool update_uid, bool update_gid)
{
struct eventfs_inode *ei = ti->private;
if (!ei)
return;
if (update_uid)
ei->attr.mode &= ~EVENTFS_SAVE_UID;
if (update_gid)
ei->attr.mode &= ~EVENTFS_SAVE_GID;
if (!ei->entry_attrs)
return;
for (int i = 0; i < ei->nr_entries; i++) {
if (update_uid)
ei->entry_attrs[i].mode &= ~EVENTFS_SAVE_UID;
if (update_gid)
ei->entry_attrs[i].mode &= ~EVENTFS_SAVE_GID;
}
}
/* Return the evenfs_inode of the "events" directory */
static struct eventfs_inode *eventfs_find_events(struct dentry *dentry)
{

65
fs/tracefs/inode.c
View File

@ -30,20 +30,47 @@ static struct vfsmount *tracefs_mount;
static int tracefs_mount_count;
static bool tracefs_registered;
/*
* Keep track of all tracefs_inodes in order to update their
* flags if necessary on a remount.
*/
static DEFINE_SPINLOCK(tracefs_inode_lock);
static LIST_HEAD(tracefs_inodes);
static struct inode *tracefs_alloc_inode(struct super_block *sb)
{
struct tracefs_inode *ti;
unsigned long flags;
ti = kmem_cache_alloc(tracefs_inode_cachep, GFP_KERNEL);
if (!ti)
return NULL;
spin_lock_irqsave(&tracefs_inode_lock, flags);
list_add_rcu(&ti->list, &tracefs_inodes);
spin_unlock_irqrestore(&tracefs_inode_lock, flags);
return &ti->vfs_inode;
}
static void tracefs_free_inode_rcu(struct rcu_head *rcu)
{
struct tracefs_inode *ti;
ti = container_of(rcu, struct tracefs_inode, rcu);
kmem_cache_free(tracefs_inode_cachep, ti);
}
static void tracefs_free_inode(struct inode *inode)
{
kmem_cache_free(tracefs_inode_cachep, get_tracefs(inode));
struct tracefs_inode *ti = get_tracefs(inode);
unsigned long flags;
spin_lock_irqsave(&tracefs_inode_lock, flags);
list_del_rcu(&ti->list);
spin_unlock_irqrestore(&tracefs_inode_lock, flags);
call_rcu(&ti->rcu, tracefs_free_inode_rcu);
}
static ssize_t default_read_file(struct file *file, char __user *buf,
@ -313,6 +340,8 @@ static int tracefs_apply_options(struct super_block *sb, bool remount)
struct tracefs_fs_info *fsi = sb->s_fs_info;
struct inode *inode = d_inode(sb->s_root);
struct tracefs_mount_opts *opts = &fsi->mount_opts;
struct tracefs_inode *ti;
bool update_uid, update_gid;
umode_t tmp_mode;
/*
@ -332,6 +361,25 @@ static int tracefs_apply_options(struct super_block *sb, bool remount)
if (!remount || opts->opts & BIT(Opt_gid))
inode->i_gid = opts->gid;
if (remount && (opts->opts & BIT(Opt_uid) || opts->opts & BIT(Opt_gid))) {
update_uid = opts->opts & BIT(Opt_uid);
update_gid = opts->opts & BIT(Opt_gid);
rcu_read_lock();
list_for_each_entry_rcu(ti, &tracefs_inodes, list) {
if (update_uid)
ti->flags &= ~TRACEFS_UID_PERM_SET;
if (update_gid)
ti->flags &= ~TRACEFS_GID_PERM_SET;
if (ti->flags & TRACEFS_EVENT_INODE)
eventfs_remount(ti, update_uid, update_gid);
}
rcu_read_unlock();
}
return 0;
}
@ -398,7 +446,22 @@ static int tracefs_d_revalidate(struct dentry *dentry, unsigned int flags)
return !(ei && ei->is_freed);
}
static void tracefs_d_iput(struct dentry *dentry, struct inode *inode)
{
struct tracefs_inode *ti = get_tracefs(inode);
/*
* This inode is being freed and cannot be used for
* eventfs. Clear the flag so that it doesn't call into
* eventfs during the remount flag updates. The eventfs_inode
* gets freed after an RCU cycle, so the content will still
* be safe if the iteration is going on now.
*/
ti->flags &= ~TRACEFS_EVENT_INODE;
}
static const struct dentry_operations tracefs_dentry_operations = {
.d_iput = tracefs_d_iput,
.d_revalidate = tracefs_d_revalidate,
.d_release = tracefs_d_release,
};

7
fs/tracefs/internal.h
View File

@ -11,8 +11,12 @@ enum {
};
struct tracefs_inode {
struct inode vfs_inode;
union {
struct inode vfs_inode;
struct rcu_head rcu;
};
/* The below gets initialized with memset_after(ti, 0, vfs_inode) */
struct list_head list;
unsigned long flags;
void *private;
};
@ -73,6 +77,7 @@ struct dentry *tracefs_end_creating(struct dentry *dentry);
struct dentry *tracefs_failed_creating(struct dentry *dentry);
struct inode *tracefs_get_inode(struct super_block *sb);
void eventfs_remount(struct tracefs_inode *ti, bool update_uid, bool update_gid);
void eventfs_d_release(struct dentry *dentry);
#endif /* _TRACEFS_INTERNAL_H */