The commit fd4363fff3d9 (x86: Introduce int3 (breakpoint)-based instruction
patching) uses the same technique that has been used in ftrace since 08d636b
("ftrace/x86: Have arch x86_64 use breakpoints instead of stop machine")
It would be great to use the new generic implementation and clean up the x86
ftrace code a bit.
Let's start with ftrace_modify_code. It does basically the same as text_poke_bp.
In addition, it checks whether the replaced code is the expected one to avoid
problems with modules. The checking code can be split from
ftrace_modify_code_direct.
Note that we do not longer need to set modifying_ftrace_code in
ftrace_update_ftrace_func. The int3 interupt will be handled by
poke_int3_handler.
Signed-off-by: Petr Mladek <pmla...@suse.cz>
---
arch/x86/kernel/ftrace.c | 105 ++++++++++++++++++++---------------------------
1 file changed, 45 insertions(+), 60 deletions(-)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 42a392a..f96dbcc 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -52,6 +52,33 @@ union ftrace_code_union {
} __attribute__((packed));
};
+/*
+ * Note: Due to modules and __init, code can
+ * disappear and change, we need to protect against faulting
+ * as well as code changing. We do this by using the
+ * probe_kernel_* functions.
+ */
+static int
+ftrace_check_code(unsigned long ip, unsigned const char *expected)
+{
+ unsigned char actual[MCOUNT_INSN_SIZE];
+
+ if (probe_kernel_read(actual, (void *)ip, MCOUNT_INSN_SIZE))
+ return -EFAULT;
+
+ if (memcmp(actual, expected, MCOUNT_INSN_SIZE) != 0) {
+ WARN_ONCE(1,
+ "ftrace check failed: %x %x%x%x%x vs. %x %x%x%x%x\n",
+ actual[0],
+ actual[1], actual[2], actual[3], actual[4],
+ expected[0],
+ expected[1], expected[2], expected[3], expected[4]);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int ftrace_calc_offset(long ip, long addr)
{
return (int)(addr - ip);
@@ -103,28 +130,12 @@ static int
ftrace_modify_code_direct(unsigned long ip, unsigned const char *old_code,
unsigned const char *new_code)
{
- unsigned char replaced[MCOUNT_INSN_SIZE];
-
- /*
- * Note: Due to modules and __init, code can
- * disappear and change, we need to protect against faulting
- * as well as code changing. We do this by using the
- * probe_kernel_* functions.
- *
- * No real locking needed, this code is run through
- * kstop_machine, or before SMP starts.
- */
-
- /* read the text we want to modify */
- if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE))
- return -EFAULT;
+ int ret;
- /* Make sure it is what we expect it to be */
- if (memcmp(replaced, old_code, MCOUNT_INSN_SIZE) != 0)
- return -EINVAL;
+ ret = ftrace_check_code(ip, old_code);
/* replace the text with the new text */
- if (do_ftrace_mod_code(ip, new_code))
+ if (!ret && do_ftrace_mod_code(ip, new_code))
return -EPERM;
sync_core();
@@ -132,6 +143,21 @@ ftrace_modify_code_direct(unsigned long ip, unsigned const
char *old_code,
return 0;
}
+static int
+ftrace_modify_code(unsigned long ip, unsigned const char *old_code,
+ unsigned const char *new_code)
+{
+ int ret;
+
+ ret = ftrace_check_code(ip, old_code);
+
+ /* replace the text with the new text */
+ if (!ret)
+ text_poke_bp((void **)&ip, new_code, MCOUNT_INSN_SIZE, 1, NULL);
+
+ return ret;
+}
+
int ftrace_make_nop(struct module *mod,
struct dyn_ftrace *rec, unsigned long addr)
{
@@ -202,10 +228,6 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long
addr)
*/
atomic_t modifying_ftrace_code __read_mostly;
-static int
-ftrace_modify_code(unsigned long ip, unsigned const char *old_code,
- unsigned const char *new_code);
-
/*
* Should never be called:
* As it is only called by __ftrace_replace_code() which is called by
@@ -230,9 +252,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
memcpy(old, &ftrace_call, MCOUNT_INSN_SIZE);
new = ftrace_call_replace(ip, (unsigned long)func);
- /* See comment above by declaration of modifying_ftrace_code */
- atomic_inc(&modifying_ftrace_code);
-
ret = ftrace_modify_code(ip, old, new);
/* Also update the regs callback function */
@@ -243,8 +262,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
ret = ftrace_modify_code(ip, old, new);
}
- atomic_dec(&modifying_ftrace_code);
-
return ret;
}
@@ -609,38 +626,6 @@ void ftrace_replace_code(int enable)
}
}
-static int
-ftrace_modify_code(unsigned long ip, unsigned const char *old_code,
- unsigned const char *new_code)
-{
- int ret;
-
- ret = add_break(ip, old_code);
- if (ret)
- goto out;
-
- run_sync();
-
- ret = add_update_code(ip, new_code);
- if (ret)
- goto fail_update;
-
- run_sync();
-
- ret = ftrace_write(ip, new_code, 1);
- if (ret) {
- ret = -EPERM;
- goto out;
- }
- run_sync();
- out:
- return ret;
-
- fail_update:
- probe_kernel_write((void *)ip, &old_code[0], 1);
- goto out;
-}
-
void arch_ftrace_update_code(int command)
{
/* See comment above by declaration of modifying_ftrace_code */
--
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/
