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 <pmla...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
