Re: [PATCH v3] tracefs: Add read-only eventfs filesystem at /sys/kernel/events

[Steven Rostedt]

Replying to Sashiko: https://sashiko.dev/#/patchset/20260810160708.3460a2fd%40gandalf.local.home

> --- Patch [1]: [PATCH v3] tracefs: Add read-only eventfs filesystem at /sys/kernel/events ---
> commit 5fedbb447b6c46a029edb4492a02145a903d2475
> Author: Anubhav Shelat <ashelat@xxxxxxxxxx>
> 
> tracefs: Add read-only eventfs filesystem at /sys/kernel/events
> 
> This commit introduces a read-only pseudo-filesystem "eventfs" mounted at
> /sys/kernel/events, exposing trace event format and id files to unprivileged
> users. It reuses the eventfs_inode lazy-lookup infrastructure with new
> super_operations to share the tracefs inode allocator.
> 
> > diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
> > index a524584353279..3af1cf5abe4ab 100644
> > --- a/fs/tracefs/event_inode.c
> > +++ b/fs/tracefs/event_inode.c
> [ ... ]
> > @@ -561,20 +653,40 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
> [ ... ]
> >  /*
> >   * 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);
> 
> This is a pre-existing issue, but does exposing this directory iteration
> code to unprivileged users create a severe security vulnerability?
> 
> Looking at __eventfs_iterate(), there is an existing integer truncation
> when setting up the loop:
> 
> fs/tracefs/event_inode.c:__eventfs_iterate() {
>         ...
>         c = ctx->pos - 2;
>         ...
>         for (i = c; i < ei->nr_entries; i++, ctx->pos++) {
>                 ...
>                 entry = &ei->entries[i];
>                 ...
>                 r = entry->callback(name, &mode, &cdata, &fops);
> 
> If an attacker uses lseek() to set ctx->pos to 0x100000000, ctx->pos - 2
> truncates into the 32-bit signed integer 'c', resulting in c = -2.

Once again, this is not possible. That's because both eventfs and tracefs
have their super_block maxbytes set to the default which is 0x7fffffff.
Anything more than that will return an error. You can't use lseek() bigger
than that.

> 
> Since the 30-bit unsigned bitfield ei->nr_entries is promoted to a signed
> int in C, the loop condition (i < ei->nr_entries) evaluates as (-2 < 5),
> which is true.
> 
> This allows a negative index to be used in &ei->entries[i], reading
> out-of-bounds memory to find the function pointer entry->callback, which is
> then immediately executed.
> 
> Because this patch makes eventfs available to unprivileged users via the
> new 0555 mount, can this be used by any local user to trigger the
> out-of-bounds read and control-flow hijack?

With the following code:

readdir.c:
-------------------------------8<-------------------------------
#define _LARGEFILE64_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdarg.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
#include <dirent.h>

#include <sys/types.h>
#include <sys/stat.h>

int main(int argc, char **argv)
{
	unsigned long long addr;
	struct dirent *dirent;
	DIR *dir;
	char *file;
	int fd;

	if (argc != 3) {
		fprintf(stderr,"usage: %s dir addr\n",argv[0]);
		exit(-1);
	}

	file = argv[1];
	addr = strtoull(argv[2], NULL, 0);

	fd = open(file, O_RDONLY);
	if (fd < 0) {
		perror("open");
		exit(0);
	}
	printf("lseek to %llx\n", addr);
	if (lseek64(fd, addr, SEEK_SET) == (off_t)-1) {
		perror("lseek");
		exit(-1);
	}

	dir = fdopendir(fd);
	dirent = readdir(dir);
	if (!dirent) {
		printf("Nothing to read\n");
		exit(0);
	}

	printf("entry: %s\n", dirent->d_name);
	return 0;
}
------------------------------->8-------------------------------

I run:

~# ./readdir /sys/kernel/events 0x7fffffff
lseek to 7fffffff
Nothing to read

~# ./readdir /sys/kernel/events/sched 0x7fffffff
lseek to 7fffffff
Nothing to read

~# ./readdir /sys/kernel/events/sched/sched_switch 0x7fffffff
lseek to 7fffffff
Nothing to read

~# ./readdir /sys/kernel/events/ 0x100000000
lseek to 100000000
lseek: Invalid argument

~# ./readdir /sys/kernel/events/sched 0x100000000
lseek to 100000000
lseek: Invalid argument

~# ./readdir /sys/kernel/events/sched/sched_switch 0x100000000
lseek to 100000000
lseek: Invalid argument

~# ./readdir /sys/kernel/events/ 2
lseek to 2
entry: header_page

~# ./readdir /sys/kernel/events/sched 2
lseek to 2
entry: sched_wake_idle_without_ipi

~# ./readdir /sys/kernel/events/sched/sched_switch/ 2
lseek to 2
entry: format

How can we hit the condition that Sashiko is reporting?

-- Steve