Re: [PATCH] x86/ftrace: Fix ebp in ftrace_regs_caller that screws up unwinder

[Josh Poimboeuf]

On Thu, Apr 20, 2017 at 05:22:36PM -0400, Steven Rostedt (VMware) wrote:
> From: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
> 
> Fengguang Wu's zero day bot triggered a stack unwinder dump. This can
> be easily triggered when CONFIG_FRAME_POINTERS is enabled and -mfentry
> is in use on x86_32.
> 
>  ># cd /sys/kernel/debug/tracing
>  ># echo 'p:schedule schedule' > kprobe_events
>  ># echo stacktrace > events/kprobes/schedule/trigger
> 
> This is because the code that implemented fentry in the
> ftrace_regs_caller tried to use the least amount of #ifdefs, and
> modified ebp when CC_USE_FENTRY was defined to point to the parent ip
> as it does when CC_USE_FENTRY is not defined. But when
> CONFIG_FRAME_POINTERS is set, it corrupts the ebp register for this
> frame while doing the tracing.
> 
> NOTE, it does not corrupt ebp in any other way. It is just a bad
> frame pointer when calling into the tracing infrastructure. The original
> ebp is restored before returning from the fentry call. But if a stack
> trace is performed inside the tracing, the unwinder will notice the bad
> ebp.
> 
> Instead of toying with ebp with CC_USING_FENTRY, just slap the parent
> ip into the second parameter (%edx), and have an #else that does it the
> original way.
> 
> The unwinder will unfortunately miss the function being traced, as the
> stack frame is not set up yet for it, as it is for x86_64. But fixing
> that is a bit more complex and did not work before anyway.
> 
> This has been tested with and without FRAME_POINTERS being set while
> using -mfentry, as well as using an older compiler that uses mcount.
> 
> Reported-by: kernel test robot <fengguang.wu@xxxxxxxxx>
> Analyzed-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Link: https://lists.01.org/pipermail/lkp/2017-April/006165.html
> Fixes: 644e0e8dc76b ("x86/ftrace: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set")
> Signed-off-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>

It's actually CONFIG_FRAME_POINTER (no 'S').  Otherwise,

Reviewed-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>

-- 
Josh