eventfs: Hold eventfs_mutex and SRCU when remount walks events

Commit 340f0c7067a9 ("eventfs: Update all the eventfs_inodes from the
events descriptor") had eventfs_set_attrs() recurse through ei->children
on remount.  The walk only holds the rcu_read_lock() taken by
tracefs_apply_options() over tracefs_inodes, which is wrong:

  - list_for_each_entry over ei->children races with the list_del_rcu()
    in eventfs_remove_rec() -- LIST_POISON1 deref, same shape as
    d2603279c7d6.
  - eventfs_inodes are freed via call_srcu(&eventfs_srcu, ...).
    rcu_read_lock() does not extend an SRCU grace period, so ti->private
    can be reclaimed under the walk.
  - The writes to ei->attr race with eventfs_set_attr(), which holds
    eventfs_mutex.

Reproducer:

  while :; do mount -o remount,uid=$((RANDOM%1000)) /sys/kernel/tracing; done &
  while :; do
      echo "p:kp submit_bio" > /sys/kernel/tracing/kprobe_events
      echo > /sys/kernel/tracing/kprobe_events
  done

Wrap the events portion of tracefs_apply_options() in
eventfs_remount_lock()/_unlock() that take eventfs_mutex and
srcu_read_lock(&eventfs_srcu).  eventfs_set_attrs() doesn't sleep so the
nested rcu_read_lock() is fine; lockdep_assert_held() pins the contract.

Comment in tracefs_drop_inode() said "RCU cycle" -- it is SRCU.

Fixes: 340f0c7067a9 ("eventfs: Update all the eventfs_inodes from the events descriptor")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260418191737.10289-1-devnexen@gmail.com
Signed-off-by: David Carlier <devnexen@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 8dd554508828b8142bf39831d266b2c56bf27d2c..26b6453de30ef668c32f47743d7fa6477f5398d6 100644 (file)
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -244,6 +244,8 @@ static void eventfs_set_attrs(struct eventfs_inode *ei, bool update_uid, kuid_t
 {
        struct eventfs_inode *ei_child;
 
+       lockdep_assert_held(&eventfs_mutex);
+
        /* Update events/<system>/<event> */
        if (WARN_ON_ONCE(level > 3))
                return;
@@ -886,3 +888,15 @@ void eventfs_remove_events_dir(struct eventfs_inode *ei)
        d_invalidate(dentry);
        d_make_discardable(dentry);
 }
+
+int eventfs_remount_lock(void)
+{
+       mutex_lock(&eventfs_mutex);
+       return srcu_read_lock(&eventfs_srcu);
+}
+
+void eventfs_remount_unlock(int srcu_idx)
+{
+       srcu_read_unlock(&eventfs_srcu, srcu_idx);
+       mutex_unlock(&eventfs_mutex);
+}

diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index 5602baf980f685d00f8f87ae84c670a3808f8815..1e8a78c5e996a61d9a020e1ef127fe82a6e4cf86 100644 (file)
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -313,6 +313,7 @@ static int tracefs_apply_options(struct super_block *sb, bool remount)
        struct inode *inode = d_inode(sb->s_root);
        struct tracefs_inode *ti;
        bool update_uid, update_gid;
+       int srcu_idx;
        umode_t tmp_mode;
 
        /*
@@ -337,6 +338,7 @@ static int tracefs_apply_options(struct super_block *sb, bool remount)
                update_uid = fsi->opts & BIT(Opt_uid);
                update_gid = fsi->opts & BIT(Opt_gid);
 
+               srcu_idx = eventfs_remount_lock();
                rcu_read_lock();
                list_for_each_entry_rcu(ti, &tracefs_inodes, list) {
                        if (update_uid) {
@@ -358,6 +360,7 @@ static int tracefs_apply_options(struct super_block *sb, bool remount)
                                eventfs_remount(ti, update_uid, update_gid);
                }
                rcu_read_unlock();
+               eventfs_remount_unlock(srcu_idx);
        }
 
        return 0;
@@ -403,7 +406,7 @@ static int tracefs_drop_inode(struct inode *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
+        * gets freed after an SRCU cycle, so the content will still
         * be safe if the iteration is going on now.
         */
        ti->flags &= ~TRACEFS_EVENT_INODE;

diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h
index d83c2a25f288e0fbdc2e47de9b711cddf17746c6..a4a7f8431affb0c6649e269aaa72e13d81333c2a 100644 (file)
--- a/fs/tracefs/internal.h
+++ b/fs/tracefs/internal.h
@@ -76,4 +76,7 @@ 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);
 
+int eventfs_remount_lock(void);
+void eventfs_remount_unlock(int srcu_idx);
+
 #endif /* _TRACEFS_INTERNAL_H */