All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] tracefs: Add read-only eventfs filesystem at /sys/kernel/events
@ 2026-11-02 20:27 Steven Rostedt
  2026-11-02 20:44 ` sashiko-bot
  2026-11-03  2:18 ` Masami Hiramatsu (Google)
  0 siblings, 2 replies; 5+ messages in thread
From: Steven Rostedt @ 2026-11-02 20:27 UTC (permalink / raw)
  To: LKML, Linux Trace Kernel
  Cc: Masami Hiramatsu, Mathieu Desnoyers, Anubhav Shelat

From: Anubhav Shelat <ashelat@redhat.com>

Introduce a new read-only pseudo-filesystem "eventfs" mounted at
/sys/kernel/events that exposes trace event format and id files
(mode 0444) to unprivileged users. This allows tools like perf to
discover event formats without requiring access to the full
tracefs/debugfs mount.

The eventfs file system reuses the existing eventfs_inode lazy-lookup
infrastructure. A new set of super_operations
(eventfs_ro_super_operations) shares the tracefs inode allocator so
that eventfs_get_inode() and get_tracefs() work on the RO superblock.
The superblock is manually initialized to ensure the root inode is
allocated with tracefs_alloc_inode, allowing the root to serve
directly as the events directory without another events subdirectory.

Each qualifying event gets a subsystem directory containing format
and id files. The top-level events directory also exposes header_page
and header_event. Similar to tracefs, a change will need to be made in
systemd to mount this filesystem automatically.

[
  Steven Rostedt rewrote to use the lookup functions to determine read
  only files to show. The struct eventfs_entry now has a "readonly" field
  and if it is set, it will be displayed in the read only file system.
  The original code [1] duplicated the eventfs_inodes for every event and
  subsystem. The new approach uses the existing eventfs_inodes (without
  requiring more memory to store them) and uses the eventfs_entry arrays to
  show which files are allowed to be read readonly or not.

  [1] https://patch.msgid.link/20260715135231.338535-4-ashelat@redhat.com
]

Assisted-by: CLAUDE:claude-opus-4 Apogee
Signed-off-by: Anubhav Shelat <ashelat@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
Changes since v3: https://patch.msgid.link/20260810160708.3460a2fd@gandalf.local.home

- Fixed kernel doc name for eventfs_create_events_ro_copy() (Masami Hiramatsu)

- Removed duplicate initialization of ei in that function. (Masami Hiramatsu)

- Added include of linux/magic.h as it failed to build on some
  architectures. (kernel test robot)

 fs/tracefs/event_inode.c    | 192 ++++++++++++++++++++++++++++++++++--
 fs/tracefs/inode.c          |  21 +++-
 fs/tracefs/internal.h       |   6 ++
 include/linux/tracefs.h     |   3 +
 include/uapi/linux/magic.h  |   1 +
 kernel/trace/trace_events.c |  13 +++
 6 files changed, 225 insertions(+), 11 deletions(-)

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 6457df72173d..8a4125cd0d48 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -16,14 +16,87 @@
  */
 #include <linux/fsnotify.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
 #include <linux/namei.h>
 #include <linux/workqueue.h>
 #include <linux/security.h>
 #include <linux/tracefs.h>
+#include <linux/magic.h>
 #include <linux/kref.h>
 #include <linux/delay.h>
 #include "internal.h"
 
+static struct vfsmount *eventfs_ro_mount;
+static int eventfs_ro_mount_count;
+
+static int eventfs_ro_fill_super(struct super_block *sb, struct fs_context *fc)
+{
+	struct inode *inode;
+	struct dentry *root;
+
+	sb->s_blocksize = PAGE_SIZE;
+	sb->s_blocksize_bits = PAGE_SHIFT;
+	sb->s_magic = EVENTFS_SUPER_MAGIC;
+	sb->s_op = &eventfs_ro_super_operations;
+	sb->s_time_gran = 1;
+	sb->s_flags |= SB_RDONLY;
+
+	inode = new_inode(sb);
+	if (!inode)
+		return -ENOMEM;
+
+	inode->i_ino = 1;
+	inode->i_mode = S_IFDIR | 0555;
+	simple_inode_init_ts(inode);
+	inode->i_op = &simple_dir_inode_operations;
+	inode->i_fop = &simple_dir_operations;
+	set_nlink(inode, 2);
+
+	set_default_d_op(sb, &tracefs_dentry_operations);
+
+	root = d_make_root(inode);
+	if (!root)
+		return -ENOMEM;
+
+	sb->s_root = root;
+
+	return 0;
+}
+
+static int eventfs_ro_get_tree(struct fs_context *fc)
+{
+	return get_tree_single(fc, eventfs_ro_fill_super);
+}
+
+static const struct fs_context_operations eventfs_ro_context_ops = {
+	.get_tree	= eventfs_ro_get_tree,
+};
+
+static int eventfs_ro_init_fs_context(struct fs_context *fc)
+{
+	fc->ops = &eventfs_ro_context_ops;
+	return 0;
+}
+
+struct file_system_type eventfs_ro_fs_type = {
+	.owner =	THIS_MODULE,
+	.name =		"eventfs",
+	.init_fs_context = eventfs_ro_init_fs_context,
+	.kill_sb =	kill_anon_super,
+};
+
+struct dentry *eventfs_ro_get_root(void)
+{
+	int error;
+
+	error = simple_pin_fs(&eventfs_ro_fs_type, &eventfs_ro_mount,
+			      &eventfs_ro_mount_count);
+	if (error)
+		return ERR_PTR(error);
+
+	return dget(eventfs_ro_mount->mnt_root);
+}
+
 /*
  * eventfs_mutex protects the eventfs_inode (ei) dentry. Any access
  * to the ei->dentry must be done under this mutex and after checking
@@ -169,7 +242,11 @@ static inline struct eventfs_inode *get_ei(struct eventfs_inode *ei)
 static struct dentry *eventfs_root_lookup(struct inode *dir,
 					  struct dentry *dentry,
 					  unsigned int flags);
+static struct dentry *eventfs_root_lookup_ro(struct inode *dir,
+					     struct dentry *dentry,
+					     unsigned int flags);
 static int eventfs_iterate(struct file *file, struct dir_context *ctx);
+static int eventfs_ro_iterate(struct file *file, struct dir_context *ctx);
 
 static void update_attr(struct eventfs_attr *attr, struct iattr *iattr)
 {
@@ -247,6 +324,10 @@ static const struct inode_operations eventfs_dir_inode_operations = {
 	.setattr	= eventfs_set_attr,
 };
 
+static const struct inode_operations eventfs_ro_dir_inode_operations = {
+	.lookup		= eventfs_root_lookup_ro,
+};
+
 static const struct inode_operations eventfs_file_inode_operations = {
 	.setattr	= eventfs_set_attr,
 };
@@ -257,6 +338,12 @@ static const struct file_operations eventfs_file_operations = {
 	.llseek		= generic_file_llseek,
 };
 
+static const struct file_operations eventfs_ro_file_operations = {
+	.read		= generic_read_dir,
+	.iterate_shared	= eventfs_ro_iterate,
+	.llseek		= generic_file_llseek,
+};
+
 static void eventfs_set_attrs(struct eventfs_inode *ei, bool update_uid, kuid_t uid,
 			      bool update_gid, kgid_t gid, int level)
 {
@@ -420,7 +507,7 @@ static struct dentry *lookup_file(struct eventfs_inode *parent_ei,
  * a eventfs_inode.
  */
 static struct dentry *lookup_dir_entry(struct dentry *dentry,
-	struct eventfs_inode *pei, struct eventfs_inode *ei)
+	struct eventfs_inode *pei, struct eventfs_inode *ei, bool ro)
 {
 	struct inode *inode;
 	umode_t mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
@@ -429,8 +516,13 @@ static struct dentry *lookup_dir_entry(struct dentry *dentry,
 	if (unlikely(!inode))
 		return ERR_PTR(-ENOMEM);
 
-	inode->i_op = &eventfs_dir_inode_operations;
-	inode->i_fop = &eventfs_file_operations;
+	if (ro) {
+		inode->i_op = &eventfs_ro_dir_inode_operations;
+		inode->i_fop = &eventfs_ro_file_operations;
+	} else {
+		inode->i_op = &eventfs_dir_inode_operations;
+		inode->i_fop = &eventfs_file_operations;
+	}
 
 	/* All directories will have the same inode number */
 	inode->i_ino = eventfs_dir_ino(ei);
@@ -534,9 +626,10 @@ lookup_file_dentry(struct dentry *dentry,
  * list, if @dentry found go ahead and create the file/dir
  */
 
-static struct dentry *eventfs_root_lookup(struct inode *dir,
-					  struct dentry *dentry,
-					  unsigned int flags)
+static struct dentry *__eventfs_root_lookup(struct inode *dir,
+					    struct dentry *dentry,
+					    unsigned int flags,
+					    bool ro)
 {
 	struct eventfs_inode *ei_child;
 	struct tracefs_inode *ti;
@@ -559,7 +652,7 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
 		/* A child is freed and removed from the list at the same time */
 		if (WARN_ON_ONCE(ei_child->is_freed))
 			return NULL;
-		return lookup_dir_entry(dentry, ei, ei_child);
+		return lookup_dir_entry(dentry, ei, ei_child, ro);
 	}
 
 	for (int i = 0; i < ei->nr_entries; i++) {
@@ -571,20 +664,44 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
 		if (strcmp(name, entry->name) != 0)
 			continue;
 
+		if (ro && !entry->read_only)
+			return NULL;
+
 		data = ei->data;
 		if (entry->callback(name, &mode, &data, &fops) <= 0)
 			return NULL;
 
+		/*
+		 * Read-only files gets all users read access,
+		 * but also should make sure this is not writable.
+		 */
+		if (ro)
+			mode |= (mode & ~0222) | 0444;
+
 		return lookup_file_dentry(dentry, ei, i, mode, data, fops);
 
 	}
 	return NULL;
 }
 
+static struct dentry *eventfs_root_lookup(struct inode *dir,
+					  struct dentry *dentry,
+					  unsigned int flags)
+{
+	return __eventfs_root_lookup(dir, dentry, flags, false);
+}
+
+static struct dentry *eventfs_root_lookup_ro(struct inode *dir,
+					     struct dentry *dentry,
+					     unsigned int flags)
+{
+	return __eventfs_root_lookup(dir, dentry, flags, true);
+}
+
 /*
  * Walk the children of a eventfs_inode to fill in getdents().
  */
-static int eventfs_iterate(struct file *file, struct dir_context *ctx)
+static int __eventfs_iterate(struct file *file, struct dir_context *ctx, bool ro)
 {
 	const struct file_operations *fops;
 	struct inode *f_inode = file_inode(file);
@@ -630,6 +747,9 @@ static int eventfs_iterate(struct file *file, struct dir_context *ctx)
 		entry = &ei->entries[i];
 		name = entry->name;
 
+		if (ro && !entry->read_only)
+			continue;
+
 		/* If ei->is_freed then just bail here, nothing more to do */
 		scoped_guard(mutex, &eventfs_mutex) {
 			if (ei->is_freed)
@@ -688,6 +808,16 @@ static int eventfs_iterate(struct file *file, struct dir_context *ctx)
 	return 1;
 }
 
+static int eventfs_iterate(struct file *file, struct dir_context *ctx)
+{
+	return __eventfs_iterate(file, ctx, false);
+}
+
+static int eventfs_ro_iterate(struct file *file, struct dir_context *ctx)
+{
+	return __eventfs_iterate(file, ctx, true);
+}
+
 /**
  * eventfs_create_dir - Create the eventfs_inode for this directory
  * @name: The name of the directory to create.
@@ -845,6 +975,52 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry
 	return ERR_PTR(-ENOMEM);
 }
 
+/**
+ * eventfs_create_events_ro_copy - create a read-only events directory
+ * @name: The name of the top level directory to create.
+ * @entries: A list of entries that represent the files under this directory
+ * @size: The number of @entries
+ * @data: The default data to pass to the files (an entry may override it).
+ *
+ * This function configures the eventfs filesystem root as a read-only
+ * trace event directory using the existing eventfs_inode lazy-lookup
+ * infrastructure.
+ *
+ * See eventfs_create_dir() for use of @entries.
+ */
+int eventfs_create_events_ro_copy(const char *name, struct eventfs_inode *ei)
+{
+	static struct dentry *dentry;
+	struct tracefs_inode *ti;
+	struct inode *inode;
+
+	/* Can only be called once. */
+	if (dentry)
+		return -EBUSY;
+
+	/* Reference acquired but never freed */
+	dentry = eventfs_ro_get_root();
+	if (IS_ERR(dentry))
+		return PTR_ERR(dentry);
+
+	inode = d_inode(dentry);
+
+	INIT_LIST_HEAD(&ei->children);
+	INIT_LIST_HEAD(&ei->list);
+
+	ti = get_tracefs(inode);
+	ti->flags |= TRACEFS_EVENT_INODE;
+	ti->private = ei;
+
+	inode->i_op = &eventfs_ro_dir_inode_operations;
+	inode->i_fop = &eventfs_ro_file_operations;
+
+	/* This is never freed */
+	dentry->d_fsdata = get_ei(ei);
+
+	return 0;
+}
+
 /**
  * eventfs_remove_rec - remove eventfs dir or file from list
  * @ei: eventfs_inode to be removed.
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index 5020365ca704..555af5402adc 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -423,6 +423,14 @@ static const struct super_operations tracefs_super_operations = {
 	.show_options	= tracefs_show_options,
 };
 
+const struct super_operations eventfs_ro_super_operations = {
+	.alloc_inode    = tracefs_alloc_inode,
+	.free_inode     = tracefs_free_inode,
+	.destroy_inode  = tracefs_destroy_inode,
+	.drop_inode     = tracefs_drop_inode,
+	.statfs		= simple_statfs,
+};
+
 /*
  * It would be cleaner if eventfs had its own dentry ops.
  *
@@ -455,7 +463,7 @@ static int tracefs_d_delete(const struct dentry *dentry)
 	return dentry->d_fsdata == NULL;
 }
 
-static const struct dentry_operations tracefs_dentry_operations = {
+const struct dentry_operations tracefs_dentry_operations = {
 	.d_revalidate = tracefs_d_revalidate,
 	.d_release = tracefs_d_release,
 	.d_delete = tracefs_d_delete,
@@ -801,8 +809,15 @@ static int __init tracefs_init(void)
 		return -EINVAL;
 
 	retval = register_filesystem(&trace_fs_type);
-	if (!retval)
-		tracefs_registered = true;
+	if (retval)
+		return retval;
+	tracefs_registered = true;
+
+	retval = sysfs_create_mount_point(kernel_kobj, "events");
+	if (retval)
+		return retval;
+
+	retval = register_filesystem(&eventfs_ro_fs_type);
 
 	return retval;
 }
diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h
index c61481d04c8e..c92376bc3121 100644
--- a/fs/tracefs/internal.h
+++ b/fs/tracefs/internal.h
@@ -2,6 +2,10 @@
 #ifndef _TRACEFS_INTERNAL_H
 #define _TRACEFS_INTERNAL_H
 
+extern const struct dentry_operations tracefs_dentry_operations;
+extern const struct super_operations eventfs_ro_super_operations;
+extern struct file_system_type eventfs_ro_fs_type;
+
 enum {
 	TRACEFS_EVENT_INODE		= BIT(1),
 	TRACEFS_GID_PERM_SET		= BIT(2),
@@ -73,6 +77,8 @@ struct dentry *tracefs_end_creating(struct dentry *dentry);
 struct dentry *tracefs_failed_creating(struct dentry *dentry);
 struct inode *tracefs_get_inode(struct super_block *sb);
 
+struct dentry *eventfs_ro_get_root(void);
+
 void eventfs_remount(struct tracefs_inode *ti, bool update_uid, bool update_gid);
 void eventfs_d_release(struct dentry *dentry);
 
diff --git a/include/linux/tracefs.h b/include/linux/tracefs.h
index bc354d340046..41e97a6bd69a 100644
--- a/include/linux/tracefs.h
+++ b/include/linux/tracefs.h
@@ -75,6 +75,7 @@ struct eventfs_entry {
 	const char			*name;
 	eventfs_callback		callback;
 	eventfs_release			release;
+	bool				read_only;
 };
 
 struct eventfs_inode;
@@ -87,6 +88,8 @@ struct eventfs_inode *eventfs_create_dir(const char *name, struct eventfs_inode
 					 const struct eventfs_entry *entries,
 					 int size, void *data);
 
+int eventfs_create_events_ro_copy(const char *name, struct eventfs_inode *ei);
+
 void eventfs_remove_events_dir(struct eventfs_inode *ei);
 void eventfs_remove_dir(struct eventfs_inode *ei);
 
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index 66a91c8b1cb9..521916f64dee 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -76,6 +76,7 @@
 #define STACK_END_MAGIC		0x57AC6E9D
 
 #define TRACEFS_MAGIC          0x74726163
+#define EVENTFS_SUPER_MAGIC    0x65766673
 
 #define V9FS_MAGIC		0x01021997
 
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 49d135bc2fbf..1976920beb1a 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -3298,17 +3298,20 @@ event_create_dir(struct eventfs_inode *parent, struct trace_event_file *file)
 		{
 			.name		= "format",
 			.callback	= event_callback,
+			.read_only	= true,
 		},
 #ifdef CONFIG_PERF_EVENTS
 		{
 			.name		= "id",
 			.callback	= event_callback,
+			.read_only	= true,
 		},
 #endif
 #ifdef CONFIG_BPF_EVENTS
 		{
 			.name		= "btf_ids",
 			.callback	= event_callback,
+			.read_only	= true,
 		},
 #endif
 #define NR_RO_EVENT_ENTRIES	(1 + IS_ENABLED(CONFIG_PERF_EVENTS) + \
@@ -4722,6 +4725,7 @@ static int events_callback(const char *name, umode_t *mode, void **data,
 static int
 create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
 {
+	static bool event_dir_ro_created;
 	struct eventfs_inode *e_events;
 	struct dentry *entry;
 	int nr_entries;
@@ -4729,10 +4733,12 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
 		{
 			.name		= "header_page",
 			.callback	= events_callback,
+			.read_only	= true,
 		},
 		{
 			.name		= "header_event",
 			.callback	= events_callback,
+			.read_only	= true,
 		},
 #define NR_RO_TOP_ENTRIES	2
 /* Readonly files must be above this line and counted by NR_RO_TOP_ENTRIES. */
@@ -4775,6 +4781,13 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
 
 	tr->event_dir = e_events;
 
+	if (!event_dir_ro_created && (tr->flags & TRACE_ARRAY_FL_GLOBAL)) {
+		int ret;
+
+		ret = eventfs_create_events_ro_copy("events", e_events);
+		if (!ret)
+			event_dir_ro_created = true;
+	}
 	return 0;
 }
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v4] tracefs: Add read-only eventfs filesystem at /sys/kernel/events
  2026-11-02 20:27 [PATCH v4] tracefs: Add read-only eventfs filesystem at /sys/kernel/events Steven Rostedt
@ 2026-11-02 20:44 ` sashiko-bot
  2026-11-02 21:04   ` Steven Rostedt
  2026-11-03  2:18 ` Masami Hiramatsu (Google)
  1 sibling, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-11-02 20:44 UTC (permalink / raw)
  To: Steven Rostedt, Linux Trace Kernel

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] tracefs: NULL dereference in eventfs_create_events_ro_copy() on lockdown
- [Medium] tracefs: bitwise OR-assignment in __eventfs_root_lookup() keeps write bits
- [Medium] tracefs: shared eventfs_attr and tracefs_inodes leak tracefs permissions
- [Medium] tracefs: missing reconfigure op on eventfs allows clearing SB_RDONLY
- [Medium] tracefs: unlocked list re-init in eventfs_create_events_ro_copy()
- [Low] tracefs: kernel-doc mismatches in eventfs_create_events_ro_copy()
--

commit c35b9718567e39dd0e81499c75ebb85b51e94ca3
Author: Anubhav Shelat <ashelat@redhat.com>

tracefs: Add read-only eventfs filesystem at /sys/kernel/events

This commit introduces a read-only eventfs pseudo-filesystem mounted at
/sys/kernel/events to expose trace event format, id, and header files to
unprivileged users by reusing the existing eventfs_inode infrastructure.

> diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
> index 6e3513b13cfa2..2a80eca136bd6 100644
> --- a/fs/tracefs/event_inode.c
> +++ b/fs/tracefs/event_inode.c
> @@ -16,14 +16,87 @@

[ ... ]

> +static int eventfs_ro_get_tree(struct fs_context *fc)
> +{
> +	return get_tree_single(fc, eventfs_ro_fill_super);
> +}
> +
> +static const struct fs_context_operations eventfs_ro_context_ops = {
> +	.get_tree	= eventfs_ro_get_tree,
> +};

[Severity: Medium]
Can a remount clear SB_RDONLY on the eventfs superblock?

eventfs_ro_fill_super() sets sb->s_flags |= SB_RDONLY, but
eventfs_ro_context_ops does not define a .reconfigure callback. In
reconfigure_super(), when fc->ops->reconfigure is NULL, VFS updates
sb->s_flags directly from fc->sb_flags and fc->sb_flags_mask, so running
mount -o remount,rw /sys/kernel/events will clear SB_RDONLY.

Because lookup_file() assigns eventfs_file_inode_operations (with
.setattr = eventfs_set_attr) to regular files in eventfs, clearing
SB_RDONLY allows chmod/chown on /sys/kernel/events files to mutate the
shared ei->entry_attrs used by tracefs. Should eventfs_ro_context_ops
provide a .reconfigure callback that rejects clearing SB_RDONLY (or should
read-only file inodes omit .setattr)?

[ ... ]

> @@ -412,7 +499,7 @@ static struct dentry *lookup_file(struct eventfs_inode *parent_ei,
>   * a eventfs_inode.
>   */
>  static struct dentry *lookup_dir_entry(struct dentry *dentry,
> -	struct eventfs_inode *pei, struct eventfs_inode *ei)
> +	struct eventfs_inode *pei, struct eventfs_inode *ei, bool ro)
>  {
>  	struct inode *inode;
>  	umode_t mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
> @@ -421,8 +508,13 @@ static struct dentry *lookup_dir_entry(struct dentry *dentry,
>  	if (unlikely(!inode))
>  		return ERR_PTR(-ENOMEM);
>  
> -	inode->i_op = &eventfs_dir_inode_operations;
> -	inode->i_fop = &eventfs_file_operations;
> +	if (ro) {
> +		inode->i_op = &eventfs_ro_dir_inode_operations;
> +		inode->i_fop = &eventfs_ro_file_operations;
> +	} else {
> +		inode->i_op = &eventfs_dir_inode_operations;
> +		inode->i_fop = &eventfs_file_operations;
> +	}

[Severity: Low]
This isn't a bug, but lookup_dir_entry() gained the @ro argument without a
matching entry in its kernel-doc comment above.

[ ... ]

> @@ -563,20 +656,44 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
>  		if (strcmp(name, entry->name) != 0)
>  			continue;
>  
> +		if (ro && !entry->read_only)
> +			return NULL;
> +
>  		data = ei->data;
>  		if (entry->callback(name, &mode, &data, &fops) <= 0)
>  			return NULL;
>  
> +		/*
> +		 * Read-only files gets all users read access,
> +		 * but also should make sure this is not writable.
> +		 */
> +		if (ro)
> +			mode |= (mode & ~0222) | 0444;

[Severity: Medium]
Should this use = instead of |= when updating mode?

Because |= ORs the right-hand side with the original value of mode,
mode | (mode & ~0222) | 0444 simplifies to mode | 0444, leaving any write
permission bits returned by entry->callback() intact.

> +
>  		return lookup_file_dentry(dentry, ei, i, mode, data, fops);

[Severity: Medium]
Will permission or ownership changes in tracefs leak into the read-only
eventfs mount through shared eventfs_attr and tracefs_inodes state?

In __eventfs_root_lookup(), both lookup_dir_entry() and lookup_file_dentry()
pass the shared &ei->attr and &ei->entry_attrs[idx] into eventfs_get_inode()
-> update_inode_attr() even when ro is true:

fs/tracefs/event_inode.c:update_inode_attr() {
	...
	if (attr && attr->mode & EVENTFS_SAVE_MODE)
		inode->i_mode = attr->mode & EVENTFS_MODE_MASK;
	else
		inode->i_mode = mode;

	if (attr && attr->mode & EVENTFS_SAVE_UID)
		inode->i_uid = attr->uid;
	else
		inode->i_uid = rei->ei.attr.uid;
	...
}

If an administrator runs chmod or chown on files or directories under
/sys/kernel/tracing/events, or mounts tracefs with uid=/gid= options,
update_inode_attr() will override the 0444 file mode (and directory
permissions/ownership) in /sys/kernel/events with the saved tracefs
attributes, potentially revoking unprivileged read access or re-enabling
write bits. In addition, lookup_dir_entry() passes S_IFDIR | S_IRWXU |
S_IRUGO | S_IXUGO (0755) instead of 0555 when ro is true.

Similarly, because eventfs_ro_super_operations uses tracefs_alloc_inode,
eventfs inodes are added to the global tracefs_inodes list. When tracefs is
remounted with uid= or gid=, tracefs_apply_options() walks tracefs_inodes
without checking ti->vfs_inode.i_sb == sb and overwrites i_uid and i_gid on
live eventfs inodes as well:

fs/tracefs/inode.c:tracefs_apply_options() {
	...
	list_for_each_entry_rcu(ti, &tracefs_inodes, list) {
		if (update_uid) {
			ti->flags &= ~TRACEFS_UID_PERM_SET;
			ti->vfs_inode.i_uid = fsi->uid;
		}
	...
}

Should ro lookups ignore ei->attr / ei->entry_attrs (or should
eventfs_get_inode() bypass saved tracefs attributes when on the read-only
superblock), and should tracefs_apply_options() filter by superblock?

[ ... ]

> @@ -837,6 +967,52 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry
>  	return ERR_PTR(-ENOMEM);
>  }
>  
> +/**
> + * eventfs_create_events_ro_copy - create a read-only events directory
> + * @name: The name of the top level directory to create.
> + * @entries: A list of entries that represent the files under this directory
> + * @size: The number of @entries
> + * @data: The default data to pass to the files (an entry may override it).
> + *
> + * This function configures the eventfs filesystem root as a read-only
> + * trace event directory using the existing eventfs_inode lazy-lookup
> + * infrastructure.
> + *
> + * See eventfs_create_dir() for use of @entries.
> + */
> +int eventfs_create_events_ro_copy(const char *name, struct eventfs_inode *ei)

[Severity: Low]
This isn't a bug, but the kernel-doc comment for
eventfs_create_events_ro_copy() still lists @entries, @size, and @data from
eventfs_create_events_dir() instead of @ei, and the @name parameter is
unused in the function body.

> +{
> +	static struct dentry *dentry;
> +	struct tracefs_inode *ti;
> +	struct inode *inode;
> +
> +	/* Can only be called once. */
> +	if (dentry)
> +		return -EBUSY;
> +
> +	/* Reference acquired but never freed */
> +	dentry = eventfs_ro_get_root();
> +	if (IS_ERR(dentry))
> +		return PTR_ERR(dentry);
> +
> +	inode = d_inode(dentry);
> +
> +	INIT_LIST_HEAD(&ei->children);
> +	INIT_LIST_HEAD(&ei->list);
> +
> +	ti = get_tracefs(inode);
> +	ti->flags |= TRACEFS_EVENT_INODE;
> +	ti->private = ei;
> +
> +	inode->i_op = &eventfs_ro_dir_inode_operations;
> +	inode->i_fop = &eventfs_ro_file_operations;
> +
> +	/* This is never freed */
> +	dentry->d_fsdata = get_ei(ei);
> +
> +	return 0;
> +}

[Severity: Medium]
Is it safe to re-initialize ei->children and ei->list here?

In create_event_toplevel_files(), e_events has already been allocated and
initialized by eventfs_create_events_dir() -> alloc_root_ei() -> init_ei(),
and published via tracefs_end_creating(). Calling
INIT_LIST_HEAD(&ei->children) and INIT_LIST_HEAD(&ei->list) again without
holding eventfs_mutex risks resetting an already-live list.

Also, should dentry->d_fsdata and ti->private be set before setting
ti->flags |= TRACEFS_EVENT_INODE and switching inode->i_op to
eventfs_ro_dir_inode_operations, and should dentry be reset to NULL (or only
assigned after checking IS_ERR) if eventfs_ro_get_root() fails so static
dentry does not hold an ERR_PTR?

[ ... ]

> diff --git a/include/linux/tracefs.h b/include/linux/tracefs.h
> index bc354d3400464..41e97a6bd69a3 100644
> --- a/include/linux/tracefs.h
> +++ b/include/linux/tracefs.h
> @@ -75,6 +75,7 @@ struct eventfs_entry {
>  	const char			*name;
>  	eventfs_callback		callback;
>  	eventfs_release			release;
> +	bool				read_only;
>  };

[Severity: Low]
This isn't a bug, but struct eventfs_entry has a kernel-doc comment just
above this definition where @read_only (and @release) are not documented.

[ ... ]

> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 6afed2091e19c..968fdbca8054f 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c

[ ... ]

> @@ -4775,6 +4781,13 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
>  
>  	tr->event_dir = e_events;
>  
> +	if (!event_dir_ro_created && (tr->flags & TRACE_ARRAY_FL_GLOBAL)) {
> +		int ret;
> +
> +		ret = eventfs_create_events_ro_copy("events", e_events);
> +		if (!ret)
> +			event_dir_ro_created = true;
> +	}
>  	return 0;
>  }

[Severity: High]
What happens here when the kernel is booted with lockdown=confidentiality
(LOCKDOWN_TRACEFS)?

In eventfs_create_events_dir():

fs/tracefs/event_inode.c:eventfs_create_events_dir() {
	...
	if (security_locked_down(LOCKDOWN_TRACEFS))
		return NULL;
	...
}

When LOCKDOWN_TRACEFS is active, eventfs_create_events_dir() returns NULL
rather than an ERR_PTR. In create_event_toplevel_files(), IS_ERR(e_events)
evaluates to false for NULL, so e_events = NULL is passed to
eventfs_create_events_ro_copy("events", e_events), which dereferences ei
without a NULL check and crashes the kernel during boot.

Should create_event_toplevel_files() or eventfs_create_events_ro_copy()
check for !e_events (and security_locked_down(LOCKDOWN_TRACEFS)) before
dereferencing ei?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102152748.614ddc8f@gandalf.local.home?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4] tracefs: Add read-only eventfs filesystem at /sys/kernel/events
  2026-11-02 20:44 ` sashiko-bot
@ 2026-11-02 21:04   ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2026-11-02 21:04 UTC (permalink / raw)
  To: sashiko-bot; +Cc: sashiko-reviews, Linux Trace Kernel

On Mon, 02 Nov 2026 20:44:51 +0000
sashiko-bot@kernel.org wrote:


> 
> > +static int eventfs_ro_get_tree(struct fs_context *fc)
> > +{
> > +	return get_tree_single(fc, eventfs_ro_fill_super);
> > +}
> > +
> > +static const struct fs_context_operations eventfs_ro_context_ops = {
> > +	.get_tree	= eventfs_ro_get_tree,
> > +};  
> 
> [Severity: Medium]
> Can a remount clear SB_RDONLY on the eventfs superblock?
> 
> eventfs_ro_fill_super() sets sb->s_flags |= SB_RDONLY, but
> eventfs_ro_context_ops does not define a .reconfigure callback. In
> reconfigure_super(), when fc->ops->reconfigure is NULL, VFS updates
> sb->s_flags directly from fc->sb_flags and fc->sb_flags_mask, so running
> mount -o remount,rw /sys/kernel/events will clear SB_RDONLY.
> 
> Because lookup_file() assigns eventfs_file_inode_operations (with
> .setattr = eventfs_set_attr) to regular files in eventfs, clearing
> SB_RDONLY allows chmod/chown on /sys/kernel/events files to mutate the
> shared ei->entry_attrs used by tracefs. Should eventfs_ro_context_ops
> provide a .reconfigure callback that rejects clearing SB_RDONLY (or should
> read-only file inodes omit .setattr)?

Ha! I wanted to handle this but wasn't sure how. Thank you Sashiko for
telling me about the .reconfigure() method that lets me prevent the
SB_RDONLY from being changed.


> 
> [ ... ]
> 
> > @@ -412,7 +499,7 @@ static struct dentry *lookup_file(struct eventfs_inode *parent_ei,
> >   * a eventfs_inode.
> >   */
> >  static struct dentry *lookup_dir_entry(struct dentry *dentry,
> > -	struct eventfs_inode *pei, struct eventfs_inode *ei)
> > +	struct eventfs_inode *pei, struct eventfs_inode *ei, bool ro)
> >  {
> >  	struct inode *inode;
> >  	umode_t mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
> > @@ -421,8 +508,13 @@ static struct dentry *lookup_dir_entry(struct dentry *dentry,
> >  	if (unlikely(!inode))
> >  		return ERR_PTR(-ENOMEM);
> >  
> > -	inode->i_op = &eventfs_dir_inode_operations;
> > -	inode->i_fop = &eventfs_file_operations;
> > +	if (ro) {
> > +		inode->i_op = &eventfs_ro_dir_inode_operations;
> > +		inode->i_fop = &eventfs_ro_file_operations;
> > +	} else {
> > +		inode->i_op = &eventfs_dir_inode_operations;
> > +		inode->i_fop = &eventfs_file_operations;
> > +	}  
> 
> [Severity: Low]
> This isn't a bug, but lookup_dir_entry() gained the @ro argument without a
> matching entry in its kernel-doc comment above.

Oops, I'll fix that in v5.

> 
> [ ... ]
> 
> > @@ -563,20 +656,44 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
> >  		if (strcmp(name, entry->name) != 0)
> >  			continue;
> >  
> > +		if (ro && !entry->read_only)
> > +			return NULL;
> > +
> >  		data = ei->data;
> >  		if (entry->callback(name, &mode, &data, &fops) <= 0)
> >  			return NULL;
> >  
> > +		/*
> > +		 * Read-only files gets all users read access,
> > +		 * but also should make sure this is not writable.
> > +		 */
> > +		if (ro)
> > +			mode |= (mode & ~0222) | 0444;  
> 
> [Severity: Medium]
> Should this use = instead of |= when updating mode?

Oops! Duh. Yeah that was a cut-and-paste bug.

> 
> Because |= ORs the right-hand side with the original value of mode,
> mode | (mode & ~0222) | 0444 simplifies to mode | 0444, leaving any write
> permission bits returned by entry->callback() intact.
> 
> > +
> >  		return lookup_file_dentry(dentry, ei, i, mode, data, fops);  
> 
> [Severity: Medium]
> Will permission or ownership changes in tracefs leak into the read-only
> eventfs mount through shared eventfs_attr and tracefs_inodes state?
> 
> In __eventfs_root_lookup(), both lookup_dir_entry() and lookup_file_dentry()
> pass the shared &ei->attr and &ei->entry_attrs[idx] into eventfs_get_inode()
> -> update_inode_attr() even when ro is true:  
> 
> fs/tracefs/event_inode.c:update_inode_attr() {
> 	...
> 	if (attr && attr->mode & EVENTFS_SAVE_MODE)
> 		inode->i_mode = attr->mode & EVENTFS_MODE_MASK;
> 	else
> 		inode->i_mode = mode;
> 
> 	if (attr && attr->mode & EVENTFS_SAVE_UID)
> 		inode->i_uid = attr->uid;
> 	else
> 		inode->i_uid = rei->ei.attr.uid;
> 	...
> }
> 
> If an administrator runs chmod or chown on files or directories under
> /sys/kernel/tracing/events, or mounts tracefs with uid=/gid= options,
> update_inode_attr() will override the 0444 file mode (and directory
> permissions/ownership) in /sys/kernel/events with the saved tracefs
> attributes, potentially revoking unprivileged read access or re-enabling
> write bits. In addition, lookup_dir_entry() passes S_IFDIR | S_IRWXU |
> S_IRUGO | S_IXUGO (0755) instead of 0555 when ro is true.
> 
> Similarly, because eventfs_ro_super_operations uses tracefs_alloc_inode,
> eventfs inodes are added to the global tracefs_inodes list. When tracefs is
> remounted with uid= or gid=, tracefs_apply_options() walks tracefs_inodes
> without checking ti->vfs_inode.i_sb == sb and overwrites i_uid and i_gid on
> live eventfs inodes as well:
> 
> fs/tracefs/inode.c:tracefs_apply_options() {
> 	...
> 	list_for_each_entry_rcu(ti, &tracefs_inodes, list) {
> 		if (update_uid) {
> 			ti->flags &= ~TRACEFS_UID_PERM_SET;
> 			ti->vfs_inode.i_uid = fsi->uid;
> 		}
> 	...
> }
> 
> Should ro lookups ignore ei->attr / ei->entry_attrs (or should
> eventfs_get_inode() bypass saved tracefs attributes when on the read-only
> superblock), and should tracefs_apply_options() filter by superblock?

I probably should do that to be more robust. Currently all the files that
are marked as read only do not even have a write method, so they can never
be written to. The important thing is only these read-only files are
exposed in eventfs. But for correctness, modifications to permissions
should be prevented.

> 
> [ ... ]
> 
> > @@ -837,6 +967,52 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry
> >  	return ERR_PTR(-ENOMEM);
> >  }
> >  
> > +/**
> > + * eventfs_create_events_ro_copy - create a read-only events directory
> > + * @name: The name of the top level directory to create.
> > + * @entries: A list of entries that represent the files under this directory
> > + * @size: The number of @entries
> > + * @data: The default data to pass to the files (an entry may override it).
> > + *
> > + * This function configures the eventfs filesystem root as a read-only
> > + * trace event directory using the existing eventfs_inode lazy-lookup
> > + * infrastructure.
> > + *
> > + * See eventfs_create_dir() for use of @entries.
> > + */
> > +int eventfs_create_events_ro_copy(const char *name, struct eventfs_inode *ei)  
> 
> [Severity: Low]
> This isn't a bug, but the kernel-doc comment for
> eventfs_create_events_ro_copy() still lists @entries, @size, and @data from
> eventfs_create_events_dir() instead of @ei, and the @name parameter is
> unused in the function body.

Will fix.

> 
> > +{
> > +	static struct dentry *dentry;
> > +	struct tracefs_inode *ti;
> > +	struct inode *inode;
> > +
> > +	/* Can only be called once. */
> > +	if (dentry)
> > +		return -EBUSY;
> > +
> > +	/* Reference acquired but never freed */
> > +	dentry = eventfs_ro_get_root();
> > +	if (IS_ERR(dentry))
> > +		return PTR_ERR(dentry);
> > +
> > +	inode = d_inode(dentry);
> > +
> > +	INIT_LIST_HEAD(&ei->children);
> > +	INIT_LIST_HEAD(&ei->list);
> > +
> > +	ti = get_tracefs(inode);
> > +	ti->flags |= TRACEFS_EVENT_INODE;
> > +	ti->private = ei;
> > +
> > +	inode->i_op = &eventfs_ro_dir_inode_operations;
> > +	inode->i_fop = &eventfs_ro_file_operations;
> > +
> > +	/* This is never freed */
> > +	dentry->d_fsdata = get_ei(ei);
> > +
> > +	return 0;
> > +}  
> 
> [Severity: Medium]
> Is it safe to re-initialize ei->children and ei->list here?
> 
> In create_event_toplevel_files(), e_events has already been allocated and
> initialized by eventfs_create_events_dir() -> alloc_root_ei() -> init_ei(),
> and published via tracefs_end_creating(). Calling
> INIT_LIST_HEAD(&ei->children) and INIT_LIST_HEAD(&ei->list) again without
> holding eventfs_mutex risks resetting an already-live list.

Hmm, I thought I fixed that. It's one of the things that Masami mentioned.
Hmm, did I not commit the changes?

> 
> Also, should dentry->d_fsdata and ti->private be set before setting
> ti->flags |= TRACEFS_EVENT_INODE and switching inode->i_op to
> eventfs_ro_dir_inode_operations, and should dentry be reset to NULL (or only
> assigned after checking IS_ERR) if eventfs_ro_get_root() fails so static
> dentry does not hold an ERR_PTR?
> 
> [ ... ]
> 
> > diff --git a/include/linux/tracefs.h b/include/linux/tracefs.h
> > index bc354d3400464..41e97a6bd69a3 100644
> > --- a/include/linux/tracefs.h
> > +++ b/include/linux/tracefs.h
> > @@ -75,6 +75,7 @@ struct eventfs_entry {
> >  	const char			*name;
> >  	eventfs_callback		callback;
> >  	eventfs_release			release;
> > +	bool				read_only;
> >  };  
> 
> [Severity: Low]
> This isn't a bug, but struct eventfs_entry has a kernel-doc comment just
> above this definition where @read_only (and @release) are not documented.

Will fix.

> 
> [ ... ]
> 
> > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> > index 6afed2091e19c..968fdbca8054f 100644
> > --- a/kernel/trace/trace_events.c
> > +++ b/kernel/trace/trace_events.c  
> 
> [ ... ]
> 
> > @@ -4775,6 +4781,13 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
> >  
> >  	tr->event_dir = e_events;
> >  
> > +	if (!event_dir_ro_created && (tr->flags & TRACE_ARRAY_FL_GLOBAL)) {
> > +		int ret;
> > +
> > +		ret = eventfs_create_events_ro_copy("events", e_events);
> > +		if (!ret)
> > +			event_dir_ro_created = true;
> > +	}
> >  	return 0;
> >  }  
> 
> [Severity: High]
> What happens here when the kernel is booted with lockdown=confidentiality
> (LOCKDOWN_TRACEFS)?
> 
> In eventfs_create_events_dir():
> 
> fs/tracefs/event_inode.c:eventfs_create_events_dir() {
> 	...
> 	if (security_locked_down(LOCKDOWN_TRACEFS))
> 		return NULL;
> 	...
> }
> 
> When LOCKDOWN_TRACEFS is active, eventfs_create_events_dir() returns NULL
> rather than an ERR_PTR. In create_event_toplevel_files(), IS_ERR(e_events)
> evaluates to false for NULL, so e_events = NULL is passed to
> eventfs_create_events_ro_copy("events", e_events), which dereferences ei
> without a NULL check and crashes the kernel during boot.
> 
> Should create_event_toplevel_files() or eventfs_create_events_ro_copy()
> check for !e_events (and security_locked_down(LOCKDOWN_TRACEFS)) before
> dereferencing ei?
> 

Oops, will fix.

Thanks,

-- Steve


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4] tracefs: Add read-only eventfs filesystem at /sys/kernel/events
  2026-11-02 20:27 [PATCH v4] tracefs: Add read-only eventfs filesystem at /sys/kernel/events Steven Rostedt
  2026-11-02 20:44 ` sashiko-bot
@ 2026-11-03  2:18 ` Masami Hiramatsu (Google)
  2026-11-03  2:53   ` Steven Rostedt
  1 sibling, 1 reply; 5+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-11-03  2:18 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Linux Trace Kernel, Mathieu Desnoyers, Anubhav Shelat

On Mon, 2 Nov 2026 15:27:48 -0500, Steven Rostedt <rostedt@goodmis.org> wrote:
> From: Anubhav Shelat <ashelat@redhat.com>
> 
> Introduce a new read-only pseudo-filesystem "eventfs" mounted at
> /sys/kernel/events that exposes trace event format and id files
> (mode 0444) to unprivileged users. This allows tools like perf to
> discover event formats without requiring access to the full
> tracefs/debugfs mount.
> 
> The eventfs file system reuses the existing eventfs_inode lazy-lookup
> infrastructure. A new set of super_operations
> (eventfs_ro_super_operations) shares the tracefs inode allocator so
> that eventfs_get_inode() and get_tracefs() work on the RO superblock.
> The superblock is manually initialized to ensure the root inode is
> allocated with tracefs_alloc_inode, allowing the root to serve
> directly as the events directory without another events subdirectory.
> 
> Each qualifying event gets a subsystem directory containing format
> and id files. The top-level events directory also exposes header_page
> and header_event. Similar to tracefs, a change will need to be made in
> systemd to mount this filesystem automatically.
> 
> [
>   Steven Rostedt rewrote to use the lookup functions to determine read
>   only files to show. The struct eventfs_entry now has a "readonly" field
>   and if it is set, it will be displayed in the read only file system.
>   The original code [1] duplicated the eventfs_inodes for every event and
>   subsystem. The new approach uses the existing eventfs_inodes (without
>   requiring more memory to store them) and uses the eventfs_entry arrays to
>   show which files are allowed to be read readonly or not.
> 
>   [1] https://patch.msgid.link/20260715135231.338535-4-ashelat@redhat.com
> ]

Thanks for the work! As we talked offline, I think we can use this for
the backup instance (an instance copied from another instance at boot)
because that backup instance is also read only. See trace_array_is_readonly()
and TRACE_ARRAY_FL_RDONLY[1].

[1] https://lore.kernel.org/all/177502546939.1311542.1826814401724828930.stgit@mhiramat.tok.corp.google.com/

> 
> Assisted-by: CLAUDE:claude-opus-4 Apogee

BTW, this tag seems removed by commit 1e035297ffda ("docs: remove 
Assisted-by references")

Thanks,

> Signed-off-by: Anubhav Shelat <ashelat@redhat.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> Changes since v3: https://patch.msgid.link/20260810160708.3460a2fd@gandalf.local.home
> 
> - Fixed kernel doc name for eventfs_create_events_ro_copy() (Masami Hiramatsu)
> 
> - Removed duplicate initialization of ei in that function. (Masami Hiramatsu)
> 
> - Added include of linux/magic.h as it failed to build on some
>   architectures. (kernel test robot)
> 
>  fs/tracefs/event_inode.c    | 192 ++++++++++++++++++++++++++++++++++--
>  fs/tracefs/inode.c          |  21 +++-
>  fs/tracefs/internal.h       |   6 ++
>  include/linux/tracefs.h     |   3 +
>  include/uapi/linux/magic.h  |   1 +
>  kernel/trace/trace_events.c |  13 +++
>  6 files changed, 225 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
> index 6457df72173d..8a4125cd0d48 100644
> --- a/fs/tracefs/event_inode.c
> +++ b/fs/tracefs/event_inode.c
> @@ -16,14 +16,87 @@
>   */
>  #include <linux/fsnotify.h>
>  #include <linux/fs.h>
> +#include <linux/fs_context.h>
>  #include <linux/namei.h>
>  #include <linux/workqueue.h>
>  #include <linux/security.h>
>  #include <linux/tracefs.h>
> +#include <linux/magic.h>
>  #include <linux/kref.h>
>  #include <linux/delay.h>
>  #include "internal.h"
>  
> +static struct vfsmount *eventfs_ro_mount;
> +static int eventfs_ro_mount_count;
> +
> +static int eventfs_ro_fill_super(struct super_block *sb, struct fs_context *fc)
> +{
> +	struct inode *inode;
> +	struct dentry *root;
> +
> +	sb->s_blocksize = PAGE_SIZE;
> +	sb->s_blocksize_bits = PAGE_SHIFT;
> +	sb->s_magic = EVENTFS_SUPER_MAGIC;
> +	sb->s_op = &eventfs_ro_super_operations;
> +	sb->s_time_gran = 1;
> +	sb->s_flags |= SB_RDONLY;
> +
> +	inode = new_inode(sb);
> +	if (!inode)
> +		return -ENOMEM;
> +
> +	inode->i_ino = 1;
> +	inode->i_mode = S_IFDIR | 0555;
> +	simple_inode_init_ts(inode);
> +	inode->i_op = &simple_dir_inode_operations;
> +	inode->i_fop = &simple_dir_operations;
> +	set_nlink(inode, 2);
> +
> +	set_default_d_op(sb, &tracefs_dentry_operations);
> +
> +	root = d_make_root(inode);
> +	if (!root)
> +		return -ENOMEM;
> +
> +	sb->s_root = root;
> +
> +	return 0;
> +}
> +
> +static int eventfs_ro_get_tree(struct fs_context *fc)
> +{
> +	return get_tree_single(fc, eventfs_ro_fill_super);
> +}
> +
> +static const struct fs_context_operations eventfs_ro_context_ops = {
> +	.get_tree	= eventfs_ro_get_tree,
> +};
> +
> +static int eventfs_ro_init_fs_context(struct fs_context *fc)
> +{
> +	fc->ops = &eventfs_ro_context_ops;
> +	return 0;
> +}
> +
> +struct file_system_type eventfs_ro_fs_type = {
> +	.owner =	THIS_MODULE,
> +	.name =		"eventfs",
> +	.init_fs_context = eventfs_ro_init_fs_context,
> +	.kill_sb =	kill_anon_super,
> +};
> +
> +struct dentry *eventfs_ro_get_root(void)
> +{
> +	int error;
> +
> +	error = simple_pin_fs(&eventfs_ro_fs_type, &eventfs_ro_mount,
> +			      &eventfs_ro_mount_count);
> +	if (error)
> +		return ERR_PTR(error);
> +
> +	return dget(eventfs_ro_mount->mnt_root);
> +}
> +
>  /*
>   * eventfs_mutex protects the eventfs_inode (ei) dentry. Any access
>   * to the ei->dentry must be done under this mutex and after checking
> @@ -169,7 +242,11 @@ static inline struct eventfs_inode *get_ei(struct eventfs_inode *ei)
>  static struct dentry *eventfs_root_lookup(struct inode *dir,
>  					  struct dentry *dentry,
>  					  unsigned int flags);
> +static struct dentry *eventfs_root_lookup_ro(struct inode *dir,
> +					     struct dentry *dentry,
> +					     unsigned int flags);
>  static int eventfs_iterate(struct file *file, struct dir_context *ctx);
> +static int eventfs_ro_iterate(struct file *file, struct dir_context *ctx);
>  
>  static void update_attr(struct eventfs_attr *attr, struct iattr *iattr)
>  {
> @@ -247,6 +324,10 @@ static const struct inode_operations eventfs_dir_inode_operations = {
>  	.setattr	= eventfs_set_attr,
>  };
>  
> +static const struct inode_operations eventfs_ro_dir_inode_operations = {
> +	.lookup		= eventfs_root_lookup_ro,
> +};
> +
>  static const struct inode_operations eventfs_file_inode_operations = {
>  	.setattr	= eventfs_set_attr,
>  };
> @@ -257,6 +338,12 @@ static const struct file_operations eventfs_file_operations = {
>  	.llseek		= generic_file_llseek,
>  };
>  
> +static const struct file_operations eventfs_ro_file_operations = {
> +	.read		= generic_read_dir,
> +	.iterate_shared	= eventfs_ro_iterate,
> +	.llseek		= generic_file_llseek,
> +};
> +
>  static void eventfs_set_attrs(struct eventfs_inode *ei, bool update_uid, kuid_t uid,
>  			      bool update_gid, kgid_t gid, int level)
>  {
> @@ -420,7 +507,7 @@ static struct dentry *lookup_file(struct eventfs_inode *parent_ei,
>   * a eventfs_inode.
>   */
>  static struct dentry *lookup_dir_entry(struct dentry *dentry,
> -	struct eventfs_inode *pei, struct eventfs_inode *ei)
> +	struct eventfs_inode *pei, struct eventfs_inode *ei, bool ro)
>  {
>  	struct inode *inode;
>  	umode_t mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
> @@ -429,8 +516,13 @@ static struct dentry *lookup_dir_entry(struct dentry *dentry,
>  	if (unlikely(!inode))
>  		return ERR_PTR(-ENOMEM);
>  
> -	inode->i_op = &eventfs_dir_inode_operations;
> -	inode->i_fop = &eventfs_file_operations;
> +	if (ro) {
> +		inode->i_op = &eventfs_ro_dir_inode_operations;
> +		inode->i_fop = &eventfs_ro_file_operations;
> +	} else {
> +		inode->i_op = &eventfs_dir_inode_operations;
> +		inode->i_fop = &eventfs_file_operations;
> +	}
>  
>  	/* All directories will have the same inode number */
>  	inode->i_ino = eventfs_dir_ino(ei);
> @@ -534,9 +626,10 @@ lookup_file_dentry(struct dentry *dentry,
>   * list, if @dentry found go ahead and create the file/dir
>   */
>  
> -static struct dentry *eventfs_root_lookup(struct inode *dir,
> -					  struct dentry *dentry,
> -					  unsigned int flags)
> +static struct dentry *__eventfs_root_lookup(struct inode *dir,
> +					    struct dentry *dentry,
> +					    unsigned int flags,
> +					    bool ro)
>  {
>  	struct eventfs_inode *ei_child;
>  	struct tracefs_inode *ti;
> @@ -559,7 +652,7 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
>  		/* A child is freed and removed from the list at the same time */
>  		if (WARN_ON_ONCE(ei_child->is_freed))
>  			return NULL;
> -		return lookup_dir_entry(dentry, ei, ei_child);
> +		return lookup_dir_entry(dentry, ei, ei_child, ro);
>  	}
>  
>  	for (int i = 0; i < ei->nr_entries; i++) {
> @@ -571,20 +664,44 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
>  		if (strcmp(name, entry->name) != 0)
>  			continue;
>  
> +		if (ro && !entry->read_only)
> +			return NULL;
> +
>  		data = ei->data;
>  		if (entry->callback(name, &mode, &data, &fops) <= 0)
>  			return NULL;
>  
> +		/*
> +		 * Read-only files gets all users read access,
> +		 * but also should make sure this is not writable.
> +		 */
> +		if (ro)
> +			mode |= (mode & ~0222) | 0444;
> +
>  		return lookup_file_dentry(dentry, ei, i, mode, data, fops);
>  
>  	}
>  	return NULL;
>  }
>  
> +static struct dentry *eventfs_root_lookup(struct inode *dir,
> +					  struct dentry *dentry,
> +					  unsigned int flags)
> +{
> +	return __eventfs_root_lookup(dir, dentry, flags, false);
> +}
> +
> +static struct dentry *eventfs_root_lookup_ro(struct inode *dir,
> +					     struct dentry *dentry,
> +					     unsigned int flags)
> +{
> +	return __eventfs_root_lookup(dir, dentry, flags, true);
> +}
> +
>  /*
>   * Walk the children of a eventfs_inode to fill in getdents().
>   */
> -static int eventfs_iterate(struct file *file, struct dir_context *ctx)
> +static int __eventfs_iterate(struct file *file, struct dir_context *ctx, bool ro)
>  {
>  	const struct file_operations *fops;
>  	struct inode *f_inode = file_inode(file);
> @@ -630,6 +747,9 @@ static int eventfs_iterate(struct file *file, struct dir_context *ctx)
>  		entry = &ei->entries[i];
>  		name = entry->name;
>  
> +		if (ro && !entry->read_only)
> +			continue;
> +
>  		/* If ei->is_freed then just bail here, nothing more to do */
>  		scoped_guard(mutex, &eventfs_mutex) {
>  			if (ei->is_freed)
> @@ -688,6 +808,16 @@ static int eventfs_iterate(struct file *file, struct dir_context *ctx)
>  	return 1;
>  }
>  
> +static int eventfs_iterate(struct file *file, struct dir_context *ctx)
> +{
> +	return __eventfs_iterate(file, ctx, false);
> +}
> +
> +static int eventfs_ro_iterate(struct file *file, struct dir_context *ctx)
> +{
> +	return __eventfs_iterate(file, ctx, true);
> +}
> +
>  /**
>   * eventfs_create_dir - Create the eventfs_inode for this directory
>   * @name: The name of the directory to create.
> @@ -845,6 +975,52 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry
>  	return ERR_PTR(-ENOMEM);
>  }
>  
> +/**
> + * eventfs_create_events_ro_copy - create a read-only events directory
> + * @name: The name of the top level directory to create.
> + * @entries: A list of entries that represent the files under this directory
> + * @size: The number of @entries
> + * @data: The default data to pass to the files (an entry may override it).
> + *
> + * This function configures the eventfs filesystem root as a read-only
> + * trace event directory using the existing eventfs_inode lazy-lookup
> + * infrastructure.
> + *
> + * See eventfs_create_dir() for use of @entries.
> + */
> +int eventfs_create_events_ro_copy(const char *name, struct eventfs_inode *ei)
> +{
> +	static struct dentry *dentry;
> +	struct tracefs_inode *ti;
> +	struct inode *inode;
> +
> +	/* Can only be called once. */
> +	if (dentry)
> +		return -EBUSY;
> +
> +	/* Reference acquired but never freed */
> +	dentry = eventfs_ro_get_root();
> +	if (IS_ERR(dentry))
> +		return PTR_ERR(dentry);
> +
> +	inode = d_inode(dentry);
> +
> +	INIT_LIST_HEAD(&ei->children);
> +	INIT_LIST_HEAD(&ei->list);
> +
> +	ti = get_tracefs(inode);
> +	ti->flags |= TRACEFS_EVENT_INODE;
> +	ti->private = ei;
> +
> +	inode->i_op = &eventfs_ro_dir_inode_operations;
> +	inode->i_fop = &eventfs_ro_file_operations;
> +
> +	/* This is never freed */
> +	dentry->d_fsdata = get_ei(ei);
> +
> +	return 0;
> +}
> +
>  /**
>   * eventfs_remove_rec - remove eventfs dir or file from list
>   * @ei: eventfs_inode to be removed.
> diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
> index 5020365ca704..555af5402adc 100644
> --- a/fs/tracefs/inode.c
> +++ b/fs/tracefs/inode.c
> @@ -423,6 +423,14 @@ static const struct super_operations tracefs_super_operations = {
>  	.show_options	= tracefs_show_options,
>  };
>  
> +const struct super_operations eventfs_ro_super_operations = {
> +	.alloc_inode    = tracefs_alloc_inode,
> +	.free_inode     = tracefs_free_inode,
> +	.destroy_inode  = tracefs_destroy_inode,
> +	.drop_inode     = tracefs_drop_inode,
> +	.statfs		= simple_statfs,
> +};
> +
>  /*
>   * It would be cleaner if eventfs had its own dentry ops.
>   *
> @@ -455,7 +463,7 @@ static int tracefs_d_delete(const struct dentry *dentry)
>  	return dentry->d_fsdata == NULL;
>  }
>  
> -static const struct dentry_operations tracefs_dentry_operations = {
> +const struct dentry_operations tracefs_dentry_operations = {
>  	.d_revalidate = tracefs_d_revalidate,
>  	.d_release = tracefs_d_release,
>  	.d_delete = tracefs_d_delete,
> @@ -801,8 +809,15 @@ static int __init tracefs_init(void)
>  		return -EINVAL;
>  
>  	retval = register_filesystem(&trace_fs_type);
> -	if (!retval)
> -		tracefs_registered = true;
> +	if (retval)
> +		return retval;
> +	tracefs_registered = true;
> +
> +	retval = sysfs_create_mount_point(kernel_kobj, "events");
> +	if (retval)
> +		return retval;
> +
> +	retval = register_filesystem(&eventfs_ro_fs_type);
>  
>  	return retval;
>  }
> diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h
> index c61481d04c8e..c92376bc3121 100644
> --- a/fs/tracefs/internal.h
> +++ b/fs/tracefs/internal.h
> @@ -2,6 +2,10 @@
>  #ifndef _TRACEFS_INTERNAL_H
>  #define _TRACEFS_INTERNAL_H
>  
> +extern const struct dentry_operations tracefs_dentry_operations;
> +extern const struct super_operations eventfs_ro_super_operations;
> +extern struct file_system_type eventfs_ro_fs_type;
> +
>  enum {
>  	TRACEFS_EVENT_INODE		= BIT(1),
>  	TRACEFS_GID_PERM_SET		= BIT(2),
> @@ -73,6 +77,8 @@ struct dentry *tracefs_end_creating(struct dentry *dentry);
>  struct dentry *tracefs_failed_creating(struct dentry *dentry);
>  struct inode *tracefs_get_inode(struct super_block *sb);
>  
> +struct dentry *eventfs_ro_get_root(void);
> +
>  void eventfs_remount(struct tracefs_inode *ti, bool update_uid, bool update_gid);
>  void eventfs_d_release(struct dentry *dentry);
>  
> diff --git a/include/linux/tracefs.h b/include/linux/tracefs.h
> index bc354d340046..41e97a6bd69a 100644
> --- a/include/linux/tracefs.h
> +++ b/include/linux/tracefs.h
> @@ -75,6 +75,7 @@ struct eventfs_entry {
>  	const char			*name;
>  	eventfs_callback		callback;
>  	eventfs_release			release;
> +	bool				read_only;
>  };
>  
>  struct eventfs_inode;
> @@ -87,6 +88,8 @@ struct eventfs_inode *eventfs_create_dir(const char *name, struct eventfs_inode
>  					 const struct eventfs_entry *entries,
>  					 int size, void *data);
>  
> +int eventfs_create_events_ro_copy(const char *name, struct eventfs_inode *ei);
> +
>  void eventfs_remove_events_dir(struct eventfs_inode *ei);
>  void eventfs_remove_dir(struct eventfs_inode *ei);
>  
> diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
> index 66a91c8b1cb9..521916f64dee 100644
> --- a/include/uapi/linux/magic.h
> +++ b/include/uapi/linux/magic.h
> @@ -76,6 +76,7 @@
>  #define STACK_END_MAGIC		0x57AC6E9D
>  
>  #define TRACEFS_MAGIC          0x74726163
> +#define EVENTFS_SUPER_MAGIC    0x65766673
>  
>  #define V9FS_MAGIC		0x01021997
>  
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 49d135bc2fbf..1976920beb1a 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -3298,17 +3298,20 @@ event_create_dir(struct eventfs_inode *parent, struct trace_event_file *file)
>  		{
>  			.name		= "format",
>  			.callback	= event_callback,
> +			.read_only	= true,
>  		},
>  #ifdef CONFIG_PERF_EVENTS
>  		{
>  			.name		= "id",
>  			.callback	= event_callback,
> +			.read_only	= true,
>  		},
>  #endif
>  #ifdef CONFIG_BPF_EVENTS
>  		{
>  			.name		= "btf_ids",
>  			.callback	= event_callback,
> +			.read_only	= true,
>  		},
>  #endif
>  #define NR_RO_EVENT_ENTRIES	(1 + IS_ENABLED(CONFIG_PERF_EVENTS) + \
> @@ -4722,6 +4725,7 @@ static int events_callback(const char *name, umode_t *mode, void **data,
>  static int
>  create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
>  {
> +	static bool event_dir_ro_created;
>  	struct eventfs_inode *e_events;
>  	struct dentry *entry;
>  	int nr_entries;
> @@ -4729,10 +4733,12 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
>  		{
>  			.name		= "header_page",
>  			.callback	= events_callback,
> +			.read_only	= true,
>  		},
>  		{
>  			.name		= "header_event",
>  			.callback	= events_callback,
> +			.read_only	= true,
>  		},
>  #define NR_RO_TOP_ENTRIES	2
>  /* Readonly files must be above this line and counted by NR_RO_TOP_ENTRIES. */
> @@ -4775,6 +4781,13 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
>  
>  	tr->event_dir = e_events;
>  
> +	if (!event_dir_ro_created && (tr->flags & TRACE_ARRAY_FL_GLOBAL)) {
> +		int ret;
> +
> +		ret = eventfs_create_events_ro_copy("events", e_events);
> +		if (!ret)
> +			event_dir_ro_created = true;
> +	}
>  	return 0;
>  }
>  
> -- 
> 2.53.0
> 
>

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4] tracefs: Add read-only eventfs filesystem at /sys/kernel/events
  2026-11-03  2:18 ` Masami Hiramatsu (Google)
@ 2026-11-03  2:53   ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2026-11-03  2:53 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: LKML, Linux Trace Kernel, Mathieu Desnoyers, Anubhav Shelat

On Tue, 03 Nov 2026 11:18:03 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

\
> Thanks for the work! As we talked offline, I think we can use this for
> the backup instance (an instance copied from another instance at boot)
> because that backup instance is also read only. See trace_array_is_readonly()
> and TRACE_ARRAY_FL_RDONLY[1].

Will do.

> 
> [1] https://lore.kernel.org/all/177502546939.1311542.1826814401724828930.stgit@mhiramat.tok.corp.google.com/
> 
> > 
> > Assisted-by: CLAUDE:claude-opus-4 Apogee  
> 
> BTW, this tag seems removed by commit 1e035297ffda ("docs: remove 
> Assisted-by references")

As I was one of the first people to advocate for that tag, and I wasn't
invited to the Maintainers Summit to argue against the removal of that
tag, I will not remove any assisted by tags.

It may not be required anymore, but hell, I'll keep them for commits
I'm responsible for. I still find them useful.

-- Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-03  2:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-02 20:27 [PATCH v4] tracefs: Add read-only eventfs filesystem at /sys/kernel/events Steven Rostedt
2026-11-02 20:44 ` sashiko-bot
2026-11-02 21:04   ` Steven Rostedt
2026-11-03  2:18 ` Masami Hiramatsu (Google)
2026-11-03  2:53   ` Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.