From patchwork Thu May 2 15:15:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13651767 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 216A115AAD3; Thu, 2 May 2024 15:15:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714662934; cv=none; b=ddI2wGpjjMg4bnw8VuA/4tVZkoKZI85Osu4hWo3/8QWEW/gv1T00YvZxePBCG/cKgdni3rXwl+6wQEC/ubK5VQENBCfIUK1DH881SCT72EUE2GqL3uRA3zD+pzCYMRe+M1cVIDgRw4X1/Nnm1J/itI/l6eQsdMf0Qo3P1MWcFVQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714662934; c=relaxed/simple; bh=blD4FyFN/Ay6kzfycDUilXFE+LDcgUi63fpEwEhTB5g=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=B//93VC8EzhJ2ps2QDUa7F/yCu/RhLmBwN3oTsUAjbpzSDtYPn3BUBS46jctAXyxVUEAr/mLNi8+92odO8IrcrIN4MqRS0AXYVd8izyYaqw/R5xejF+U23fz4aSJ7iBCJ+s0qIYldsC/Z5G5bPTdhJQi2ZzcTMniatKZih/da78= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id C0480C32789; Thu, 2 May 2024 15:15:33 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1s2YAW-00000003ISk-2GNt; Thu, 02 May 2024 11:16:20 -0400 Message-ID: <20240502151620.400578783@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 02 May 2024 11:15:48 -0400 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Cc: Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Andrew Morton , stable@vger.kernel.org Subject: [PATCH v2 1/5] tracefs: Reset permissions on remount if permissions are options References: <20240502151547.973653253@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" 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. Cc: stable@vger.kernel.org Fixes: 8186fff7ab649 ("tracefs/eventfs: Use root and instance inodes as default ownership") Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 29 +++++++++++++++++++++++ fs/tracefs/inode.c | 50 +++++++++++++++++++++++++++++++++++++++- fs/tracefs/internal.h | 7 +++++- 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index f5510e26f0f6..a754c6725a52 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -301,6 +301,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) { diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c index 5545e6bf7d26..5f2417df1c43 100644 --- a/fs/tracefs/inode.c +++ b/fs/tracefs/inode.c @@ -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; } diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h index 15c26f9aaad4..29f0c999975b 100644 --- a/fs/tracefs/internal.h +++ b/fs/tracefs/internal.h @@ -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 */ From patchwork Thu May 2 15:15:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13651769 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 59F9515ADBF; Thu, 2 May 2024 15:15:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714662934; cv=none; b=ee1eKrMEsweybu8w2X/8u9STIkCb7ETD1/M+0zOk6JY4Bt6c1zwHz1/1KJt73OiDsi3yXx5PtXWJ8q6424GVfJPSDrMqbQqym1K8OZ/78eyD8YV/UWCWpTURO1BAeff7mv/iFRHXA/0s+4ukwRsjfQhQDcrM66Hux1M9R7bB0l0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714662934; c=relaxed/simple; bh=Zjm2gWdBScHrZHJ6OHWJqK9+Y6TkySrNHNWy7Q+/g+0=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=O/YgVGYPwadP9DBQU6La9t29ryRTEqTDjnShoATExOuGc8KdfmiSENdZ2otS8vS/ccwL30sSwIkdv4XIU/NKUwA6p7lFUIhf8P2lKaqQjLqfvl5t5cTUui+HVpnu56G1mNNKriVnIaLeYW+thMlDThS/wN/hTHIXVnKKwL+23iI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 13A7AC4AF1A; Thu, 2 May 2024 15:15:34 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1s2YAW-00000003ITF-2uZX; Thu, 02 May 2024 11:16:20 -0400 Message-ID: <20240502151620.558412090@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 02 May 2024 11:15:49 -0400 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Cc: Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Andrew Morton , stable@vger.kernel.org Subject: [PATCH v2 2/5] tracefs: Still use mount point as default permissions for instances References: <20240502151547.973653253@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" If the instances directory's permissions were never change, then have it and its children use the mount point permissions as the default. Currently, the permissions of instance directories are determined by the instance directory's permissions itself. But if the tracefs file system is remounted and changes the permissions, the instance directory and its children should use the new permission. But because both the instance directory and its children use the instance directory's inode for permissions, it misses the update. To demonstrate this: # cd /sys/kernel/tracing/ # mkdir instances/foo # ls -ld instances/foo drwxr-x--- 5 root root 0 May 1 19:07 instances/foo # ls -ld instances drwxr-x--- 3 root root 0 May 1 18:57 instances # ls -ld current_tracer -rw-r----- 1 root root 0 May 1 18:57 current_tracer # mount -o remount,gid=1002 . # ls -ld instances drwxr-x--- 3 root root 0 May 1 18:57 instances # ls -ld instances/foo/ drwxr-x--- 5 root root 0 May 1 19:07 instances/foo/ # ls -ld current_tracer -rw-r----- 1 root lkp 0 May 1 18:57 current_tracer Notice that changing the group id to that of "lkp" did not affect the instances directory nor its children. It should have been: # ls -ld current_tracer -rw-r----- 1 root root 0 May 1 19:19 current_tracer # ls -ld instances/foo/ drwxr-x--- 5 root root 0 May 1 19:25 instances/foo/ # ls -ld instances drwxr-x--- 3 root root 0 May 1 19:19 instances # mount -o remount,gid=1002 . # ls -ld current_tracer -rw-r----- 1 root lkp 0 May 1 19:19 current_tracer # ls -ld instances drwxr-x--- 3 root lkp 0 May 1 19:19 instances # ls -ld instances/foo/ drwxr-x--- 5 root lkp 0 May 1 19:25 instances/foo/ Where all files were updated by the remount gid update. Cc: stable@vger.kernel.org Fixes: 8186fff7ab649 ("tracefs/eventfs: Use root and instance inodes as default ownership") Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/inode.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c index 5f2417df1c43..e1bf65efdbb3 100644 --- a/fs/tracefs/inode.c +++ b/fs/tracefs/inode.c @@ -180,16 +180,39 @@ static void set_tracefs_inode_owner(struct inode *inode) { struct tracefs_inode *ti = get_tracefs(inode); struct inode *root_inode = ti->private; + kuid_t uid; + kgid_t gid; + + uid = root_inode->i_uid; + gid = root_inode->i_gid; + + /* + * If the root is not the mount point, then check the root's + * permissions. If it was never set, then default to the + * mount point. + */ + if (root_inode != d_inode(root_inode->i_sb->s_root)) { + struct tracefs_inode *rti; + + rti = get_tracefs(root_inode); + root_inode = d_inode(root_inode->i_sb->s_root); + + if (!(rti->flags & TRACEFS_UID_PERM_SET)) + uid = root_inode->i_uid; + + if (!(rti->flags & TRACEFS_GID_PERM_SET)) + gid = root_inode->i_gid; + } /* * If this inode has never been referenced, then update * the permissions to the superblock. */ if (!(ti->flags & TRACEFS_UID_PERM_SET)) - inode->i_uid = root_inode->i_uid; + inode->i_uid = uid; if (!(ti->flags & TRACEFS_GID_PERM_SET)) - inode->i_gid = root_inode->i_gid; + inode->i_gid = gid; } static int tracefs_permission(struct mnt_idmap *idmap, From patchwork Thu May 2 15:15:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13651768 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 59FEA15B0E7; Thu, 2 May 2024 15:15:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714662934; cv=none; b=kUaTa6Qgo/ROGuffzDekM5xLj6XjpXL8lL6wTS31t8uJCmQIBe/tl+ff0/X7PiRyVSYU9XQVYkq3o3iwZwyz5hgXBFAaOlQCiB4mORg/WuQZSP+wo8MZnscGezY8LGmeAb8BSnjbiDNNHnvOrOzUCWkqFWLJkcy9oXsJpneSo4I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714662934; c=relaxed/simple; bh=/hEvsIWw8xq5n6yLQhKQJHjRomIpW/YtSsUGay22gSE=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=ZMj2l4zrSlE0Js1u9qC3EfDs60o+ZnSG22kNMKPaQYMKjILsXS2BjIjGDXLKgof9VgRuaVlKxsFesN4H+82FjiW9wP2mZVO2hzByxpEWCzmE1XD6ofNTk93tl73Iy8dEJfoi/e9oedOMJjaiZKCZOBJbmdEJtBU/h/130U4Bs1k= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 13B8AC4AF1B; Thu, 2 May 2024 15:15:34 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1s2YAW-00000003ITj-3ag4; Thu, 02 May 2024 11:16:20 -0400 Message-ID: <20240502151620.714547671@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 02 May 2024 11:15:50 -0400 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Cc: Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Andrew Morton , stable@vger.kernel.org Subject: [PATCH v2 3/5] eventfs: Do not differentiate the toplevel events directory References: <20240502151547.973653253@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" The toplevel events directory is really no different than the events directory of instances. Having the two be different caused inconsistencies and made it harder to fix the permissions bugs. Make all events directories act the same. Cc: stable@vger.kernel.org Fixes: 8186fff7ab649 ("tracefs/eventfs: Use root and instance inodes as default ownership") Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 29 ++++++++--------------------- fs/tracefs/internal.h | 7 +++---- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index a754c6725a52..2971609946e5 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -68,7 +68,6 @@ enum { EVENTFS_SAVE_MODE = BIT(16), EVENTFS_SAVE_UID = BIT(17), EVENTFS_SAVE_GID = BIT(18), - EVENTFS_TOPLEVEL = BIT(19), }; #define EVENTFS_MODE_MASK (EVENTFS_SAVE_MODE - 1) @@ -232,14 +231,10 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry, return ret; } -static void update_top_events_attr(struct eventfs_inode *ei, struct super_block *sb) +static void update_events_attr(struct eventfs_inode *ei, struct super_block *sb) { struct inode *root; - /* Only update if the "events" was on the top level */ - if (!ei || !(ei->attr.mode & EVENTFS_TOPLEVEL)) - return; - /* Get the tracefs root inode. */ root = d_inode(sb->s_root); ei->attr.uid = root->i_uid; @@ -252,10 +247,10 @@ static void set_top_events_ownership(struct inode *inode) struct eventfs_inode *ei = ti->private; /* The top events directory doesn't get automatically updated */ - if (!ei || !ei->is_events || !(ei->attr.mode & EVENTFS_TOPLEVEL)) + if (!ei || !ei->is_events) return; - update_top_events_attr(ei, inode->i_sb); + update_events_attr(ei, inode->i_sb); if (!(ei->attr.mode & EVENTFS_SAVE_UID)) inode->i_uid = ei->attr.uid; @@ -284,7 +279,7 @@ static int eventfs_permission(struct mnt_idmap *idmap, return generic_permission(idmap, inode, mask); } -static const struct inode_operations eventfs_root_dir_inode_operations = { +static const struct inode_operations eventfs_dir_inode_operations = { .lookup = eventfs_root_lookup, .setattr = eventfs_set_attr, .getattr = eventfs_get_attr, @@ -352,7 +347,7 @@ static struct eventfs_inode *eventfs_find_events(struct dentry *dentry) // Walk upwards until you find the events inode } while (!ei->is_events); - update_top_events_attr(ei, dentry->d_sb); + update_events_attr(ei, dentry->d_sb); return ei; } @@ -458,7 +453,7 @@ static struct dentry *lookup_dir_entry(struct dentry *dentry, update_inode_attr(dentry, inode, &ei->attr, S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO); - inode->i_op = &eventfs_root_dir_inode_operations; + inode->i_op = &eventfs_dir_inode_operations; inode->i_fop = &eventfs_file_operations; /* All directories will have the same inode number */ @@ -838,14 +833,6 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry uid = d_inode(dentry->d_parent)->i_uid; gid = d_inode(dentry->d_parent)->i_gid; - /* - * If the events directory is of the top instance, then parent - * is NULL. Set the attr.mode to reflect this and its permissions will - * default to the tracefs root dentry. - */ - if (!parent) - ei->attr.mode = EVENTFS_TOPLEVEL; - /* This is used as the default ownership of the files and directories */ ei->attr.uid = uid; ei->attr.gid = gid; @@ -854,13 +841,13 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry INIT_LIST_HEAD(&ei->list); ti = get_tracefs(inode); - ti->flags |= TRACEFS_EVENT_INODE | TRACEFS_EVENT_TOP_INODE; + ti->flags |= TRACEFS_EVENT_INODE; ti->private = ei; inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO; inode->i_uid = uid; inode->i_gid = gid; - inode->i_op = &eventfs_root_dir_inode_operations; + inode->i_op = &eventfs_dir_inode_operations; inode->i_fop = &eventfs_file_operations; dentry->d_fsdata = get_ei(ei); diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h index 29f0c999975b..f704d8348357 100644 --- a/fs/tracefs/internal.h +++ b/fs/tracefs/internal.h @@ -4,10 +4,9 @@ enum { TRACEFS_EVENT_INODE = BIT(1), - TRACEFS_EVENT_TOP_INODE = BIT(2), - TRACEFS_GID_PERM_SET = BIT(3), - TRACEFS_UID_PERM_SET = BIT(4), - TRACEFS_INSTANCE_INODE = BIT(5), + TRACEFS_GID_PERM_SET = BIT(2), + TRACEFS_UID_PERM_SET = BIT(3), + TRACEFS_INSTANCE_INODE = BIT(4), }; struct tracefs_inode { From patchwork Thu May 2 15:15:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13651770 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9178415B141; Thu, 2 May 2024 15:15:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714662934; cv=none; b=OIPh0YhFe+yaQUjzrK4nLzm55QFgwm65U6wl9veXspZEiuEmhnlk4vUIXXeCiFwzs8XGNBiwjpQ4AlvqWcsAKrF9kuBT3DW7edk+zQ2dqlLJfkopfB3Hd5NqjqbIqmDo/vk4bKKXwSGwtaLDUgfqcq0yItndeaOwg5l1ape6K3w= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714662934; c=relaxed/simple; bh=i+Q36AjRIPs7Ry987XtuVA+g0h3Lg22zj9IHFjW5j1M=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=caoiMNtYht+Lb/j7iFBhQvUPGepsLXd++M4a9rhwyf4iRogkRXZPxCNmKE6NLynUmBE0nrdpBWRm796Y2GPXUv3KNFIo8sVFbwrZrC2A8s5t9O+MPN554DehmqkUH7cGJqTLu9FiKbimRVLulML/q6NNDNellyRs9Qlwq7/eWGE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 51FF2C4AF18; Thu, 2 May 2024 15:15:34 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1s2YAX-00000003IUD-024u; Thu, 02 May 2024 11:16:21 -0400 Message-ID: <20240502151620.874018137@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 02 May 2024 11:15:51 -0400 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Cc: Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Andrew Morton , stable@vger.kernel.org Subject: [PATCH v2 4/5] eventfs: Do not treat events directory different than other directories References: <20240502151547.973653253@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" Treat the events directory the same as other directories when it comes to permissions. The events directory was considered different because it's dentry is persistent, whereas the other directory dentries are created when accessed. But the way tracefs now does its ownership by using the root dentry's permissions as the default permissions, the events directory can get out of sync when a remount is performed setting the group and user permissions. Remove the special case for the events directory on setting the attributes. This allows the updates caused by remount to work properly as well as simplifies the code. Cc: stable@vger.kernel.org Fixes: 8186fff7ab649 ("tracefs/eventfs: Use root and instance inodes as default ownership") Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 2971609946e5..ff4ee3dad56a 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -199,21 +199,7 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry, * determined by the parent directory. */ if (dentry->d_inode->i_mode & S_IFDIR) { - /* - * The events directory dentry is never freed, unless its - * part of an instance that is deleted. It's attr is the - * default for its child files and directories. - * Do not update it. It's not used for its own mode or ownership. - */ - if (ei->is_events) { - /* But it still needs to know if it was modified */ - if (iattr->ia_valid & ATTR_UID) - ei->attr.mode |= EVENTFS_SAVE_UID; - if (iattr->ia_valid & ATTR_GID) - ei->attr.mode |= EVENTFS_SAVE_GID; - } else { - update_attr(&ei->attr, iattr); - } + update_attr(&ei->attr, iattr); } else { name = dentry->d_name.name; From patchwork Thu May 2 15:15:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13651771 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8F95615B13F; Thu, 2 May 2024 15:15:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714662934; cv=none; b=d+/pfJvScT963yngEXn7Sx0WtJqc+u42qIYwS7RYCyA+lMBCcjQ0Y+d6IN+7uS0vpeP0ozN0hm/lD6QE0Eb5NssKkfDtq/w2QAWW6qt5TUYBtPrgvBuak3KS0F9c8YAcKLxRnE49i+1X278R0JK0jtNGZUjQ7xXHvkCjqCvNWAg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714662934; c=relaxed/simple; bh=X3ZZVdZWOdcjoe18hYCSjWceXc6V6boUtWZbJa/y5Us=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=m0pS0ELkOrw2VjkNr/e6gUbMN1zqvOFm4GybJgeli8HixACoUc4wsUokDq0jcnh92T43Z3cc1JtbZZYy9q1t1PH++F/I0fn+4zXCRlkUQGiDNRdrYPfJzXu2WnTgBXAqkmvwgBsEaFkCV7mouziwKD3R5qxw8duekkd4dJhKsf4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 77929C4AF50; Thu, 2 May 2024 15:15:34 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1s2YAX-00000003IUh-0hxq; Thu, 02 May 2024 11:16:21 -0400 Message-ID: <20240502151621.029168056@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 02 May 2024 11:15:52 -0400 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Cc: Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Andrew Morton , stable@vger.kernel.org Subject: [PATCH v2 5/5] eventfs: Have "events" directory get permissions from its parent References: <20240502151547.973653253@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" The events directory gets its permissions from the root inode. But this can cause an inconsistency if the instances directory changes its permissions, as the permissions of the created directories under it should inherit the permissions of the instances directory when directories under it are created. Currently the behavior is: # cd /sys/kernel/tracing # chgrp 1002 instances # mkdir instances/foo # ls -l instances/foo [..] -r--r----- 1 root lkp 0 May 1 18:55 buffer_total_size_kb -rw-r----- 1 root lkp 0 May 1 18:55 current_tracer -rw-r----- 1 root lkp 0 May 1 18:55 error_log drwxr-xr-x 1 root root 0 May 1 18:55 events --w------- 1 root lkp 0 May 1 18:55 free_buffer drwxr-x--- 2 root lkp 0 May 1 18:55 options drwxr-x--- 10 root lkp 0 May 1 18:55 per_cpu -rw-r----- 1 root lkp 0 May 1 18:55 set_event All the files and directories under "foo" has the "lkp" group except the "events" directory. That's because its getting its default value from the mount point instead of its parent. Have the "events" directory make its default value based on its parent's permissions. That now gives: # ls -l instances/foo [..] -rw-r----- 1 root lkp 0 May 1 21:16 buffer_subbuf_size_kb -r--r----- 1 root lkp 0 May 1 21:16 buffer_total_size_kb -rw-r----- 1 root lkp 0 May 1 21:16 current_tracer -rw-r----- 1 root lkp 0 May 1 21:16 error_log drwxr-xr-x 1 root lkp 0 May 1 21:16 events --w------- 1 root lkp 0 May 1 21:16 free_buffer drwxr-x--- 2 root lkp 0 May 1 21:16 options drwxr-x--- 10 root lkp 0 May 1 21:16 per_cpu -rw-r----- 1 root lkp 0 May 1 21:16 set_event Cc: stable@vger.kernel.org Fixes: 8186fff7ab649 ("tracefs/eventfs: Use root and instance inodes as default ownership") Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index ff4ee3dad56a..3ea32159d556 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -37,6 +37,7 @@ static DEFINE_MUTEX(eventfs_mutex); struct eventfs_root_inode { struct eventfs_inode ei; + struct inode *parent_inode; struct dentry *events_dir; }; @@ -219,12 +220,23 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry, static void update_events_attr(struct eventfs_inode *ei, struct super_block *sb) { - struct inode *root; + struct eventfs_root_inode *rei; + struct inode *parent; + + rei = get_root_inode(ei); + + /* Use the parent inode permissions unless root set its permissions */ + parent = rei->parent_inode; - /* Get the tracefs root inode. */ - root = d_inode(sb->s_root); - ei->attr.uid = root->i_uid; - ei->attr.gid = root->i_gid; + if (rei->ei.attr.mode & EVENTFS_SAVE_UID) + ei->attr.uid = rei->ei.attr.uid; + else + ei->attr.uid = parent->i_uid; + + if (rei->ei.attr.mode & EVENTFS_SAVE_GID) + ei->attr.gid = rei->ei.attr.gid; + else + ei->attr.gid = parent->i_gid; } static void set_top_events_ownership(struct inode *inode) @@ -810,6 +822,7 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry // Note: we have a ref to the dentry from tracefs_start_creating() rei = get_root_inode(ei); rei->events_dir = dentry; + rei->parent_inode = d_inode(dentry->d_sb->s_root); ei->entries = entries; ei->nr_entries = size; @@ -819,10 +832,15 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry uid = d_inode(dentry->d_parent)->i_uid; gid = d_inode(dentry->d_parent)->i_gid; - /* This is used as the default ownership of the files and directories */ ei->attr.uid = uid; ei->attr.gid = gid; + /* + * When the "events" directory is created, it takes on the + * permissions of its parent. But can be reset on remount. + */ + ei->attr.mode |= EVENTFS_SAVE_UID | EVENTFS_SAVE_GID; + INIT_LIST_HEAD(&ei->children); INIT_LIST_HEAD(&ei->list);