Re: tracing: user events UAF crash report

On 25.07.24 23:14, Steven Rostedt wrote:

On Thu, 25 Jul 2024 22:41:23 +0200
Mathias Krause [off-list ref] wrote:

quoted

quoted

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 6ef29eba90ce..5fbfa1c885de 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c

@@ -1627,12 +1627,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)

  

Nope, still the same splats.

Can you reshow the splats. Because I'm now confused.

Sure, see attached serial.log.

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);

Apparently, 'file' was free'd now and reading the 'flags' member
triggers KASAN.

Second report is:

[   76.367688] BUG: KASAN: slab-use-after-free in f_start+0x2e4/0x3d0

which faddr2line says is:

f_start+0x2e4/0x3d0:
trace_get_fields at include/linux/trace_events.h:482
(inlined by) f_next at kernel/trace/trace_events.c:1545
(inlined by) f_start at kernel/trace/trace_events.c:1641

480 trace_get_fields(struct trace_event_call *event_call)
481 {
482     if (!event_call->class->get_fields)
483         return &event_call->class->fields;

The one we ran into first.

So still something doesn't match up with how lifetimes of objects are
managed.

Did you remove all the other patches before applying this one?

Sure. That's what I have on top of v6.10:

minipli@nuc:~/src/linux (tracefs)$ git diff v6.10 > ~/6.10-tracefs_dbg.diff

Please ignore the WARN()s. They're left-overs from earlier debug
attempts of mine.

Thanks,
Mathias