linux-kernel - Re: [GIT PULL] ftrace: Fixes for v6.13

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: <CAHk-=whrxZwrdm2zHrTmEdRoiC_29XR+0nJpq2CkZjm-ZdOTNg@mail.gmail.com>
Date: Sat, 14 Dec 2024 20:00:57 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, 
	Al Viro <viro@...iv.linux.org.uk>, Michal Simek <monstr@...str.eu>
Subject: Re: [GIT PULL] ftrace: Fixes for v6.13

On Sat, 14 Dec 2024 at 19:03, Steven Rostedt <rostedt@...dmis.org> wrote:
>
> It gets set by the persistent ring buffer code, via a call to
> ring_buffer_last_boot_delta().

That seems entirely broken, since it basically depends on the kernel
being the same one. Which isn't even checked for.

Not to mention that it wouldn't work for modules anyway.

This kind of random hackery needs to DIE.

Tell people to turn of KASLR for cross-boot tracing. Or just not do it.

> > If "people use stale pointers for '%s' and we actually care" is a real
> > issue, we could very possibly teach vsnprintf() about that. The code
> > already has a "check_pointer()" thing for just NULL pointer magic
> > printouts.
>
> The check code was added because I was sick and tired of fixing bugs in
> trace events. People would use the TRACE_EVENT() TP_printk() like a normal
> printk with things like:
>
>   TP_fast_assign(
>       __entry->ipv6 = ptr_to_ipv6;
>   )
>
>   TP_printk("ipv6=%pI6", __entry->ipv6)

I think you are confused.

You are talking about the checks in test_event_printk(). That is fine
- that's a sanity check of the trace fmt string at trace event
registration time.

But that's not at all the code that the new "fix" is all about.

The new "fix" is for the disgusting horror that is
trace_check_vprintf(), which is the "I'll take the trace format
string, and I'll print it out partly using the regular vprintf()
machinery, but I'll partly dig into it and do horrendous things to
it".

And *THAT* is the disgusting code, and it only deals with '%s' (and
the odd symbol relocation case that I think is completely broken).

And the '%s' case we could at least partly handle in lib/vsprintf.c -
not the odd tracing special cases (that you might as well just handle
in test_event_printk()), but the "we can check that it doesn't fault".

           Linus