Commit 07004a8c authored by David Carlier's avatar David Carlier Committed by Steven Rostedt
Browse files

eventfs: Hold eventfs_mutex and SRCU when remount walks events 

Commit 340f0c70 ("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
    d2603279.
  - 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: 340f0c70 ("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: default avatarDavid Carlier <devnexen@gmail.com>
Signed-off-by: default avatarSteven Rostedt <rostedt@goodmis.org>
parent f67950b2

fs/tracefs/event_inode.c

+14 −0
Original line number Diff line number Diff line
@@ -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);

}

fs/tracefs/inode.c

+4 −1
Original line number Diff line number Diff line
@@ -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;

fs/tracefs/internal.h

+3 −0
Original line number Diff line number Diff line
@@ -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 */