linux-kernel - Re: [PATCH 05/19] x86/dumpstack: fix function graph tracing stack dump reliability issues

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 2 Aug 2016 17:16:10 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Josh Poimboeuf <jpoimboe@...hat.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...nel.org>,
	"H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	linux-kernel@...r.kernel.org,
	Andy Lutomirski <luto@...capital.net>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Brian Gerst <brgerst@...il.com>,
	Kees Cook <keescook@...omium.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Byungchul Park <byungchul.park@....com>
Subject: Re: [PATCH 05/19] x86/dumpstack: fix function graph tracing stack
 dump reliability issues

On Tue, 2 Aug 2016 16:00:11 -0500
Josh Poimboeuf <jpoimboe@...hat.com> wrote:

   [<ffffffff81061d8b>] nmi_raise_cpu_backtrace+0x1b/0x20
> 
> The ret_stack is out of sync with the stack dump because the stack dump
> was started with the regs from the NMI, instead of being started from
> the current frame.
> 
> So I guess there are a couple of ways to fix it:
> 
>   a) keep track of the return address pointer like we discussed above;
> 
>      or
> 
>   b) have the unwinder count the # of skipped frames which refer to
>      'return_to_handler', and pass that as the initial index value to
>      ftrace_graph_ret_addr().
> 
> Option a) would be much cleaner.  But to fix it for both mcount and
> fentry, we couldn't override 'fp' so I guess we'd need to add a new
> field to ftrace_ret_stack.

Actually, what about calling ftrace_graph_ret_addr() to figure out the
next stack conversion only if reliable or CONFIG_FRAME_POINTER is not
enabled?

	unsigned long real_addr = addr;

	[...]

	if (!IS_ENABLED(CONFIG_FRAME_POINTER) || reliable)
		real_addr = ftrace_graph_ret_addr(task, graph, addr);
	if (addr != real_addr)
		ops->address(data, addr, 0);
	ops->address(data, real_addr, reliable);

Then we only need the fp use case when FRAME_POINTER is not set. As
mcount forces FRAME_POINTER, we only need to worry about the fentry
case.

-- Steve

> 
> Option b) is uglier, but I could probably make it work with the new
> unwinder.
>