Re: [PATCH 2/6] eventfsfs: initialize the tracefs inode properly
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2024-01-30 19:47:51
Also in:
lkml
On Tue, 30 Jan 2024 11:03:51 -0800 Linus Torvalds [off-list ref] wrote:
The tracefs-specific fields in the inode were not initialized before the inode was exposed to others through the dentry with 'd_instantiate()'. And the ->flags file was initialized incorrectly with a '|=', when the old value was stale. It should have just been a straight assignment.
The ti is allocated from fs/tracefs/inode.c that has:
static struct inode *tracefs_alloc_inode(struct super_block *sb)
{
struct tracefs_inode *ti;
ti = kmem_cache_alloc(tracefs_inode_cachep, GFP_KERNEL);
if (!ti)
return NULL;
ti->flags = 0;
return &ti->vfs_inode;
}
Shouldn't that make it valid?
Granted, the eventfs inodes don't have any of the tracefs inode flags set.
But I purposely made he ti->flags initialized to zero.
Is this update really necessary?
Or do I need to make sure that the iput() clears it?
The flags are used by tracefs, so I want to know if there's not a bug there.
-- Steve
quoted hunk ↗ jump to hunk
Move the field initializations up to before the d_instantiate, and fix the use of uninitialized data. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- fs/tracefs/event_inode.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 2d128bedd654..c0d977e6c0f2 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c@@ -328,7 +328,9 @@ static struct dentry *create_file(const char *name, umode_t mode, inode->i_ino = EVENTFS_FILE_INODE_INO; ti = get_tracefs(inode); - ti->flags |= TRACEFS_EVENT_INODE; + ti->flags = TRACEFS_EVENT_INODE; + ti->private = NULL; // Directories have 'ei', files not + d_instantiate(dentry, inode); fsnotify_create(dentry->d_parent->d_inode, dentry); return eventfs_end_creating(dentry);@@ -367,7 +369,8 @@ static struct dentry *create_dir(struct eventfs_inode *ei, struct dentry *parent inode->i_ino = eventfs_dir_ino(ei); ti = get_tracefs(inode); - ti->flags |= TRACEFS_EVENT_INODE; + ti->flags = TRACEFS_EVENT_INODE; + ti->private = ei; inc_nlink(inode); d_instantiate(dentry, inode);@@ -513,7 +516,6 @@ create_file_dentry(struct eventfs_inode *ei, int idx, static void eventfs_post_create_dir(struct eventfs_inode *ei) { struct eventfs_inode *ei_child; - struct tracefs_inode *ti; lockdep_assert_held(&eventfs_mutex);@@ -523,9 +525,6 @@ static void eventfs_post_create_dir(struct eventfs_inode *ei) srcu_read_lock_held(&eventfs_srcu)) { ei_child->d_parent = ei->dentry; } - - ti = get_tracefs(ei->dentry->d_inode); - ti->private = ei; } /**@@ -943,7 +942,7 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry INIT_LIST_HEAD(&ei->list); ti = get_tracefs(inode); - ti->flags |= TRACEFS_EVENT_INODE | TRACEFS_EVENT_TOP_INODE; + ti->flags = TRACEFS_EVENT_INODE | TRACEFS_EVENT_TOP_INODE; ti->private = ei; inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;