Commit e9d9df44 authored by Lai Jiangshan's avatar Lai Jiangshan Committed by Steven Rostedt

ftrace: protect running nmi (V3)

When I review the sensitive code ftrace_nmi_enter(), I found
the atomic variable nmi_running does protect NMI VS do_ftrace_mod_code(),
but it can not protects NMI(entered nmi) VS NMI(ftrace_nmi_enter()).

cpu#1                   | cpu#2                 | cpu#3
ftrace_nmi_enter()      | do_ftrace_mod_code()  |
  not modify            |                       |
------------------------|-----------------------|--
executing               | set mod_code_write = 1|
executing             --|-----------------------|--------------------
executing               |                       | ftrace_nmi_enter()
executing               |                       |    do modify
------------------------|-----------------------|-----------------
ftrace_nmi_exit()       |                       |

cpu#3 may be being modified the code which is still being executed on cpu#1,
it will have undefined results and possibly take a GPF, this patch
prevents it occurred.
Signed-off-by: default avatarLai Jiangshan <laijs@cn.fujitsu.com>
LKML-Reference: <49C0B411.30003@cn.fujitsu.com>
Signed-off-by: default avatarSteven Rostedt <srostedt@redhat.com>
parent 4acd4d00
...@@ -79,11 +79,11 @@ static unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr) ...@@ -79,11 +79,11 @@ static unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
* *
* 1) Put the instruction pointer into the IP buffer * 1) Put the instruction pointer into the IP buffer
* and the new code into the "code" buffer. * and the new code into the "code" buffer.
* 2) Set a flag that says we are modifying code * 2) Wait for any running NMIs to finish and set a flag that says
* 3) Wait for any running NMIs to finish. * we are modifying code, it is done in an atomic operation.
* 4) Write the code * 3) Write the code
* 5) clear the flag. * 4) clear the flag.
* 6) Wait for any running NMIs to finish. * 5) Wait for any running NMIs to finish.
* *
* If an NMI is executed, the first thing it does is to call * 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 * "ftrace_nmi_enter". This will check if the flag is set to write
...@@ -95,9 +95,9 @@ static unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr) ...@@ -95,9 +95,9 @@ static unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
* are the same as what exists. * are the same as what exists.
*/ */
#define MOD_CODE_WRITE_FLAG (1 << 31) /* set when NMI should do the write */
static atomic_t nmi_running = ATOMIC_INIT(0); static atomic_t nmi_running = ATOMIC_INIT(0);
static int mod_code_status; /* holds return value of text write */ static int mod_code_status; /* holds return value of text write */
static int mod_code_write; /* set when NMI should do the write */
static void *mod_code_ip; /* holds the IP to write to */ static void *mod_code_ip; /* holds the IP to write to */
static void *mod_code_newcode; /* holds the text to write to the IP */ static void *mod_code_newcode; /* holds the text to write to the IP */
...@@ -114,6 +114,20 @@ int ftrace_arch_read_dyn_info(char *buf, int size) ...@@ -114,6 +114,20 @@ int ftrace_arch_read_dyn_info(char *buf, int size)
return r; return r;
} }
static void clear_mod_flag(void)
{
int old = atomic_read(&nmi_running);
for (;;) {
int new = old & ~MOD_CODE_WRITE_FLAG;
if (old == new)
break;
old = atomic_cmpxchg(&nmi_running, old, new);
}
}
static void ftrace_mod_code(void) static void ftrace_mod_code(void)
{ {
/* /*
...@@ -127,27 +141,39 @@ static void ftrace_mod_code(void) ...@@ -127,27 +141,39 @@ static void ftrace_mod_code(void)
/* if we fail, then kill any new writers */ /* if we fail, then kill any new writers */
if (mod_code_status) if (mod_code_status)
mod_code_write = 0; clear_mod_flag();
} }
void ftrace_nmi_enter(void) void ftrace_nmi_enter(void)
{ {
atomic_inc(&nmi_running); if (atomic_inc_return(&nmi_running) & MOD_CODE_WRITE_FLAG) {
/* Must have nmi_running seen before reading write flag */ smp_rmb();
smp_mb();
if (mod_code_write) {
ftrace_mod_code(); ftrace_mod_code();
atomic_inc(&nmi_update_count); atomic_inc(&nmi_update_count);
} }
/* Must have previous changes seen before executions */
smp_mb();
} }
void ftrace_nmi_exit(void) void ftrace_nmi_exit(void)
{ {
/* Finish all executions before clearing nmi_running */ /* Finish all executions before clearing nmi_running */
smp_wmb(); smp_mb();
atomic_dec(&nmi_running); atomic_dec(&nmi_running);
} }
static void wait_for_nmi_and_set_mod_flag(void)
{
if (!atomic_cmpxchg(&nmi_running, 0, MOD_CODE_WRITE_FLAG))
return;
do {
cpu_relax();
} while (atomic_cmpxchg(&nmi_running, 0, MOD_CODE_WRITE_FLAG));
nmi_wait_count++;
}
static void wait_for_nmi(void) static void wait_for_nmi(void)
{ {
if (!atomic_read(&nmi_running)) if (!atomic_read(&nmi_running))
...@@ -167,14 +193,9 @@ do_ftrace_mod_code(unsigned long ip, void *new_code) ...@@ -167,14 +193,9 @@ do_ftrace_mod_code(unsigned long ip, void *new_code)
mod_code_newcode = new_code; mod_code_newcode = new_code;
/* The buffers need to be visible before we let NMIs write them */ /* 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(); smp_mb();
wait_for_nmi(); wait_for_nmi_and_set_mod_flag();
/* Make sure all running NMIs have finished before we write the code */ /* Make sure all running NMIs have finished before we write the code */
smp_mb(); smp_mb();
...@@ -182,13 +203,9 @@ do_ftrace_mod_code(unsigned long ip, void *new_code) ...@@ -182,13 +203,9 @@ do_ftrace_mod_code(unsigned long ip, void *new_code)
ftrace_mod_code(); ftrace_mod_code();
/* Make sure the write happens before clearing the bit */ /* Make sure the write happens before clearing the bit */
smp_wmb();
mod_code_write = 0;
/* make sure NMIs see the cleared bit */
smp_mb(); smp_mb();
clear_mod_flag();
wait_for_nmi(); wait_for_nmi();
return mod_code_status; return mod_code_status;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment