eventfs: Use children field for rcu head and add memory barriers

When an eventfs inode is freed, it sets ei->is_freed and then uses its
ei->list to add it to the srcu link list as the list field is a union with
the rcu list head. As the ei->list is used to iterate over an SRCU
protected list without taking the eventfs_mutex, there's nothing stopping
the iteration over that list to see the ei->rcu instead of the ei->list
and it will read a corrupt target.

To fix this, change the union of the rcu list head with the children list.
On freeing the eventfs inode, set the is_free and execute a smp_wmb()
before adding the eventfs inode to the SRCU list.

On iteration of the ei->children list, at the start, execute a smp_rmb()
and then read the is_freed of the ei to see if the children list is still
valid. If is_freed is set, then the ei_child read is not valid and the
loop should exit immediately.

Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260808094215.4252430d@robin
Fixes: 704f960dbee2f ("eventfs: Read ei->entries before ei->children in eventfs_iterate()")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260806022719.375354-1-shuangpeng.kernel%40gmail.com
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 93bc4f83b73e3fbadb1d6445f2ccafc3a3697af1..a524584353279fa209893cab8845ef5affac5367 100644 (file)
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -124,7 +124,17 @@ static inline void put_ei(struct eventfs_inode *ei)
 static inline void free_ei(struct eventfs_inode *ei)
 {
        if (ei) {
+               /* The ei should have no children if it is being freed. */
+               WARN_ON_ONCE(!list_empty(&ei->children));
                ei->is_freed = 1;
+               /*
+                * The SRCU iteration has a smp_rmb() to make sure it
+                * sees a child (that may have already been freed)
+                * before it reads is_free. If is_free is set, it must
+                * not use the child it acquired from ei->children, as
+                * the list may be used for SRCU.
+                */
+               smp_wmb();
                put_ei(ei);
        }
 }
@@ -627,6 +637,20 @@ static int eventfs_iterate(struct file *file, struct dir_context *ctx)
        list_for_each_entry_srcu(ei_child, &ei->children, list,
                                 srcu_read_lock_held(&eventfs_srcu)) {
 
+               /*
+                * If the ei is being freed, then the ei->children may be
+                * being used as the rcu list, which means the next element
+                * may be garbage. The ei->is_free is set before switching
+                * the ei->children over to ei->rcu. The read memory barrier
+                * here makes sure the ei_child is read before is_free is
+                * updated.
+                *
+                * Matches the smp_wmb() in free_ei()
+                */
+               smp_rmb();
+               if (ei->is_freed)
+                       return -EINVAL;
+
                if (c > 0) {
                        c--;
                        continue;

diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h
index a4a7f8431affb0c6649e269aaa72e13d81333c2a..c61481d04c8eebd77d707b4c52582c0c4e076e4e 100644 (file)
--- a/fs/tracefs/internal.h
+++ b/fs/tracefs/internal.h
@@ -46,11 +46,11 @@ struct eventfs_attr {
  * @ino:       The saved inode number
  */
 struct eventfs_inode {
+       struct list_head        list;
        union {
-               struct list_head        list;
+               struct list_head        children;
                struct rcu_head         rcu;
        };
-       struct list_head                children;
        const struct eventfs_entry      *entries;
        const char                      *name;
        struct eventfs_attr             *entry_attrs;