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, 7 May 2014 18:40:14 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Frederic Weisbecker <fweisbec@...il.com>
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


* Frederic Weisbecker <fweisbec@...il.com> wrote:

> 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.

Agreed, that's a very good point.

Also, consider the following holistic argument, what is easier to 
achieve, when looking at an oops and not seeing the bug:

  - if only I had more information
  - if only I had less information

we cannot put in information that we cut out, but it's not 
particularly hard to skip overly verbose information in most cases.
 
Yes, there's a line to be drawn with verbosity: scroll-off is a 
concern when the oops does not make it to a log file.

So I don't really know.

Thanks,

	Ingo
--
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/