Re: [PATCH] eventfs: Initialize ei->children and ei->list in init_ei()
Steven Rostedt <[email protected]>
| Newsgroups | org.kernel.vger.stable,org.kernel.vger.linux-kernel,org.kernel.vger.linux-trace-kernel |
|---|---|
| Message-ID | <20260824110345.22c1cfa1@fedora> |
On Mon, 24 Aug 2026 20:16:53 +0530 Deepanshu Kartikey <[email protected]> wrote: Yeah, I saw the syzbot report and came up immediately with this fix as well. But the change log is way too verbose for such a simple fix. Did you use AI for this patch? If so, you must divulge that information, usually with a tag. > eventfs_create_events_dir() allocates the eventfs_inode via > alloc_root_ei(), but only calls INIT_LIST_HEAD() on ei->children > and ei->list after the tracefs_get_inode() check. If that check > fails, the code jumps to the fail label and calls cleanup_ei(), > which calls free_ei(): > > WARN_ON_ONCE(!list_empty(&ei->children)); > > Since ei was allocated with kzalloc(), ei->children.next is NULL > at this point, not a self-referencing pointer. list_empty() checks > head->next == head, so it returns false on an uninitialized list > head, triggering a false-positive WARN_ON_ONCE() even though the > list was never used. > > eventfs_create_dir() has the same latent issue: alloc_ei() is > called before INIT_LIST_HEAD(), leaving a window where an early > failure path could hit cleanup_ei() on an uninitialized list head. > > Move the INIT_LIST_HEAD() calls into init_ei(), which is called > by both alloc_ei() and alloc_root_ei() immediately after > allocation. This guarantees every eventfs_inode has a valid, > self-linked, empty children/list the moment it is allocated, > regardless of which failure path runs afterward. The change log only needs to say: eventfs_create_dir() allocates the eventfs_inode and initializes it with init_ei(). But this does not initialize the eventfs_inode list_heads. If the eventfs_create_dir() fails due to memory pressure, it will call free_ei() which checks to make sure the eventfs_inode has no children. But because the list wasn't initialized, it will give a false warning. Fix it by moving the list initialization into init_ei(). See, much better. Right to the point without all the AI slop. I'll take your patch, but I'm replacing the commit log with the above. -- Steve > > Fixes: 5790b1fb3d67 ("eventfs: Remove eventfs_file and just use eventfs_inode") > Reported-by: [email protected] > Closes: https://syzkaller.appspot.com/bug?extid=3ef80b4ed02226d04a06 > Cc: [email protected] > Signed-off-by: Deepanshu Kartikey <[email protected]> > --- > fs/tracefs/event_inode.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c > index 604ba3e841d2..6e3513b13cfa 100644 > --- a/fs/tracefs/event_inode.c > +++ b/fs/tracefs/event_inode.c > @@ -438,6 +438,8 @@ static inline struct eventfs_inode *init_ei(struct eventfs_inode *ei, const char > if (!ei->name) > return NULL; > kref_init(&ei->kref); > + INIT_LIST_HEAD(&ei->children); > + INIT_LIST_HEAD(&ei->list); > return ei; > } > > @@ -729,8 +731,6 @@ struct eventfs_inode *eventfs_create_dir(const char *name, struct eventfs_inode > ei->entries = entries; > ei->nr_entries = size; > ei->data = data; > - INIT_LIST_HEAD(&ei->children); > - INIT_LIST_HEAD(&ei->list); > > scoped_guard(mutex, &eventfs_mutex) { > if (!parent->is_freed) > @@ -802,9 +802,6 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry > ei->attr.uid = uid; > ei->attr.gid = gid; > > - INIT_LIST_HEAD(&ei->children); > - INIT_LIST_HEAD(&ei->list); > - > ti = get_tracefs(inode); > ti->flags |= TRACEFS_EVENT_INODE; > ti->private = ei;