linux-kernel - Re: [PATCH] x86/dumpstack: Walk frames when built with frame pointers

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]

Date:	Wed, 30 Apr 2014 23:56:08 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Richard Yao <ryao@...too.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tejun Heo <tj@...nel.org>, Vineet Gupta <vgupta@...opsys.com>,
	Jesper Nilsson <jesper.nilsson@...s.com>,
	Jiri Slaby <jslaby@...e.cz>, linux-kernel@...r.kernel.org,
	kernel@...too.org, Brian Behlendorf <behlendorf1@...l.gov>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Arnaldo Carvalho de Melo <acme@...radead.org>,
	Jiri Olsa <jolsa@...hat.com>
Subject: Re: [PATCH] x86/dumpstack: Walk frames when built with frame pointers

On Sun, Apr 27, 2014 at 02:08:20PM +0200, Ingo Molnar wrote:
> 
> * Richard Yao <ryao@...too.org> wrote:
> 
> > Stack traces are generated by scanning the stack and interpeting 
> > anything that looks like it could be a pointer to something. We do 
> > not need to do this when we have frame pointers, but we do it 
> > anyway, with the distinction that we use the return pointers to mark 
> > actual frames by the absence of a question mark.
> > 
> > The additional verbosity of stack scanning tends to bombard us with 
> > walls of text for no gain in practice, so lets switch to printing 
> > only stack frames when frame pointers are available. That we can 
> > spend less time reading stack traces and more time looking at code.
> > 
> > Signed-off-by: Richard Yao <ryao@...too.org>
> > ---
> >  arch/x86/kernel/dumpstack.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> > index d9c12d3..94ffe06 100644
> > --- a/arch/x86/kernel/dumpstack.c
> > +++ b/arch/x86/kernel/dumpstack.c
> > @@ -162,7 +162,11 @@ static void print_trace_address(void *data, unsigned long addr, int reliable)
> >  static const struct stacktrace_ops print_trace_ops = {
> >  	.stack			= print_trace_stack,
> >  	.address		= print_trace_address,
> > +#ifdef CONFIG_FRAME_POINTER
> > +	.walk_stack		= print_context_stack_bp,
> > +#else
> >  	.walk_stack		= print_context_stack,
> > +#endif
> >  };

Besides the complementary informations brought by the full stack walk,
another big argument toward keeping full stack walk is that if your frame
pointer is screwed for whatever reason, you still have a useful stack trace.

I have seen and fixed several broken frame links in x86-64 by the past. Those
are very subtle and often hardly visible issues because, if they are easily spotted
on common frame scenarios like : task > irq, they are much harder to find on trickier,
rarer frame scenarios such as: task -> softirq -> irq -> nmi -> debug exception ->....

For example before a2bbe75089d5eb9a3a46d50dd5c215e213790288
("x86: Don't use frame pointer to save old stack on irq entry"), we were missing
entire stack frames on nesting irqs (hardirq on softirqs) while using pure frame
pointer based unwinding.

Who knows if we have other remaining issues like this? Especially given the high
possible number of frame combinations between task, irq, softirq, nmi and exceptions.
Multiply the contexts possibility by the number of possible archs out there and their
stack switch implementations.

Also further frame links breakages, we have many other possibilities to end up
with misleading frame pointers. Relying on that source alone definetly reduce the
reliability of our stacktraces.

So this goes way beyond just missing complementary informations. Debugging robustness
itself is actually very concerned here if we remove the full stack walk.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/