linux-kernel - Re: [PATCH v3 1/2] eventfs: Have the inodes all for files and directories all be the same

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240122215930.GA6184@frogsfrogsfrogs>
Date: Mon, 22 Jan 2024 13:59:30 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Christian Brauner <brauner@...nel.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	Ajay Kaher <ajay.kaher@...adcom.com>, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] eventfs: Have the inodes all for files and
 directories all be the same

On Tue, Jan 16, 2024 at 05:55:32PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@...dmis.org>
> 
> The dentries and inodes are created in the readdir for the sole purpose of
> getting a consistent inode number. Linus stated that is unnecessary, and
> that all inodes can have the same inode number. For a virtual file system
> they are pretty meaningless.
> 
> Instead use a single unique inode number for all files and one for all
> directories.
> 
> Link: https://lore.kernel.org/all/20240116133753.2808d45e@gandalf.local.home/
> Link: https://lore.kernel.org/linux-trace-kernel/20240116211353.412180363@goodmis.org
> 
> Cc: Masami Hiramatsu <mhiramat@...nel.org>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> Cc: Christian Brauner <brauner@...nel.org>
> Cc: Al  Viro <viro@...IV.linux.org.uk>
> Cc: Ajay Kaher <ajay.kaher@...adcom.com>
> Suggested-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
>  fs/tracefs/event_inode.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
> index fdff53d5a1f8..5edf0b96758b 100644
> --- a/fs/tracefs/event_inode.c
> +++ b/fs/tracefs/event_inode.c
> @@ -32,6 +32,10 @@
>   */
>  static DEFINE_MUTEX(eventfs_mutex);
>  
> +/* Choose something "unique" ;-) */
> +#define EVENTFS_FILE_INODE_INO		0x12c4e37
> +#define EVENTFS_DIR_INODE_INO		0x134b2f5
> +
>  /*
>   * The eventfs_inode (ei) itself is protected by SRCU. It is released from
>   * its parent's list and will have is_freed set (under eventfs_mutex).
> @@ -352,6 +356,9 @@ static struct dentry *create_file(const char *name, umode_t mode,
>  	inode->i_fop = fop;
>  	inode->i_private = data;
>  
> +	/* All files will have the same inode number */
> +	inode->i_ino = EVENTFS_FILE_INODE_INO;
> +
>  	ti = get_tracefs(inode);
>  	ti->flags |= TRACEFS_EVENT_INODE;
>  	d_instantiate(dentry, inode);
> @@ -388,6 +395,9 @@ static struct dentry *create_dir(struct eventfs_inode *ei, struct dentry *parent
>  	inode->i_op = &eventfs_root_dir_inode_operations;
>  	inode->i_fop = &eventfs_file_operations;
>  
> +	/* All directories will have the same inode number */
> +	inode->i_ino = EVENTFS_DIR_INODE_INO;

Regrettably, this leads to find failing on 6.8-rc1 (see xfs/55[89] in
fstests):

# find /sys/kernel/debug/tracing/ >/dev/null
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’.
find: File system loop detected; ‘/sys/kernel/debug/tracing/events/initcall/initcall_start’ is part of the same file system loop as ‘/sys/kernel/debug/tracing/events/initcall’.
find: File system loop detected; ‘/sys/kernel/debug/tracing/events/initcall/initcall_level’ is part of the same file system loop as ‘/sys/kernel/debug/tracing/events/initcall’.

There were no such reports on 6.7.0; AFAICT find(1) is tripping over
parent and child subdirectory having the same dev/i_ino.  Changing this
line to the following:

	/* All directories will NOT have the same inode number */
	inode->i_ino = (unsigned long)inode;

makes the messages about filesystem loops go away, though I don't think
leaking raw kernel pointers is an awesome idea.

--D

> +
>  	ti = get_tracefs(inode);
>  	ti->flags |= TRACEFS_EVENT_INODE;
>  
> -- 
> 2.43.0
> 
> 
>