| 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
| ||
|
Message-Id: <20170317193824.b769ef33a0fe91d5842ce6c6@kernel.org>
Date: Fri, 17 Mar 2017 19:38:24 +0900
From: Masami Hiramatsu <mhiramat@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
"H. Peter Anvin" <hpa@...or.com>,
Andy Lutomirski <luto@...capital.net>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 5/5 v3] ftrace/x86-32: Add -mfentry support to x86_32
with DYNAMIC_FTRACE set
On Thu, 16 Mar 2017 16:40:36 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:
> [
> Fengguang Wu's bot told me that it failed to compile, as ftrace_32.S
> always gets compiled (as does ftrace_64.S). And the content needs to
> be protected within the #ifdef.
> ]
>
> From: "Steven Rostedt (VMware)" <rostedt@...dmis.org>
>
> x86_64 has had fentry support for some time. I did not add support to x86_32
> as I was unsure if it will be used much in the future. It is still very much
> used, and there's issues with function graph tracing with gcc playing around
> with the mcount frames, causing function graph to panic. The fentry code
> does not have this issue, and is able to cope as there is no frame to mess
> up.
>
> Note, this only add support for fentry when DYNAMIC_FTRACE is set. There's
> really no reason to not have that set, because the performance of the
> machine drops significantly when it's not enabled. I only keep
> !DYNAMIC_FTRACE around to test it off, as there's still some archs that have
> FTRACE but not DYNAMIC_FTRACE.
Looks good to me.
Reviewed-by: Masami Hiramatsu <mhiramat@...nel.org>
Thanks!
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
> ---
> arch/x86/Kconfig | 2 +-
> arch/x86/kernel/ftrace_32.S | 81 +++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 72 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index cc98d5a..8c17146 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -127,7 +127,7 @@ config X86
> select HAVE_EBPF_JIT if X86_64
> select HAVE_EFFICIENT_UNALIGNED_ACCESS
> select HAVE_EXIT_THREAD
> - select HAVE_FENTRY if X86_64
> + select HAVE_FENTRY if X86_64 || DYNAMIC_FTRACE
> select HAVE_FTRACE_MCOUNT_RECORD
> select HAVE_FUNCTION_GRAPH_TRACER
> select HAVE_FUNCTION_TRACER
> diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
> index 068a582..8530567 100644
> --- a/arch/x86/kernel/ftrace_32.S
> +++ b/arch/x86/kernel/ftrace_32.S
> @@ -12,24 +12,65 @@
> #ifdef CONFIG_FUNCTION_TRACER
> #ifdef CONFIG_DYNAMIC_FTRACE
>
> -ENTRY(mcount)
> +#ifdef CC_USING_FENTRY
> +# define function_hook __fentry__
> +EXPORT_SYMBOL(__fentry__)
> +#else
> +# define function_hook mcount
> +EXPORT_SYMBOL(mcount)
> +#endif
> +
> +/* mcount uses a frame pointer even if CONFIG_FRAME_POINTER is not set */
> +#if !defined(CC_USING_FENTRY) || defined(CONFIG_FRAME_POINTER)
> +# define USING_FRAME_POINTER
> +#endif
> +
> +#ifdef USING_FRAME_POINTER
> +# define MCOUNT_FRAME 1 /* using frame = true */
> +#else
> +# define MCOUNT_FRAME 0 /* using frame = false */
> +#endif
> +
> +ENTRY(function_hook)
> ret
> -END(mcount)
> +END(function_hook)
>
> ENTRY(ftrace_caller)
>
> +#ifdef USING_FRAME_POINTER
> +# ifdef CC_USING_FENTRY
> + /*
> + * Frame pointers are of ip followed by bp.
> + * Since fentry is an immediate jump, we are left with
> + * parent-ip, function-ip. We need to add a frame with
> + * parent-ip followed by ebp.
> + */
> + pushl 4(%esp) /* parent ip */
> pushl %ebp
> movl %esp, %ebp
> -
> + pushl 2*4(%esp) /* function ip */
> +# endif
> + /* For mcount, the function ip is directly above */
> + pushl %ebp
> + movl %esp, %ebp
> +#endif
> pushl %eax
> pushl %ecx
> pushl %edx
> pushl $0 /* Pass NULL as regs pointer */
> - movl 5*4(%esp), %eax
> - /* Copy original ebp into %edx */
> +
> +#ifdef USING_FRAME_POINTER
> + /* Load parent ebp into edx */
> movl 4*4(%esp), %edx
> +#else
> + /* There's no frame pointer, load the appropriate stack addr instead */
> + lea 4*4(%esp), %edx
> +#endif
> +
> + movl (MCOUNT_FRAME+4)*4(%esp), %eax /* load the rip */
> /* Get the parent ip */
> - movl 0x4(%edx), %edx
> + movl 4(%edx), %edx /* edx has ebp */
> +
> movl function_trace_op, %ecx
> subl $MCOUNT_INSN_SIZE, %eax
>
> @@ -41,7 +82,14 @@ ftrace_call:
> popl %edx
> popl %ecx
> popl %eax
> +#ifdef USING_FRAME_POINTER
> popl %ebp
> +# ifdef CC_USING_FENTRY
> + addl $4,%esp /* skip function ip */
> + popl %ebp /* this is the orig bp */
> + addl $4, %esp /* skip parent ip */
> +# endif
> +#endif
> .Lftrace_ret:
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> .globl ftrace_graph_call
> @@ -82,6 +130,10 @@ ENTRY(ftrace_regs_caller)
> pushl %edx
> pushl %ecx
> pushl %ebx
> +#ifdef CC_USING_FENTRY
> + /* Load 4 off of the parent ip addr into ebp */
> + lea 14*4(%esp), %ebp
> +#endif
>
> movl 12*4(%esp), %eax /* Load ip (1st parameter) */
> subl $MCOUNT_INSN_SIZE, %eax /* Adjust ip */
> @@ -120,7 +172,7 @@ GLOBAL(ftrace_regs_call)
> jmp .Lftrace_ret
> #else /* ! CONFIG_DYNAMIC_FTRACE */
>
> -ENTRY(mcount)
> +ENTRY(function_hook)
> cmpl $__PAGE_OFFSET, %esp
> jb ftrace_stub /* Paging not enabled yet? */
>
> @@ -152,9 +204,8 @@ ftrace_stub:
> popl %ecx
> popl %eax
> jmp ftrace_stub
> -END(mcount)
> +END(function_hook)
> #endif /* CONFIG_DYNAMIC_FTRACE */
> -EXPORT_SYMBOL(mcount)
> #endif /* CONFIG_FUNCTION_TRACER */
>
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> @@ -162,9 +213,15 @@ ENTRY(ftrace_graph_caller)
> pushl %eax
> pushl %ecx
> pushl %edx
> - movl 0xc(%esp), %eax
> + movl 3*4(%esp), %eax
> + /* Even with frame pointers, fentry doesn't have one here */
> +#ifdef CC_USING_FENTRY
> + lea 4*4(%esp), %edx
> + movl $0, %ecx
> +#else
> lea 0x4(%ebp), %edx
> movl (%ebp), %ecx
> +#endif
> subl $MCOUNT_INSN_SIZE, %eax
> call prepare_ftrace_return
> popl %edx
> @@ -177,7 +234,11 @@ END(ftrace_graph_caller)
> return_to_handler:
> pushl %eax
> pushl %edx
> +#ifdef CC_USING_FENTRY
> + movl $0, %eax
> +#else
> movl %ebp, %eax
> +#endif
> call ftrace_return_to_handler
> movl %eax, %ecx
> popl %edx
> --
> 2.9.3
>
--
Masami Hiramatsu <mhiramat@...nel.org>
Powered by blists - more mailing lists