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: <1b1b09fa-0064-429c-9f78-385119c5e691@grsecurity.net>
Date: Thu, 25 Jul 2024 22:41:23 +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 22:15, Steven Rostedt wrote:
> On Thu, 25 Jul 2024 21:42:41 +0200
> Mathias Krause <minipli@...ecurity.net> wrote:
> 
>> Right. But the point is, that 'event_call' is really some '&user->call'.
>> With 'user' being free'd memory, what gives? Dereferencing 'event_call'
>> is UB, so this function is doomed to fail because it cannot know if its
>> only argument points to still valid memory or not. And that's the core
>> issue -- calling that function for an object that's long gone -- the
>> missing refcounting I hinted at in my first Email.
> 
> Ah, I missed that the call was part of the user structure. But I think I
> found the real fix.
> 
>> [...]
> 
> I believe the issue is that f_start() needs to check if the event file has
> been freed.
> 
> New patch:
> 
> 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.

Sorry,
Mathias