| Message ID | 20240122152748.46897388@gandalf.local.home (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 834bf76add3e6168038150f162cbccf1fd492a67 |
| Headers | show |
| Series | eventfs: Save directory inodes in the eventfs_inode structure | expand |
On Mon, Jan 22, 2024 at 03:27:48PM -0500, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > The eventfs inodes and directories are allocated when referenced. But this > leaves the issue of keeping consistent inode numbers and the number is > only saved in the inode structure itself. When the inode is no longer > referenced, it can be freed. When the file that the inode was representing > is referenced again, the inode is once again created, but the inode number > needs to be the same as it was before. > > Just making the inode numbers the same for all files is fine, but that > does not work with directories. The find command will check for loops via > the inode number and having the same inode number for directories triggers: > > # find /sys/kernel/tracing > find: File system loop detected; > '/sys/kernel/debug/tracing/events/initcall/initcall_finish' is part of the same file system loop as > '/sys/kernel/debug/tracing/events/initcall'. > [..] > > Linus pointed out that the eventfs_inode structure ends with a single > 32bit int, and on 64 bit machines, there's likely a 4 byte hole due to > alignment. We can use this hole to store the inode number for the > eventfs_inode. All directories in eventfs are represented by an > eventfs_inode and that data structure can hold its inode number. > > That last int was also purposely placed at the end of the structure to > prevent holes from within. Now that there's a 4 byte number to hold the > inode, both the inode number and the last integer can be moved up in the > structure for better cache locality, where the llist and rcu fields can be > moved to the end as they are only used when the eventfs_inode is being > deleted. > > Link: https://lore.kernel.org/all/CAMuHMdXKiorg-jiuKoZpfZyDJ3Ynrfb8=X+c7x0Eewxn-YRdCA@mail.gmail.com/ > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Fixes: 53c41052ba31 ("eventfs: Have the inodes all for files and directories all be the same") > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> Since I reviewed the earlier patch, I will repeat here for the formal one too. :) Thanks for avoiding the hashing! Reviewed-by: Kees Cook <keescook@chromium.org>
diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 6795fda2af19..6b211522a13e 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -34,7 +34,15 @@ static DEFINE_MUTEX(eventfs_mutex); /* Choose something "unique" ;-) */ #define EVENTFS_FILE_INODE_INO 0x12c4e37 -#define EVENTFS_DIR_INODE_INO 0x134b2f5 + +/* Just try to make something consistent and unique */ +static int eventfs_dir_ino(struct eventfs_inode *ei) +{ + if (!ei->ino) + ei->ino = get_next_ino(); + + return ei->ino; +} /* * The eventfs_inode (ei) itself is protected by SRCU. It is released from @@ -396,7 +404,7 @@ static struct dentry *create_dir(struct eventfs_inode *ei, struct dentry *parent inode->i_fop = &eventfs_file_operations; /* All directories will have the same inode number */ - inode->i_ino = EVENTFS_DIR_INODE_INO; + inode->i_ino = eventfs_dir_ino(ei); ti = get_tracefs(inode); ti->flags |= TRACEFS_EVENT_INODE; @@ -802,7 +810,7 @@ static int eventfs_iterate(struct file *file, struct dir_context *ctx) name = ei_child->name; - ino = EVENTFS_DIR_INODE_INO; + ino = eventfs_dir_ino(ei_child); if (!dir_emit(ctx, name, strlen(name), ino, DT_DIR)) goto out_dec; diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h index 12b7d0150ae9..45397df9bb65 100644 --- a/fs/tracefs/internal.h +++ b/fs/tracefs/internal.h @@ -55,6 +55,10 @@ struct eventfs_inode { struct eventfs_attr *entry_attrs; struct eventfs_attr attr; void *data; + unsigned int is_freed:1; + unsigned int is_events:1; + unsigned int nr_entries:30; + unsigned int ino; /* * Union - used for deletion * @llist: for calling dput() if needed after RCU @@ -64,9 +68,6 @@ struct eventfs_inode { struct llist_node llist; struct rcu_head rcu; }; - unsigned int is_freed:1; - unsigned int is_events:1; - unsigned int nr_entries:30; }; static inline struct tracefs_inode *get_tracefs(const struct inode *inode)