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

Steven Rostedt <[email protected]>
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-kernel,org.kernel.vger.linux-trace-kernel
Message-ID <[email protected]>
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 <[email protected]>
> 
> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.