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

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]

Message-Id: <20171004003720.ee20bc25823c76cda8d63ef7@kernel.org>
Date:   Wed, 4 Oct 2017 00:37:20 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>,
        Ingo Molnar <mingo@...nel.org>
Cc:     Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        "David S . Miller" <davem@...emloft.net>,
        linux-kernel@...r.kernel.org, x86@...nel.org,
        Richard Weinberger <richard@...ma-star.at>
Subject: Re: [PATCH] x86/kprobes: Set up frame pointer in kprobe trampoline

On Tue,  3 Oct 2017 08:51:43 -0500
Josh Poimboeuf <jpoimboe@...hat.com> 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@...nel.org>

Thanks!

> 
> Reported-and-tested-by: Richard Weinberger <richard@...ma-star.at>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
> ---
>  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@...nel.org>