eventfs: Hold eventfs_mutex and SRCU when remount walks events
Subsystems (2)
PATCH 1/1 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
doneWrap 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.
--- fs/tracefs/event_inode.c | 14 ++++++++++++++ fs/tracefs/inode.c | 5 ++++- fs/tracefs/internal.h | 3 +++ 3 files changed, 21 insertions(+), 1 deletion(-)
fs/tracefs/event_inode.c +14 -0
@@ -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
@@ -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
@@ -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 */
Signature · 1 line
2.53.0