linux-kernel - Re: tracing: user events UAF crash report

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: <a7d66736-80d3-4f3d-9cca-ec46e019b4cb@grsecurity.net>
Date: Thu, 25 Jul 2024 23:32:30 +0200
From: Mathias Krause <minipli@...ecurity.net>
To: Steven Rostedt <rostedt@...dmis.org>
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 25.07.24 23:14, Steven Rostedt wrote:
> On Thu, 25 Jul 2024 22:41:23 +0200
> Mathias Krause <minipli@...ecurity.net> wrote:
> 
>>> 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
View attachment "serial.log" of type "text/x-log" (448173 bytes)

View attachment "6.10-tracefs_dbg.diff" of type "text/x-patch" (1881 bytes)