Re: [PATCH] x86/kprobes: Set up frame pointer in kprobe trampoline

[Masami Hiramatsu]

On Tue,  3 Oct 2017 08:51:43 -0500
Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:

> Richard Weinberger saw an unwinder warning when running bcc's opensnoop:
> 
>   WARNING: kernel stack frame pointer at ffff99ef4076bea0 in opensnoop:2008 has bad value 0000000000000008
>   unwind stack type:0 next_sp:          (null) mask:0x2 graph_idx:0
>   ...
>   ffff99ef4076be88: ffff99ef4076bea0 (0xffff99ef4076bea0)
>   ffff99ef4076be90: ffffffffac442721 (optimized_callback +0x81/0x90)
>   ...
> 
> A lockdep stack trace was initiated from inside a kprobe handler, when
> the unwinder noticed a bad frame pointer on the stack.  The bad frame
> pointer is related to the fact that the kprobe optprobe trampoline
> doesn't save the frame pointer before calling into optimized_callback().

OK, this looks good to me :)

Acked-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>

Thanks!

> 
> Reported-and-tested-by: Richard Weinberger <richard@xxxxxxxxxxxxx>
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> ---
>  arch/x86/kernel/kprobes/common.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/kprobes/common.h b/arch/x86/kernel/kprobes/common.h
> index e2c2a1970869..595b56b37d27 100644
> --- a/arch/x86/kernel/kprobes/common.h
> +++ b/arch/x86/kernel/kprobes/common.h
> @@ -3,6 +3,15 @@
>  
>  /* Kprobes and Optprobes common header */
>  
> +#include <asm/asm.h>
> +
> +#ifdef CONFIG_FRAME_POINTER
> +# define SAVE_RBP_STRING "	push %" _ASM_BP "\n" \
> +			 "	mov  %" _ASM_SP ", %" _ASM_BP "\n"
> +#else
> +# define SAVE_RBP_STRING "	push %" _ASM_BP "\n"
> +#endif
> +
>  #ifdef CONFIG_X86_64
>  #define SAVE_REGS_STRING			\
>  	/* Skip cs, ip, orig_ax. */		\
> @@ -17,7 +26,7 @@
>  	"	pushq %r10\n"			\
>  	"	pushq %r11\n"			\
>  	"	pushq %rbx\n"			\
> -	"	pushq %rbp\n"			\
> +	SAVE_RBP_STRING				\
>  	"	pushq %r12\n"			\
>  	"	pushq %r13\n"			\
>  	"	pushq %r14\n"			\
> @@ -48,7 +57,7 @@
>  	"	pushl %es\n"			\
>  	"	pushl %ds\n"			\
>  	"	pushl %eax\n"			\
> -	"	pushl %ebp\n"			\
> +	SAVE_RBP_STRING				\
>  	"	pushl %edi\n"			\
>  	"	pushl %esi\n"			\
>  	"	pushl %edx\n"			\
> -- 
> 2.13.6
> 


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>