ftrace: nmi safe code modification
authorSteven Rostedt <rostedt@goodmis.org>
Thu, 30 Oct 2008 20:08:32 +0000 (16:08 -0400)
committerIngo Molnar <mingo@elte.hu>
Thu, 30 Oct 2008 20:30:08 +0000 (21:30 +0100)
Impact: fix crashes that can occur in NMI handlers, if their code is modified

Modifying code is something that needs special care. On SMP boxes,
if code that is being modified is also being executed on another CPU,
that CPU will have undefined results.

The dynamic ftrace uses kstop_machine to make the system act like a
uniprocessor system. But this does not address NMIs, that can still
run on other CPUs.

One approach to handle this is to make all code that are used by NMIs
not be traced. But NMIs can call notifiers that spread throughout the
kernel and this will be very hard to maintain, and the chance of missing
a function is very high.

The approach that this patch takes is to have the NMIs modify the code
if the modification is taking place. The way this works is that just
writing to code executing on another CPU is not harmful if what is
written is the same as what exists.

Two buffers are used: an IP buffer and a "code" buffer.

The steps that the patcher takes are:

 1) Put in the instruction pointer into the IP buffer
    and the new code into the "code" buffer.
 2) Set a flag that says we are modifying code
 3) Wait for any running NMIs to finish.
 4) Write the code
 5) clear the flag.
 6) Wait for any running NMIs to finish.

If an NMI is executed, it will also write the pending code.
Multiple writes are OK, because what is being written is the same.
Then the patcher must wait for all running NMIs to finish before
going to the next line that must be patched.

This is basically the RCU approach to code modification.

Thanks to Ingo Molnar for suggesting the idea, and to Arjan van de Ven
for his guidence on what is safe and what is not.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
arch/arm/include/asm/ftrace.h
arch/powerpc/include/asm/ftrace.h
arch/sh/include/asm/ftrace.h
arch/sparc/include/asm/ftrace.h
arch/x86/include/asm/ftrace.h
arch/x86/kernel/ftrace.c
include/linux/hardirq.h

index 39c8bc1a006afbfbca896948ae86c4268e7ed52e..d4c24a7a92805eb3a87d1e682cdaed5ed288d2a5 100644 (file)
@@ -1,6 +1,11 @@
 #ifndef _ASM_ARM_FTRACE
 #define _ASM_ARM_FTRACE
 
+#ifndef __ASSEMBLY__
+#define ftrace_nmi_enter()     do { } while (0)
+#define ftrace_nmi_exit()      do { } while (0)
+#endif
+
 #ifdef CONFIG_FUNCTION_TRACER
 #define MCOUNT_ADDR            ((long)(mcount))
 #define MCOUNT_INSN_SIZE       4 /* sizeof mcount call */
index b298f7a631e6cd9620094c72da4c82832afcdb43..7652755dc00066d0384875844ac57c41bcda5054 100644 (file)
@@ -1,6 +1,11 @@
 #ifndef _ASM_POWERPC_FTRACE
 #define _ASM_POWERPC_FTRACE
 
+#ifndef __ASSEMBLY__
+#define ftrace_nmi_enter()     do { } while (0)
+#define ftrace_nmi_exit()      do { } while (0)
+#endif
+
 #ifdef CONFIG_FUNCTION_TRACER
 #define MCOUNT_ADDR            ((long)(_mcount))
 #define MCOUNT_INSN_SIZE       4 /* sizeof mcount call */
index 3aed362c9463a6ccb492f6ef20ae4bb14ce27a75..cdf2cb0b9ffe4e945ca9d9017b68d2039739554f 100644 (file)
@@ -1,6 +1,11 @@
 #ifndef __ASM_SH_FTRACE_H
 #define __ASM_SH_FTRACE_H
 
+#ifndef __ASSEMBLY__
+#define ftrace_nmi_enter()     do { } while (0)
+#define ftrace_nmi_exit()      do { } while (0)
+#endif
+
 #ifndef __ASSEMBLY__
 extern void mcount(void);
 #endif
index d27716cd38c1ac0243a74104f3106850d24397b1..33a95feeb137350ac3c1269a4fdbaee6475cb254 100644 (file)
@@ -1,6 +1,11 @@
 #ifndef _ASM_SPARC64_FTRACE
 #define _ASM_SPARC64_FTRACE
 
+#ifndef __ASSEMBLY__
+#define ftrace_nmi_enter()     do { } while (0)
+#define ftrace_nmi_exit()      do { } while (0)
+#endif
+
 #ifdef CONFIG_MCOUNT
 #define MCOUNT_ADDR            ((long)(_mcount))
 #define MCOUNT_INSN_SIZE       4 /* sizeof mcount call */
index 9e8bc29b8b17dd3739d7479af6920c3c627130cb..f2ed6b704a75b9dbe8b79df52f31899cdb91bbb1 100644 (file)
@@ -17,6 +17,21 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
         */
        return addr - 1;
 }
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+extern void ftrace_nmi_enter(void);
+extern void ftrace_nmi_exit(void);
+#else
+#define ftrace_nmi_enter()     do { } while (0)
+#define ftrace_nmi_exit()      do { } while (0)
+#endif
+#endif
+
+#else /* CONFIG_FUNCTION_TRACER */
+
+#ifndef __ASSEMBLY__
+#define ftrace_nmi_enter()     do { } while (0)
+#define ftrace_nmi_exit()      do { } while (0)
 #endif
 
 #endif /* CONFIG_FUNCTION_TRACER */
index 50ea0ac8c9bf2c27a53323b93b5d473bd7e1d028..fe5f859130b5841d68b55b2861e7b506921ccd8e 100644 (file)
@@ -56,6 +56,111 @@ unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
        return calc.code;
 }
 
+/*
+ * Modifying code must take extra care. On an SMP machine, if
+ * the code being modified is also being executed on another CPU
+ * that CPU will have undefined results and possibly take a GPF.
+ * We use kstop_machine to stop other CPUS from exectuing code.
+ * But this does not stop NMIs from happening. We still need
+ * to protect against that. We separate out the modification of
+ * the code to take care of this.
+ *
+ * Two buffers are added: An IP buffer and a "code" buffer.
+ *
+ * 1) Put in the instruction pointer into the IP buffer
+ *    and the new code into the "code" buffer.
+ * 2) Set a flag that says we are modifying code
+ * 3) Wait for any running NMIs to finish.
+ * 4) Write the code
+ * 5) clear the flag.
+ * 6) Wait for any running NMIs to finish.
+ *
+ * If an NMI is executed, the first thing it does is to call
+ * "ftrace_nmi_enter". This will check if the flag is set to write
+ * and if it is, it will write what is in the IP and "code" buffers.
+ *
+ * The trick is, it does not matter if everyone is writing the same
+ * content to the code location. Also, if a CPU is executing code
+ * it is OK to write to that code location if the contents being written
+ * are the same as what exists.
+ */
+
+static atomic_t in_nmi;
+static int mod_code_status;
+static int mod_code_write;
+static void *mod_code_ip;
+static void *mod_code_newcode;
+
+static void ftrace_mod_code(void)
+{
+       /*
+        * Yes, more than one CPU process can be writing to mod_code_status.
+        *    (and the code itself)
+        * But if one were to fail, then they all should, and if one were
+        * to succeed, then they all should.
+        */
+       mod_code_status = probe_kernel_write(mod_code_ip, mod_code_newcode,
+                                            MCOUNT_INSN_SIZE);
+
+}
+
+void ftrace_nmi_enter(void)
+{
+       atomic_inc(&in_nmi);
+       /* Must have in_nmi seen before reading write flag */
+       smp_mb();
+       if (mod_code_write)
+               ftrace_mod_code();
+}
+
+void ftrace_nmi_exit(void)
+{
+       /* Finish all executions before clearing in_nmi */
+       smp_wmb();
+       atomic_dec(&in_nmi);
+}
+
+static void wait_for_nmi(void)
+{
+       while (atomic_read(&in_nmi))
+               cpu_relax();
+}
+
+static int
+do_ftrace_mod_code(unsigned long ip, void *new_code)
+{
+       mod_code_ip = (void *)ip;
+       mod_code_newcode = new_code;
+
+       /* The buffers need to be visible before we let NMIs write them */
+       smp_wmb();
+
+       mod_code_write = 1;
+
+       /* Make sure write bit is visible before we wait on NMIs */
+       smp_mb();
+
+       wait_for_nmi();
+
+       /* Make sure all running NMIs have finished before we write the code */
+       smp_mb();
+
+       ftrace_mod_code();
+
+       /* Make sure the write happens before clearing the bit */
+       smp_wmb();
+
+       mod_code_write = 0;
+
+       /* make sure NMIs see the cleared bit */
+       smp_mb();
+
+       wait_for_nmi();
+
+       return mod_code_status;
+}
+
+
 int
 ftrace_modify_code(unsigned long ip, unsigned char *old_code,
                   unsigned char *new_code)
@@ -81,7 +186,7 @@ ftrace_modify_code(unsigned long ip, unsigned char *old_code,
                return -EINVAL;
 
        /* replace the text with the new text */
-       if (probe_kernel_write((void *)ip, new_code, MCOUNT_INSN_SIZE))
+       if (do_ftrace_mod_code(ip, new_code))
                return -EPERM;
 
        sync_core();
index 181006cc94a03ecd29630d80d91762589fc1c316..0087cb43becf9059676f3396f5297aab796b3968 100644 (file)
@@ -5,6 +5,7 @@
 #include <linux/smp_lock.h>
 #include <linux/lockdep.h>
 #include <asm/hardirq.h>
+#include <asm/ftrace.h>
 #include <asm/system.h>
 
 /*
@@ -161,7 +162,17 @@ extern void irq_enter(void);
  */
 extern void irq_exit(void);
 
-#define nmi_enter()            do { lockdep_off(); __irq_enter(); } while (0)
-#define nmi_exit()             do { __irq_exit(); lockdep_on(); } while (0)
+#define nmi_enter()                            \
+       do {                                    \
+               ftrace_nmi_enter();             \
+               lockdep_off();                  \
+               __irq_enter();                  \
+       } while (0)
+#define nmi_exit()                             \
+       do {                                    \
+               __irq_exit();                   \
+               lockdep_on();                   \
+               ftrace_nmi_exit();              \
+       } while (0)
 
 #endif /* LINUX_HARDIRQ_H */