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

[Steven Rostedt]

On Sun, 15 Dec 2024 07:42:35 -0500
Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote:

> On 2024-12-15 05:05, Steven Rostedt wrote:
> > On Sat, 14 Dec 2024 21:19:01 -0800
> > Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:  
> 
> [...]
> 
> >>
> >> Just disable it unconditionally.
> >>  
> > 
> > I can do that, but I'm not looking forward to seeing random crashes in the
> > trace event code again :-(
> > 
> > Honestly, I did not like this code when I wrote it, but I have no idea how
> > to stop the "%s" bug from happening before it gets out to production. This
> > worked. Do you have any suggestions for alternatives?  
> 
> IMHO, deferred execution of TP_printk() code in kernel context is
> a fundamental mistake causing all those problems. This opens the
> door to store pointers to strings (or anything else really)
> that sit in kernel modules which can be unloaded between

Module unloading will clear out the ring buffers to prevent issues.

> tracing and TP_printk() execution, or as we are seeing here
> pointers to data which can be mapped at different addresses
> across kernel reboot, into the ring buffer.
> 
> If TP_printk() don't have access to load data from random kernel
> memory in the first place, and can only read from the buffer, we
> would not be having those misuses, and there would be nothing to
> work-around as the strings/data would all be serialized into the
> ring buffer.
> 
> In LTTng we've taken the approach to only read the trace data
> at post-processing from user-space (we don't have the equivalent
> of TP_printk(), and that's on purpose).
> 
> I wonder if we could keep the ftrace trace_pipe pretty-printing
> behavior, while isolating the TP_printk() execution into a
> userspace process which would only map the ring buffer ? This way,

That would change the entire use of tracefs, especially in the embedded
world. Note, this hasn't been a major issue since the test/check logic was
put in place. It catches pretty much all issues with the delayed printing.

-- Steve


> users trying to misuse TP_printk() would get immediate feedback
> about their mistake because they cannot print the trace. We could
> print a dmesg warning about crash of a usermode helper program,
> for instance.