linux-kernel - Re: [PATCH 4/5] ftrace/x86: Have arch x86_64 use breakpoints instead of stop machine

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] [day] [month] [year] [list]

Date:	Thu, 26 Apr 2012 08:13:02 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Frederic Weisbecker <fweisbec@...il.com>,
	"H. Peter Anvin" <hpa@...or.com>, yrl.pp-manager.tt@...achi.com
Subject: Re: [PATCH 4/5] ftrace/x86: Have arch x86_64 use breakpoints
 instead of stop machine

On Thu, 2012-04-26 at 19:43 +0900, Masami Hiramatsu wrote:
> (2012/04/26 3:48), Steven Rostedt wrote:
> > From: Steven Rostedt <srostedt@...hat.com>
> > 
> > This method changes x86 to add a breakpoint to the mcount locations
> > instead of calling stop machine.
> > 
> > Now that iret can be handled by NMIs, we perform the following to
> > update code:
> > 
> > 1) Add a breakpoint to all locations that will be modified
> > 
> > 2) Sync all cores
> > 
> > 3) Update all locations to be either a nop or call (except breakpoint
> >    op)
> > 
> > 4) Sync all cores
> > 
> > 5) Remove the breakpoint with the new code.
> > 
> > 6) Sync all cores
> > 
> > [
> >   Added updates that Masami suggested:
> >    Use unlikely(modifying_ftrace_code) in int3 trap to keep kprobes efficient.
> >    Don't use NOTIFY_* in ftrace handler in int3 as it is not a notifier.
> > ]
> 
> I just have a few comments :)
> 

Thanks Masami,

below is the changes I made. I'll post a v2.

-- Steve

diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index f866c10..18d9005 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -51,8 +51,7 @@ struct dyn_arch_ftrace {
 	/* No extra data needed for x86 */
 };
 
-int ftrace_int3_handler(int cmd, const char *str,
-		struct pt_regs *regs, long err, int trap, int sig);
+int ftrace_int3_handler(struct pt_regs *regs);
 
 #endif /*  CONFIG_DYNAMIC_FTRACE */
 #endif /* __ASSEMBLY__ */
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 24108af..80af347 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -344,10 +344,9 @@ int modifying_ftrace_code __read_mostly;
  * call to a nop. While the change is taking place, we treat
  * it just like it was a nop.
  */
-int ftrace_int3_handler(int cmd, const char *str,
-		 struct pt_regs *regs, long err, int trap, int sig)
+int ftrace_int3_handler(struct pt_regs *regs)
 {
-	if (!modifying_ftrace_code || cmd != DIE_INT3 || !regs)
+	if (WARN_ON_ONCE(!regs))
 		return 0;
 
 	if (!ftrace_location(regs->ip - 1))
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 1712485..92d5756 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -306,10 +306,9 @@ gp_in_kernel:
 /* May run on IST stack. */
 dotraplinkage void __kprobes notrace do_int3(struct pt_regs *regs, long error_code)
 {
-#ifdef CONFIG_FUNCTION_TRACER
+#ifdef CONFIG_DYNAMIC_FTRACE
 	/* ftrace must be first, everything else may cause a recursive crash */
-	if (unlikely(modifying_ftrace_code) &&
-	    ftrace_int3_handler(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP))
+	if (unlikely(modifying_ftrace_code) && ftrace_int3_handler(regs))
 		return;
 #endif
 #ifdef CONFIG_KGDB_LOW_LEVEL_TRAP


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/