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: <20240725190632.2755cb70@gandalf.local.home>
Date: Thu, 25 Jul 2024 19:06:32 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Mathias Krause <minipli@...ecurity.net>
Cc: Ajay Kaher <ajay.kaher@...adcom.com>, Masami Hiramatsu
 <mhiramat@...nel.org>, Ilkka Naulapää
 <digirigawa@...il.com>, Linus Torvalds <torvalds@...ux-foundation.org>, Al
 Viro <viro@...iv.linux.org.uk>, linux-trace-kernel@...r.kernel.org,
 linux-kernel@...r.kernel.org, regressions@...mhuis.info, Dan Carpenter
 <dan.carpenter@...aro.org>, Vasavi Sirnapalli
 <vasavi.sirnapalli@...adcom.com>, Alexey Makhalov
 <alexey.makhalov@...adcom.com>, Florian Fainelli
 <florian.fainelli@...adcom.com>, Beau Belgrave <beaub@...ux.microsoft.com>
Subject: Re: tracing: user events UAF crash report

On Thu, 25 Jul 2024 23:32:30 +0200
Mathias Krause <minipli@...ecurity.net> wrote:

> That was for a single run of
> tools/testing/selftests/user_events/ftrace_test with the read loop of
> /sys/kernel/tracing/events/user_events/__test_event/format in a
> different shell.
> 
> > 
> > destroy_user_event() which is under event_mutex calls
> > user_event_set_call_visible() with false, that will then call:
> > 
> > trace_remove_event_call() -> probe_remove_event_call() ->
> >  __trace_remove_event_call() -> event_remove() ->
> >  remove_event_from_tracers()
> > 
> > Where remove_event_from_tracers() loops over all the instances and will set
> > each of the file pointers flags associated to the event: EVENT_FILE_FL_FREED
> > 
> > Then it returns back to destroy_user_event() that would free the event.
> > 
> > The f_start() that was in your crash, with the new patch, should take the
> > event_mutex before referencing the event that was freed. And with that flag
> > being set, it should exit out.  
> 
> Looking at the very first report:
> 
> [   76.306946] BUG: KASAN: slab-use-after-free in f_start+0x36e/0x3d0
> 
> That's what faddr2line gives me:
> 
> f_start+0x36e/0x3d0:
> f_start at kernel/trace/trace_events.c:1637 (discriminator 1)
> 
> Which is:
> 1635     mutex_lock(&event_mutex);
> 1636     file = event_file_data(m->private);
> 1637     if (!file || (file->flags & EVENT_FILE_FL_FREED))
> 1638         return ERR_PTR(-ENODEV);

BAH! I finally figured it out.

I was able to reproduce it and this does stop the UAF from happening.

The issue was, as a short cut, I had the "format" file's i_private point to
the "call" entry directly, and not go via the "file". This is because the
all format files are the same for the same "call", so no reason to
differentiate them.  The other files maintain state (like the "enable",
"trigger", etc). But this means if the file were to disappear, the "format"
file would be unaware of it.

This should fix it for you. It fixed it for me.

-- Steve


diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 6ef29eba90ce..852643d957de 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1540,7 +1540,8 @@ enum {
 
 static void *f_next(struct seq_file *m, void *v, loff_t *pos)
 {
-	struct trace_event_call *call = event_file_data(m->private);
+	struct trace_event_file *file = event_file_data(m->private);
+	struct trace_event_call *call = file->event_call;
 	struct list_head *common_head = &ftrace_common_fields;
 	struct list_head *head = trace_get_fields(call);
 	struct list_head *node = v;
@@ -1572,7 +1573,8 @@ static void *f_next(struct seq_file *m, void *v, loff_t *pos)
 
 static int f_show(struct seq_file *m, void *v)
 {
-	struct trace_event_call *call = event_file_data(m->private);
+	struct trace_event_file *file = event_file_data(m->private);
+	struct trace_event_call *call = file->event_call;
 	struct ftrace_event_field *field;
 	const char *array_descriptor;
 
@@ -1627,12 +1629,14 @@ static int f_show(struct seq_file *m, void *v)
 
 static void *f_start(struct seq_file *m, loff_t *pos)
 {
+	struct trace_event_file *file;
 	void *p = (void *)FORMAT_HEADER;
 	loff_t l = 0;
 
 	/* ->stop() is called even if ->start() fails */
 	mutex_lock(&event_mutex);
-	if (!event_file_data(m->private))
+	file = event_file_data(m->private);
+	if (!file || (file->flags & EVENT_FILE_FL_FREED))
 		return ERR_PTR(-ENODEV);
 
 	while (l < *pos && p)
@@ -2485,7 +2489,6 @@ static int event_callback(const char *name, umode_t *mode, void **data,
 	if (strcmp(name, "format") == 0) {
 		*mode = TRACE_MODE_READ;
 		*fops = &ftrace_event_format_fops;
-		*data = call;
 		return 1;
 	}
 
-- 
2.43.0


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ