lkml.org 
[lkml]   [2014]   [Feb]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 1/4] x86: Clean up remove_breakpoint() in ftrace code
Date
The ftrace recovery code does not work on x86_64. I tried to
debug it and got a bit confused by remove_breakpoint(). One problem
was reusing the "nop" pointer for different types of code. Also
the combination of two level if-condition and goto was a bit
tricky.

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
arch/x86/kernel/ftrace.c | 58 ++++++++++++++++++++++--------------------------
1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index e6253195a301..e7f3f3f565de 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -405,17 +405,16 @@ static int add_breakpoints(struct dyn_ftrace *rec, int enable)

/*
* On error, we need to remove breakpoints. This needs to
- * be done caefully. If the address does not currently have a
+ * be done carefully. If the address does not currently have a
* breakpoint, we know we are done. Otherwise, we look at the
- * remaining 4 bytes of the instruction. If it matches a nop
- * we replace the breakpoint with the nop. Otherwise we replace
- * it with the call instruction.
+ * remaining 4 bytes of the instruction. It has to be a valid
+ * code. If not, don't touch the breakpoint, we would just
+ * create a disaster.
*/
static int remove_breakpoint(struct dyn_ftrace *rec)
{
unsigned char ins[MCOUNT_INSN_SIZE];
- unsigned char brk = BREAKPOINT_INSTRUCTION;
- const unsigned char *nop;
+ const unsigned char *valid_ins;
unsigned long ftrace_addr;
unsigned long ip = rec->ip;

@@ -424,38 +423,33 @@ static int remove_breakpoint(struct dyn_ftrace *rec)
return -EFAULT;

/* If this does not have a breakpoint, we are done */
- if (ins[0] != brk)
+ if (ins[0] != BREAKPOINT_INSTRUCTION)
return -1;

- nop = ftrace_nop_replace();
+ /* Check if it is nop instruction */
+ valid_ins = ftrace_nop_replace();
+ if (memcmp(&ins[1], &valid_ins[1], MCOUNT_INSN_SIZE - 1) == 0)
+ goto update;

- /*
- * If the last 4 bytes of the instruction do not match
- * a nop, then we assume that this is a call to ftrace_addr.
- */
- if (memcmp(&ins[1], &nop[1], MCOUNT_INSN_SIZE - 1) != 0) {
- /*
- * For extra paranoidism, we check if the breakpoint is on
- * a call that would actually jump to the ftrace_addr.
- * If not, don't touch the breakpoint, we make just create
- * a disaster.
- */
- ftrace_addr = get_ftrace_addr(rec);
- nop = ftrace_call_replace(ip, ftrace_addr);
-
- if (memcmp(&ins[1], &nop[1], MCOUNT_INSN_SIZE - 1) == 0)
- goto update;
-
- /* Check both ftrace_addr and ftrace_old_addr */
- ftrace_addr = get_ftrace_old_addr(rec);
- nop = ftrace_call_replace(ip, ftrace_addr);

- if (memcmp(&ins[1], &nop[1], MCOUNT_INSN_SIZE - 1) != 0)
- return -EINVAL;
- }
+ /* Or it might be ftrace call instruction */
+ ftrace_addr = get_ftrace_addr(rec);
+ valid_ins = ftrace_call_replace(ip, ftrace_addr);
+ if (memcmp(&ins[1], &valid_ins[1], MCOUNT_INSN_SIZE - 1) == 0)
+ goto update;
+
+ /* Last chance, it might be the old ftrace call instruction */
+ ftrace_addr = get_ftrace_old_addr(rec);
+ valid_ins = ftrace_call_replace(ip, ftrace_addr);
+ if (memcmp(&ins[1], &valid_ins[1], MCOUNT_INSN_SIZE - 1) == 0)
+ goto update;
+
+ /* Hmm, it is an unknown code. Rather bail out. */
+ return -EINVAL;

update:
- return probe_kernel_write((void *)ip, &nop[0], 1);
+ /* Put back the first byte */
+ return probe_kernel_write((void *)ip, valid_ins, 1);
}

static int add_update_code(unsigned long ip, unsigned const char *new)
--
1.8.4


\
 
 \ /
  Last update: 2014-02-17 16:41    [W:0.060 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site