Re: [PATCH 4/6] ftrace, x86: make kernel text writable only for conversions

Steven Rostedt
2009-02-23T16:48:46Z

On Mon, 23 Feb 2009, Mathieu Desnoyers wrote:
> It can, by using your function tracer. It has a mode where it can
> enable/disable a filter in a callback connected on tracepoints. This
> filter is then used to enable detailed function tracing for a short time
> window. Also, you could think of tracing every function calls with
> LTTng's flight recorder mode, which only spins in memory overwriting the
> oldest information. That would provide snapshots on demand of the last
> functions called.
>
> > Now, yes, if you only select a few functions, there's no noticeable
> > overhead. And yes then you would need to do the stop_machine anyway, and
> > there will be a small window where the kernel text will be writable.
> > Tracing only a small set of functions (say a few 100) is not much of an
> > overhead, and I could see that being done on a production system.
> >
>
> This is what LTTng can do today. But that involves the function tracer
> stop_machine() call, which I dislike.

What's wrong with stop_machine? Specifically, what do you dislike about
it?

>
> > >
> > > I agree that the racy time window is not that large and is not really a
> > > security concern, but it's still just annoying.
> >
> > Annoying? how so?
> >
> > Again, the stop_machine part has nothing to do with DEBUG_RODATA, it is
> > about the safest and easiest way to modify kernel text.
> >
>
> We are running in circles here because there is no real argument
> brought.
>
> 1 - You claim that changing the kernel's mapping, which has been
> pointed out as an intrusive kernel modification, is faster than using a
> text-poke-like approach. Please provide numbers to support such claims.

Hmm, lets see. I simply set a bit in the PTE mappings. There's not many,
since a lot are 2M pages, for x86_64. Call stop_machine, and now I can
modify 1 or 20,000 locations. Set the PTE bit back. Note, the changing of
the bits are only done when CONFIG_DEBUG_RODATA is set.

text_poke requires allocating a page. Map the page into memory. Set up a
break point. Knowing what to do when that break point is hit by another
process. Modify the one location. Unmap the page. Free the page. Remove
the breakpoint.

Yes, this may be faster if I only modify one location. I would be hard
pressed that this is faster when I modify a few hundred locations.
The stop_machine method does it all at once. Not one at a time.

>
> 2 - You claim that using stop_machine is simpler and therefore safer
> than using a breakpoint-based approach. I start having some doubts about
> simplicity when you start talking about the workarounds you have to do
> for NMIs,

I agree, the NMI work around was tricky, but the final solution (which
we tested vigorously) works well. My claim that it is simpler is not about
the small steps, but rather the number of variables we need to deal with.
Stop machine shuts down all the CPUs and executes my code on one CPU.
Interrupts are disabled on all CPUs, and we only need to worry about the
NMI. Which we now do.

Your solution is about mapping another page on a running system, where
anything can happen. The number of variables that can go wrong is much
greater simply by the fact that you have no idea as to what is running at
the same time as you perform your modifications.

With stop_machine, the number of variables is much less, because I know
everything that is happening when I do the modification. I do not need to
worry about some strange driver doing some kind of tricks because it
simply is not running.

> but more importantly, you seem to recognise that the latency
> it induces would be inadequate for production systems.

Wrong. I recognise the latency of tracing all functions on a production
system. Heck, we trace spin_lock, rcu_read_lock, mutex_lock, and all that
jazz. Just slowing those functions down a bit will have a noticeable
impact. I've found that adding those functions to set_ftrace_notrace drops
the function tracer penalty, significantly.

> Therefore it's
> unusable in some LTTng use-cases just because of that. If you expect the
> function tracer to become used more widely in LTTng, these concerns
> should be addressed.

If you only want to trace a few hundred functions, then the overhead with
it on should not be significant. Depending on which functions you trace.
As mentioned above, tracing only spin_lock can slow the system down.

Set up the functions you want to trace, enable them. You can have the
ring buffer disabled (echo 0 > /debug/tracing/tracing_on) and just turn on
the ring buffer for your snapshot, and turn it off when you are done. When
all tracing is done, then disable the function tracing.

>
> If, in the end, your argument is "the function tracer works as-is now,
> and I have no time to change it given it represents too much work" or "I
> don't care about your use-cases", I'm OK with that. But please then don't
> argue that it's because it's the best technical solution when it isn't.

No, I have yet to hear a valuable argument against stop_machine. You are
pushing the burden of proof on me, when we have something that does work,
on several archs. You want me to redesign the system to be x86 only, and
then say, hey, my original code works better.

I do not see text_poke being theoretically better. The only reason you
given me to use it is because you dislike stop_machine.

-- Steve