Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755389Ab3JRO2Y (ORCPT ); Fri, 18 Oct 2013 10:28:24 -0400 Received: from cantor2.suse.de ([195.135.220.15]:52690 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754864Ab3JRO1b (ORCPT ); Fri, 18 Oct 2013 10:27:31 -0400 From: Petr Mladek To: Steven Rostedt , Frederic Weisbecker , Masami Hiramatsu , Jiri Kosina Cc: linux-kernel@vger.kernel.org, x86@kernel.org, Petr Mladek Subject: [PATCH 4/6] x86: modify ftrace function using the new int3-based framework Date: Fri, 18 Oct 2013 16:27:23 +0200 Message-Id: <1382106445-31468-5-git-send-email-pmladek@suse.cz> X-Mailer: git-send-email 1.8.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5311 Lines: 190 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 --- 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/