eventfs: Cleanup permissions in creation of inodes

The permissions being set during the creation of the inodes was updating
eventfs_inode attributes as well. Those attributes should only be touched
by the setattr or remount operations, not during the creation of inodes.
The eventfs_inode attributes should only be used to set the inodes and
should not be modified during the inode creation.

Simplify the code and fix the situation by:

 1) Removing the eventfs_find_events() and doing a simple lookup for
    the events descriptor in eventfs_get_inode()

 2) Remove update_events_attr() as the attributes should only be used
    to update the inode and should not be modified here.

 3) Add update_inode_attr() that uses the attributes to determine what
    the inode permissions should be.

 4) As the parent_inode of the eventfs_root_inode structure is no longer
    needed, remove it.

Now on creation, the inode gets the proper permissions without causing
side effects to the ei->attr field.

Link: https://lore.kernel.org/lkml/20240522165031.944088388@goodmis.org
Cc: Linus Torvalds <torvalds@linux-foundation.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>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index f32c6f7eb29fd49448973e23eafe4f82cd727b3a..320e49056d3ab31b2dda32a1ae35ea7e618bc57b 100644 (file)
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -37,7 +37,6 @@ static DEFINE_MUTEX(eventfs_mutex);
 
 struct eventfs_root_inode {
        struct eventfs_inode            ei;
-       struct inode                    *parent_inode;
        struct dentry                   *events_dir;
 };
 
@@ -229,27 +228,6 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry,
        return ret;
 }
 
-static void update_events_attr(struct eventfs_inode *ei, struct super_block *sb)
-{
-       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;
-
-       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 const struct inode_operations eventfs_dir_inode_operations = {
        .lookup         = eventfs_root_lookup,
        .setattr        = eventfs_set_attr,
@@ -321,60 +299,30 @@ void eventfs_remount(struct tracefs_inode *ti, bool update_uid, bool update_gid)
                          update_gid, ti->vfs_inode.i_gid, 0);
 }
 
-/* Return the evenfs_inode of the "events" directory */
-static struct eventfs_inode *eventfs_find_events(struct dentry *dentry)
+static void update_inode_attr(struct inode *inode, umode_t mode,
+                             struct eventfs_attr *attr, struct eventfs_root_inode *rei)
 {
-       struct eventfs_inode *ei;
-
-       do {
-               // The parent is stable because we do not do renames
-               dentry = dentry->d_parent;
-               // ... and directories always have d_fsdata
-               ei = dentry->d_fsdata;
-
-               /*
-                * If the ei is being freed, the ownership of the children
-                * doesn't matter.
-                */
-               if (ei->is_freed)
-                       return NULL;
-
-               // Walk upwards until you find the events inode
-       } while (!ei->is_events);
-
-       update_events_attr(ei, dentry->d_sb);
-
-       return ei;
-}
-
-static void update_inode_attr(struct dentry *dentry, struct inode *inode,
-                             struct eventfs_attr *attr, umode_t mode)
-{
-       struct eventfs_inode *events_ei = eventfs_find_events(dentry);
-
-       if (!events_ei)
-               return;
-
-       inode->i_mode = mode;
-       inode->i_uid = events_ei->attr.uid;
-       inode->i_gid = events_ei->attr.gid;
-
-       if (!attr)
-               return;
-
-       if (attr->mode & EVENTFS_SAVE_MODE)
+       if (attr && attr->mode & EVENTFS_SAVE_MODE)
                inode->i_mode = attr->mode & EVENTFS_MODE_MASK;
+       else
+               inode->i_mode = mode;
 
-       if (attr->mode & EVENTFS_SAVE_UID)
+       if (attr && attr->mode & EVENTFS_SAVE_UID)
                inode->i_uid = attr->uid;
+       else
+               inode->i_uid = rei->ei.attr.uid;
 
-       if (attr->mode & EVENTFS_SAVE_GID)
+       if (attr && attr->mode & EVENTFS_SAVE_GID)
                inode->i_gid = attr->gid;
+       else
+               inode->i_gid = rei->ei.attr.gid;
 }
 
 static struct inode *eventfs_get_inode(struct dentry *dentry, struct eventfs_attr *attr,
                                       umode_t mode,  struct eventfs_inode *ei)
 {
+       struct eventfs_root_inode *rei;
+       struct eventfs_inode *pei;
        struct tracefs_inode *ti;
        struct inode *inode;
 
@@ -386,7 +334,16 @@ static struct inode *eventfs_get_inode(struct dentry *dentry, struct eventfs_att
        ti->private = ei;
        ti->flags |= TRACEFS_EVENT_INODE;
 
-       update_inode_attr(dentry, inode, attr, mode);
+       /* Find the top dentry that holds the "events" directory */
+       do {
+               dentry = dentry->d_parent;
+               /* Directories always have d_fsdata */
+               pei = dentry->d_fsdata;
+       } while (!pei->is_events);
+
+       rei = get_root_inode(pei);
+
+       update_inode_attr(inode, mode, attr, rei);
 
        return inode;
 }
@@ -823,7 +780,6 @@ 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;