Re: [PATCH 1/3] sched: add sched_task_call()

[Ingo Molnar <mingo@kernel.org>]

* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Fri, Feb 20, 2015 at 10:50:03AM +0100, Ingo Molnar wrote:
> > 
> > * Jiri Kosina <jkosina@suse.cz> wrote:
> > 
> > > Alright, so to sum it up:
> > > 
> > > - current stack dumping (even looking at /proc/<pid>/stack) is not 
> > >   guaranteed to yield "correct" results in case the task is running at the 
> > >   time the stack is being examined
> > 
> > Don't even _think_ about trying to base something as 
> > dangerous as live patching the kernel image on the concept 
> > of:
> > 
> >   'We can make all stack backtraces reliably correct all 
> >    the time, with no false positives, with no false
> >    negatives, 100% of the time, and quickly discover and
> >    fix bugs in that'.
> > 
> > It's not going to happen:
> > 
> >  - The correctness of stacktraces partially depends on
> >    tooling and we don't control those.
> 
> What tooling are you referring to?
> 
> Sure, we rely on the compiler to produce the correct 
> assembly for putting frame pointers on the stack. [...]

We also rely on all assembly code to have valid frames all 
the time - and this is far from a given, we've had many 
bugs in that area.

> [...]  But that's pretty straightforward. The kernel 
> already relies on the compiler to do plenty of things 
> which are much more complex than that.

So this claim scares me because it's such nonsense:

We rely on the compiler to create _functional code_ for us.  
If the compiler messes up then functionality, actual code 
breaks.

'Valid stack frame' is not functional code, it's in essence 
debug info. If a backtrace is wrong in some rare, racy case 
then that won't break the kernel, it just gives slightly 
misleading debug info in most cases.

Now you want to turn 'debug info' to have functional 
relevance, for something as intrusive as patching the 
kernel code.

You really need to accept this distinction!

> >  - More importantly, there's no strong force that ensures
> >    we can rely on stack backtraces: correcting bad 
> >    stack traces depends on people hitting those 
> >    functions and situations that generate them, seeing 
> >    a bad stack trace, noticing that it's weird and 
> >    correcting whatever code or tooling quirk causes the 
> >    stack entry to be incorrect.
> >
> > Essentially unlike other kernel code which breaks stuff 
> > if it's incorrect, there's no _functional_ dependence 
> > on stack traces, so live patching would be the first 
> > (and pretty much only) thing that breaks on bad stack 
> > traces ...
> 
> First, there are several things we do to avoid anomalies:
> 
> - we don't patch asm functions

the problem with asm functions isn't that we might want to 
patch them (although sometimes we might want to: especially 
the more interesting security fixes tend to be in assembly 
code), it's that asm functions can (easily) mess up debug 
info, without that being apparent in any functional way.

> - we only walk the stack of sleeping tasks

sleeping tasks are affected by debug info bugs too.

> Second, currently the stack unwinding code is rather 
> crude and doesn't do much in the way of error handling.

That's a big red flag in my book ...

Thanks,

	Ingo