linux-kernel - [PATCH 0/5 v2] ftrace/x86_32: Ftrace cleanup and add support for -mfentry

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-next>] [day] [month] [year] [list]
Date:   Thu, 16 Mar 2017 13:20:08 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     linux-kernel@...r.kernel.org
Cc:     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: [PATCH 0/5 v2] ftrace/x86_32: Ftrace cleanup and add support for -mfentry

With the issues of gcc screwing around with the mcount stack frame causing
function graph tracer to panic on x86_32, and with Linus saying that we
should start deprecating mcount (at least on x86), I figured that x86_32
needs to support fentry.

First, I renamed mcount_64.S to ftrace_64.S. As we want to get away from
mcount, having the ftrace code in a file called mcount seems rather backwards.

Next I moved the ftrace code out of entry_32.S. It's not in entry_64.S
and it does not belong in entry_32.S.

I noticed that the x86_32 code has the same issue as the x86_64 did
in the past with respect to a stack frame. I fixed that just for the main
ftrace_caller. The ftrace_regs_caller is rather special, and so is
function graph tracing.

I realized the ftrace_regs_caller code was complex due to me aggressively
saving flags, even though I could still do push, lea and mov without
changing them. That made the logic a little nicer.

Finally I added the fentry code.

I tested this with an older compiler (for mcount) with and without
FRAME_POINTER set. I also did it with a new compiler (with fentry), with and
without FRAME_POINTER. I tested function tracing, stack tracing, function_graph
tracing, and kprobes (as that uses the ftrace_regs_caller).

Steven Rostedt (VMware) (5):
      x86/ftrace: Rename mcount_64.S to ftrace_64.S
      ftrace/x86-32: Move the ftrace specific code out of entry_32.S
      ftrace/x86_32: Add stack frame pointer to ftrace_caller
      ftrace/x86_32: Clean up ftrace_regs_caller
      ftrace/x86-32: Add -mfentry support to x86_32 with DYNAMIC_FTRACE set

----
 Makefile                                     |  12 +-
 arch/x86/Kconfig                             |   2 +-
 arch/x86/entry/entry_32.S                    | 168 ------------------
 arch/x86/kernel/Makefile                     |   5 +-
 arch/x86/kernel/ftrace_32.S                  | 250 +++++++++++++++++++++++++++
 arch/x86/kernel/{mcount_64.S => ftrace_64.S} |   0
 6 files changed, 260 insertions(+), 177 deletions(-)
 create mode 100644 arch/x86/kernel/ftrace_32.S
 rename arch/x86/kernel/{mcount_64.S => ftrace_64.S} (100%)

Changes from v1:

  Removed MCOUNT_FRAME_SIZE macro, as it is not used.
    (Masami Hiramatsu)

  Fixed vertical alignment of args (Josh Poimboeuf)

  s/ndef USING_FRAME_POINTER/def CC_USING_FENTRY/ (Josh Poimboeuf)

  Fixed adding $0 into orig_ax and not ax. (Josh Poimboeuf)

  Updated some comments.

Here's the diff of v1 to v2:

diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 4bf8223..7ed4137 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -9,7 +9,6 @@
 #include <asm/export.h>
 #include <asm/ftrace.h>
 
-
 #ifdef CC_USING_FENTRY
 # define function_hook	__fentry__
 EXPORT_SYMBOL(__fentry__)
@@ -24,14 +23,8 @@ EXPORT_SYMBOL(mcount)
 #endif
 
 #ifdef USING_FRAME_POINTER
-# ifdef CC_USING_FENTRY
-#  define MCOUNT_FRAME_SIZE		(4*4)	/* bp,ip and parent's  */
-# else
-#  define MCOUNT_FRAME_SIZE		4	/* just the bp         */
-# endif
 # define MCOUNT_FRAME			1	/* using frame = true  */
 #else
-# define MCOUNT_FRAME_SIZE		0	/* no stack frame      */
 # define MCOUNT_FRAME			0	/* using frame = false */
 #endif
 
@@ -52,14 +45,14 @@ ENTRY(ftrace_caller)
 	 * 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 */
+	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
+	pushl	%ebp
+	movl	%esp, %ebp
 #endif
 	pushl	%eax
 	pushl	%ecx
@@ -131,8 +124,8 @@ ENTRY(ftrace_regs_caller)
 
 	/* move flags into the location of where the return ip was */
 	movl	5*4(%esp), %eax
-	movl	$0, (%esp)			/* Load 0 into orig_ax */
-	movl	%eax, 8*4(%esp)
+	movl	$0, 5*4(%esp)			/* Load 0 into orig_ax */
+	movl	%eax, 8*4(%esp)			/* Load flags in return ip */
 
 	pushl	%ebp
 	pushl	%edi
@@ -140,7 +133,7 @@ ENTRY(ftrace_regs_caller)
 	pushl	%edx
 	pushl	%ecx
 	pushl	%ebx
-#ifndef USING_FRAME_POINTER
+#ifdef CC_USING_FENTRY
 	/* Load 4 off of the parent ip addr into ebp */
 	lea	14*4(%esp), %ebp
 #endif
@@ -160,7 +153,7 @@ GLOBAL(ftrace_regs_call)
 	movl	14*4(%esp), %eax
 	movl	%eax, 13*4(%esp)
 
-	/* Move ip back to its original location */
+	/* Move return ip back to its original location */
 	movl	12*4(%esp), %eax
 	movl	%eax, 14*4(%esp)