Re: [PATCH] tracefs: Remove unneeded buggy tracefs iput callback

[Steven Rostedt]

On Wed, 22 May 2024 12:45:04 -0400
Steven Rostedt  wrote:

> From: "Steven Rostedt (Google)" 
> 
> The iput callback was added because the remount could call into the
> eventfs code and touch the ei->entry_attrs array, which could have been
> freed when an eventfs directory is freed (via a synthetic event). But the
> entry_attrs was freed incorrectly and since been fixed to be freed after
> the last reference of the ei is done.
> 
> The iput clears the TRACEFS_EVENT_INODE flag of the tracefs_inode
> preventing it from calling the eventfs_remount() function. But the iput
> can be called after the last reference to the inode is done but the
> eventfs_inode still exists, causing the eventfs_remount() not to be called
> on an tracefs_inode when it should be.

Testing this more, I found that the iput is still needed, as the deletion
of the eventfs inodes can happen before the inode is released.

Will produce a v2 that handles this properly.

-- Steve

[PATCH] tracefs: Remove unneeded buggy tracefs iput callback

[Steven Rostedt]

From: "Steven Rostedt (Google)" 

The iput callback was added because the remount could call into the
eventfs code and touch the ei->entry_attrs array, which could have been
freed when an eventfs directory is freed (via a synthetic event). But the
entry_attrs was freed incorrectly and since been fixed to be freed after
the last reference of the ei is done.

The iput clears the TRACEFS_EVENT_INODE flag of the tracefs_inode
preventing it from calling the eventfs_remount() function. But the iput
can be called after the last reference to the inode is done but the
eventfs_inode still exists, causing the eventfs_remount() not to be called
on an tracefs_inode when it should be.

Link: 
https://lore.kernel.org/all/cak7lnarxgaww3kh9jgrnh4vk6fr8ldknkf3wq8nhmwjrvwj...@mail.gmail.com/

Cc: sta...@vger.kernel.org
Reported-by: Masahiro Yamada 
Fixes: ee4e0379475e4 ("eventfs: Free all of the eventfs_inode after RCU")
Signed-off-by: Steven Rostedt (Google) 
---
 fs/tracefs/inode.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index 9252e0d78ea2..62ca9c23b93c 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -455,22 +455,7 @@ static int tracefs_d_revalidate(struct dentry *dentry, 
unsigned int flags)
return !(ei && ei->is_freed);
 }
 
-static void tracefs_d_iput(struct dentry *dentry, struct inode *inode)
-{
-   struct tracefs_inode *ti = get_tracefs(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
-* be safe if the iteration is going on now.
-*/
-   ti->flags &= ~TRACEFS_EVENT_INODE;
-}
-
 static const struct dentry_operations tracefs_dentry_operations = {
-   .d_iput = tracefs_d_iput,
.d_revalidate = tracefs_d_revalidate,
.d_release = tracefs_d_release,
 };
-- 
2.43.0