linux-kernel - Re: [for-linus][PATCH 1/3] 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]

Date: Mon, 22 Jan 2024 14:44:43 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Geert Uytterhoeven <geert@...ux-m68k.org>, Kees Cook
 <keescook@...omium.org>, linux-kernel@...r.kernel.org, Masami Hiramatsu
 <mhiramat@...nel.org>, Mark Rutland <mark.rutland@....com>, Mathieu
 Desnoyers <mathieu.desnoyers@...icios.com>, Andrew Morton
 <akpm@...ux-foundation.org>, Christian Brauner <brauner@...nel.org>, Al
 Viro <viro@...iv.linux.org.uk>, Ajay Kaher <ajay.kaher@...adcom.com>
Subject: Re: [for-linus][PATCH 1/3] eventfs: Have the inodes all for files
 and directories all be the same

On Mon, 22 Jan 2024 10:19:12 -0800
Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Mon, 22 Jan 2024 at 09:39, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > Actually, why not juist add an inode number to your data structures,
> > at least for directories? And just do a static increment on it as they
> > get registered?
> >
> > That avoids the whole issue with possibly leaking kernel address data.  
> 
> The 'nlink = 1' thing doesn't seem to make 'find' any happier for this
> case, sadly.
> 
> But the inode number in the 'struct eventfs_inode' looks trivial. And
> doesn't even grow that structure on 64-bit architectures at least,
> because the struct is already 64-bit aligned, and had only one 32-bit
> entry at the end.
> 
> On 32-bit architectures the structure size grows, but I'm not sure the
> allocation size grows. Our kmalloc() is quantized at odd numbers.
> 
> IOW, this trivial patch seems to be much safer than worrying about
> some pointer exposure.

I originally wanted to avoid the addition of the 4 bytes, but your comment
about it not making a difference on 64bit due to alignment makes sense.

Slightly different version below.

-- Steve

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..1a574d306ea9 100644
--- a/fs/tracefs/internal.h
+++ b/fs/tracefs/internal.h
@@ -64,6 +64,7 @@ struct eventfs_inode {
 		struct llist_node	llist;
 		struct rcu_head		rcu;
 	};
+	unsigned int			ino;
 	unsigned int			is_freed:1;
 	unsigned int			is_events:1;
 	unsigned int			nr_entries:30;