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

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2024-12-15 05:05, Steven Rostedt wrote:
> On Sat, 14 Dec 2024 21:19:01 -0800
> Linus Torvalds <torvalds@linux-foundation.org> 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
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,
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.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com