From c932c6b7c913a5661e04059045fa1eac762c82fa Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (Red Hat)" Date: Fri, 21 Feb 2014 10:43:12 -0500 Subject: ftrace/x86: Run a sync after fixup on failure If a failure occurs while enabling a trace, it bails out and will remove the tracepoints to be back to what the code originally was. But the fix up had some bugs in it. By injecting a failure in the code, the fix up ran to completion, but shortly afterward the system rebooted. There was two bugs here. The first was that there was no final sync run across the CPUs after the fix up was done, and before the ftrace int3 handler flag was reset. That means that other CPUs could still see the breakpoint and trigger on it long after the flag was cleared, and the int3 handler would think it was a spurious interrupt. Worse yet, the int3 handler could hit other breakpoints because the ftrace int3 handler flag would have prevented the int3 handler from going further. Here's a description of the issue: CPU0 CPU1 ---- ---- remove_breakpoint(); modifying_ftrace_code = 0; [still sees breakpoint] [sees modifying_ftrace_code as zero] [no breakpoint handler] [goto failed case] [trap exception - kernel breakpoint, no handler] BUG() The second bug was that the removal of the breakpoints required the "within()" logic updates instead of accessing the ip address directly. As the kernel text is mapped read-only when CONFIG_DEBUG_RODATA is set, and the removal of the breakpoint is a modification of the kernel text. The ftrace_write() includes the "within()" logic, where as, the probe_kernel_write() does not. This prevented the breakpoint from being removed at all. Link: http://lkml.kernel.org/r/1392650573-3390-1-git-send-email-pmladek@suse.cz Reported-by: Petr Mladek Tested-by: Petr Mladek Acked-by: H. Peter Anvin Signed-off-by: Steven Rostedt --- arch/x86/kernel/ftrace.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index e6253195a301..6b566c82d82c 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -455,7 +455,7 @@ static int remove_breakpoint(struct dyn_ftrace *rec) } update: - return probe_kernel_write((void *)ip, &nop[0], 1); + return ftrace_write(ip, nop, 1); } static int add_update_code(unsigned long ip, unsigned const char *new) @@ -634,6 +634,7 @@ void ftrace_replace_code(int enable) rec = ftrace_rec_iter_record(iter); remove_breakpoint(rec); } + run_sync(); } static int @@ -664,7 +665,7 @@ ftrace_modify_code(unsigned long ip, unsigned const char *old_code, return ret; fail_update: - probe_kernel_write((void *)ip, &old_code[0], 1); + ftrace_write(ip, old_code, 1); goto out; } -- cgit v1.2.3 From 12729f14d8357fb845d75155228b21e76360272d Mon Sep 17 00:00:00 2001 From: Petr Mladek Date: Mon, 24 Feb 2014 17:12:20 +0100 Subject: ftrace/x86: One more missing sync after fixup of function modification failure If a failure occurs while modifying ftrace function, it bails out and will remove the tracepoints to be back to what the code originally was. There is missing the final sync run across the CPUs after the fix up is done and before the ftrace int3 handler flag is reset. Here's the description of the problem: CPU0 CPU1 ---- ---- remove_breakpoint(); modifying_ftrace_code = 0; [still sees breakpoint] [sees modifying_ftrace_code as zero] [no breakpoint handler] [goto failed case] [trap exception - kernel breakpoint, no handler] BUG() Link: http://lkml.kernel.org/r/1393258342-29978-2-git-send-email-pmladek@suse.cz Fixes: 8a4d0a687a5 "ftrace: Use breakpoint method to update ftrace caller" Acked-by: Frederic Weisbecker Acked-by: H. Peter Anvin Signed-off-by: Petr Mladek Signed-off-by: Steven Rostedt --- arch/x86/kernel/ftrace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch/x86') diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 6b566c82d82c..69885e2f2095 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -660,8 +660,8 @@ ftrace_modify_code(unsigned long ip, unsigned const char *old_code, ret = -EPERM; goto out; } - run_sync(); out: + run_sync(); return ret; fail_update: -- cgit v1.2.3 From 92550405c493a3c2fa14bf37d1d60cd6c7d0f585 Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (Red Hat)" Date: Tue, 25 Feb 2014 21:33:59 -0500 Subject: ftrace/x86: Have ftrace_write() return -EPERM and clean up callers Having ftrace_write() return -EPERM on failure, as that's what the callers return, then we can clean up the code a bit. That is, instead of: if (ftrace_write(...)) return -EPERM; return 0; or if (ftrace_write(...)) { ret = -EPERM; goto_out; } We can instead have: return ftrace_write(...); or ret = ftrace_write(...); if (ret) goto out; Signed-off-by: Steven Rostedt --- arch/x86/kernel/ftrace.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 69885e2f2095..8cabf638cb63 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -308,7 +308,10 @@ static int ftrace_write(unsigned long ip, const char *val, int size) if (within(ip, (unsigned long)_text, (unsigned long)_etext)) ip = (unsigned long)__va(__pa_symbol(ip)); - return probe_kernel_write((void *)ip, val, size); + if (probe_kernel_write((void *)ip, val, size)) + return -EPERM; + + return 0; } static int add_break(unsigned long ip, const char *old) @@ -323,10 +326,7 @@ static int add_break(unsigned long ip, const char *old) if (memcmp(replaced, old, MCOUNT_INSN_SIZE) != 0) return -EINVAL; - if (ftrace_write(ip, &brk, 1)) - return -EPERM; - - return 0; + return ftrace_write(ip, &brk, 1); } static int add_brk_on_call(struct dyn_ftrace *rec, unsigned long addr) @@ -463,9 +463,7 @@ static int add_update_code(unsigned long ip, unsigned const char *new) /* skip breakpoint */ ip++; new++; - if (ftrace_write(ip, new, MCOUNT_INSN_SIZE - 1)) - return -EPERM; - return 0; + return ftrace_write(ip, new, MCOUNT_INSN_SIZE - 1); } static int add_update_call(struct dyn_ftrace *rec, unsigned long addr) @@ -520,10 +518,7 @@ static int finish_update_call(struct dyn_ftrace *rec, unsigned long addr) new = ftrace_call_replace(ip, addr); - if (ftrace_write(ip, new, 1)) - return -EPERM; - - return 0; + return ftrace_write(ip, new, 1); } static int finish_update_nop(struct dyn_ftrace *rec) @@ -533,9 +528,7 @@ static int finish_update_nop(struct dyn_ftrace *rec) new = ftrace_nop_replace(); - if (ftrace_write(ip, new, 1)) - return -EPERM; - return 0; + return ftrace_write(ip, new, 1); } static int finish_update(struct dyn_ftrace *rec, int enable) @@ -656,10 +649,6 @@ ftrace_modify_code(unsigned long ip, unsigned const char *old_code, run_sync(); ret = ftrace_write(ip, new_code, 1); - if (ret) { - ret = -EPERM; - goto out; - } out: run_sync(); return ret; -- cgit v1.2.3 From af64a7cb09db77344c596a0bf3d57d77257e8bf5 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Mon, 24 Feb 2014 19:59:58 +0100 Subject: ftrace: Pass retval through return in ftrace_dyn_arch_init() No architecture uses the "data" parameter in ftrace_dyn_arch_init() in any way, it just sets the value to 0. And this is used as a return value in the caller -- ftrace_init, which just checks the retval against zero. Note there is also "return 0" in every ftrace_dyn_arch_init. So it is enough to check the retval and remove all the indirect sets of data on all archs. Link: http://lkml.kernel.org/r/1393268401-24379-3-git-send-email-jslaby@suse.cz Cc: linux-arch@vger.kernel.org Signed-off-by: Jiri Slaby Signed-off-by: Steven Rostedt --- Documentation/trace/ftrace-design.txt | 3 --- arch/arm/kernel/ftrace.c | 2 -- arch/blackfin/kernel/ftrace.c | 3 --- arch/ia64/kernel/ftrace.c | 2 -- arch/metag/kernel/ftrace.c | 3 --- arch/microblaze/kernel/ftrace.c | 3 --- arch/mips/kernel/ftrace.c | 3 --- arch/powerpc/kernel/ftrace.c | 5 ----- arch/s390/kernel/ftrace.c | 1 - arch/sh/kernel/ftrace.c | 3 --- arch/sparc/kernel/ftrace.c | 4 ---- arch/tile/kernel/ftrace.c | 2 -- arch/x86/kernel/ftrace.c | 3 --- kernel/trace/ftrace.c | 6 ++---- 14 files changed, 2 insertions(+), 41 deletions(-) (limited to 'arch/x86') diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt index 79fcafc7fd64..117168884023 100644 --- a/Documentation/trace/ftrace-design.txt +++ b/Documentation/trace/ftrace-design.txt @@ -360,9 +360,6 @@ function below should be sufficient for most people: int __init ftrace_dyn_arch_init(void *data) { - /* return value is done indirectly via data */ - *(unsigned long *)data = 0; - return 0; } diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c index 34e56647dcee..5cd0d05edf35 100644 --- a/arch/arm/kernel/ftrace.c +++ b/arch/arm/kernel/ftrace.c @@ -158,8 +158,6 @@ int ftrace_make_nop(struct module *mod, int __init ftrace_dyn_arch_init(void *data) { - *(unsigned long *)data = 0; - return 0; } #endif /* CONFIG_DYNAMIC_FTRACE */ diff --git a/arch/blackfin/kernel/ftrace.c b/arch/blackfin/kernel/ftrace.c index 9277905b82cf..f74c5ae6a25b 100644 --- a/arch/blackfin/kernel/ftrace.c +++ b/arch/blackfin/kernel/ftrace.c @@ -67,9 +67,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func) int __init ftrace_dyn_arch_init(void *data) { - /* return value is done indirectly via data */ - *(unsigned long *)data = 0; - return 0; } diff --git a/arch/ia64/kernel/ftrace.c b/arch/ia64/kernel/ftrace.c index 7fc8c961b1f7..cfaa93a8bbdf 100644 --- a/arch/ia64/kernel/ftrace.c +++ b/arch/ia64/kernel/ftrace.c @@ -200,7 +200,5 @@ int ftrace_update_ftrace_func(ftrace_func_t func) /* run from kstop_machine */ int __init ftrace_dyn_arch_init(void *data) { - *(unsigned long *)data = 0; - return 0; } diff --git a/arch/metag/kernel/ftrace.c b/arch/metag/kernel/ftrace.c index a774f321643f..bf593932b353 100644 --- a/arch/metag/kernel/ftrace.c +++ b/arch/metag/kernel/ftrace.c @@ -119,8 +119,5 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) /* run from kstop_machine */ int __init ftrace_dyn_arch_init(void *data) { - /* The return code is returned via data */ - writel(0, data); - return 0; } diff --git a/arch/microblaze/kernel/ftrace.c b/arch/microblaze/kernel/ftrace.c index e8a5e9cf4ed1..ffa595c7fec2 100644 --- a/arch/microblaze/kernel/ftrace.c +++ b/arch/microblaze/kernel/ftrace.c @@ -173,9 +173,6 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) int __init ftrace_dyn_arch_init(void *data) { - /* The return code is retured via data */ - *(unsigned long *)data = 0; - return 0; } diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c index 185ba258361b..013016bec9e1 100644 --- a/arch/mips/kernel/ftrace.c +++ b/arch/mips/kernel/ftrace.c @@ -206,9 +206,6 @@ int __init ftrace_dyn_arch_init(void *data) /* Remove "b ftrace_stub" to ensure ftrace_caller() is executed */ ftrace_modify_code(MCOUNT_ADDR, INSN_NOP); - /* The return code is retured via data */ - *(unsigned long *)data = 0; - return 0; } #endif /* CONFIG_DYNAMIC_FTRACE */ diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c index 9b27b293a922..d059664cdf16 100644 --- a/arch/powerpc/kernel/ftrace.c +++ b/arch/powerpc/kernel/ftrace.c @@ -533,11 +533,6 @@ void arch_ftrace_update_code(int command) int __init ftrace_dyn_arch_init(void *data) { - /* caller expects data to be zero */ - unsigned long *p = data; - - *p = 0; - return 0; } #endif /* CONFIG_DYNAMIC_FTRACE */ diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c index 224db03e9518..77b2f3a1f50a 100644 --- a/arch/s390/kernel/ftrace.c +++ b/arch/s390/kernel/ftrace.c @@ -132,7 +132,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func) int __init ftrace_dyn_arch_init(void *data) { - *(unsigned long *) data = 0; return 0; } diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c index 30e13196d35b..493997541d2c 100644 --- a/arch/sh/kernel/ftrace.c +++ b/arch/sh/kernel/ftrace.c @@ -274,9 +274,6 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) int __init ftrace_dyn_arch_init(void *data) { - /* The return code is retured via data */ - __raw_writel(0, (unsigned long)data); - return 0; } #endif /* CONFIG_DYNAMIC_FTRACE */ diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c index 03ab022e51c5..ee813b82da49 100644 --- a/arch/sparc/kernel/ftrace.c +++ b/arch/sparc/kernel/ftrace.c @@ -84,10 +84,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func) int __init ftrace_dyn_arch_init(void *data) { - unsigned long *p = data; - - *p = 0; - return 0; } #endif diff --git a/arch/tile/kernel/ftrace.c b/arch/tile/kernel/ftrace.c index f1c452092eeb..34d9ea0bca9f 100644 --- a/arch/tile/kernel/ftrace.c +++ b/arch/tile/kernel/ftrace.c @@ -169,8 +169,6 @@ int ftrace_make_nop(struct module *mod, int __init ftrace_dyn_arch_init(void *data) { - *(unsigned long *)data = 0; - return 0; } #endif /* CONFIG_DYNAMIC_FTRACE */ diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 8cabf638cb63..bbe5a5b88aad 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -670,9 +670,6 @@ void arch_ftrace_update_code(int command) int __init ftrace_dyn_arch_init(void *data) { - /* The return code is retured via data */ - *(unsigned long *)data = 0; - return 0; } #endif diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 76b6ed29d856..083c6d5fce25 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -4379,11 +4379,9 @@ void __init ftrace_init(void) addr = (unsigned long)ftrace_stub; local_irq_save(flags); - ftrace_dyn_arch_init(&addr); + ret = ftrace_dyn_arch_init(&addr); local_irq_restore(flags); - - /* ftrace_dyn_arch_init places the return code in addr */ - if (addr) + if (ret) goto failed; count = __stop_mcount_loc - __start_mcount_loc; -- cgit v1.2.3 From 3a36cb11ca65cd6804972eaf1000378ba4384ea7 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Mon, 24 Feb 2014 19:59:59 +0100 Subject: ftrace: Do not pass data to ftrace_dyn_arch_init As the data parameter is not really used by any ftrace_dyn_arch_init, remove that from ftrace_dyn_arch_init. This also removes the addr local variable from ftrace_init which is now unused. Note the documentation was imprecise as it did not suggest to set (*data) to 0. Link: http://lkml.kernel.org/r/1393268401-24379-4-git-send-email-jslaby@suse.cz Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: linux-arch@vger.kernel.org Signed-off-by: Jiri Slaby Signed-off-by: Steven Rostedt --- Documentation/trace/ftrace-design.txt | 2 +- arch/arm/kernel/ftrace.c | 2 +- arch/blackfin/kernel/ftrace.c | 2 +- arch/ia64/kernel/ftrace.c | 2 +- arch/metag/kernel/ftrace.c | 2 +- arch/microblaze/kernel/ftrace.c | 2 +- arch/mips/kernel/ftrace.c | 2 +- arch/powerpc/kernel/ftrace.c | 2 +- arch/s390/kernel/ftrace.c | 2 +- arch/sh/kernel/ftrace.c | 2 +- arch/sparc/kernel/ftrace.c | 2 +- arch/tile/kernel/ftrace.c | 2 +- arch/x86/kernel/ftrace.c | 2 +- include/linux/ftrace.h | 2 +- kernel/trace/ftrace.c | 7 ++----- 15 files changed, 16 insertions(+), 19 deletions(-) (limited to 'arch/x86') diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt index 117168884023..3f669b9e8852 100644 --- a/Documentation/trace/ftrace-design.txt +++ b/Documentation/trace/ftrace-design.txt @@ -358,7 +358,7 @@ Every arch has an init callback function. If you need to do something early on to initialize some state, this is the time to do that. Otherwise, this simple function below should be sufficient for most people: -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c index 5cd0d05edf35..c108ddcb9ba4 100644 --- a/arch/arm/kernel/ftrace.c +++ b/arch/arm/kernel/ftrace.c @@ -156,7 +156,7 @@ int ftrace_make_nop(struct module *mod, return ret; } -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/blackfin/kernel/ftrace.c b/arch/blackfin/kernel/ftrace.c index f74c5ae6a25b..095de0fa044d 100644 --- a/arch/blackfin/kernel/ftrace.c +++ b/arch/blackfin/kernel/ftrace.c @@ -65,7 +65,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func) return ftrace_modify_code(ip, call, sizeof(call)); } -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/ia64/kernel/ftrace.c b/arch/ia64/kernel/ftrace.c index cfaa93a8bbdf..3b0c2aa07857 100644 --- a/arch/ia64/kernel/ftrace.c +++ b/arch/ia64/kernel/ftrace.c @@ -198,7 +198,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func) } /* run from kstop_machine */ -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/metag/kernel/ftrace.c b/arch/metag/kernel/ftrace.c index bf593932b353..ed1d685157c2 100644 --- a/arch/metag/kernel/ftrace.c +++ b/arch/metag/kernel/ftrace.c @@ -117,7 +117,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) } /* run from kstop_machine */ -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/microblaze/kernel/ftrace.c b/arch/microblaze/kernel/ftrace.c index ffa595c7fec2..bbcd2533766c 100644 --- a/arch/microblaze/kernel/ftrace.c +++ b/arch/microblaze/kernel/ftrace.c @@ -171,7 +171,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) return ret; } -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c index 013016bec9e1..1ba7afe6ab74 100644 --- a/arch/mips/kernel/ftrace.c +++ b/arch/mips/kernel/ftrace.c @@ -198,7 +198,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func) return ftrace_modify_code(FTRACE_CALL_IP, new); } -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { /* Encode the instructions when booting */ ftrace_dyn_arch_init_insns(); diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c index d059664cdf16..71ce4cbb7e9f 100644 --- a/arch/powerpc/kernel/ftrace.c +++ b/arch/powerpc/kernel/ftrace.c @@ -531,7 +531,7 @@ void arch_ftrace_update_code(int command) ftrace_disable_ftrace_graph_caller(); } -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c index 77b2f3a1f50a..54d6493c4a56 100644 --- a/arch/s390/kernel/ftrace.c +++ b/arch/s390/kernel/ftrace.c @@ -130,7 +130,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func) return 0; } -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c index 493997541d2c..3c74f53db6db 100644 --- a/arch/sh/kernel/ftrace.c +++ b/arch/sh/kernel/ftrace.c @@ -272,7 +272,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) return ftrace_modify_code(rec->ip, old, new); } -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c index ee813b82da49..0a2d2ddff543 100644 --- a/arch/sparc/kernel/ftrace.c +++ b/arch/sparc/kernel/ftrace.c @@ -82,7 +82,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func) return ftrace_modify_code(ip, old, new); } -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/tile/kernel/ftrace.c b/arch/tile/kernel/ftrace.c index 34d9ea0bca9f..8d52d83cc516 100644 --- a/arch/tile/kernel/ftrace.c +++ b/arch/tile/kernel/ftrace.c @@ -167,7 +167,7 @@ int ftrace_make_nop(struct module *mod, return ret; } -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index bbe5a5b88aad..4b66adf17369 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -668,7 +668,7 @@ void arch_ftrace_update_code(int command) atomic_dec(&modifying_ftrace_code); } -int __init ftrace_dyn_arch_init(void *data) +int __init ftrace_dyn_arch_init(void) { return 0; } diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index e6141be2fad5..1bbb2cd631de 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -423,7 +423,7 @@ ftrace_set_early_filter(struct ftrace_ops *ops, char *buf, int enable); /* defined in arch */ extern int ftrace_ip_converted(unsigned long ip); -extern int ftrace_dyn_arch_init(void *data); +extern int ftrace_dyn_arch_init(void); extern void ftrace_replace_code(int enable); extern int ftrace_update_ftrace_func(ftrace_func_t func); extern void ftrace_caller(void); diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 083c6d5fce25..5bd70e8b09b0 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -4372,14 +4372,11 @@ void __init ftrace_init(void) { extern unsigned long __start_mcount_loc[]; extern unsigned long __stop_mcount_loc[]; - unsigned long count, addr, flags; + unsigned long count, flags; int ret; - /* Keep the ftrace pointer to the stub */ - addr = (unsigned long)ftrace_stub; - local_irq_save(flags); - ret = ftrace_dyn_arch_init(&addr); + ret = ftrace_dyn_arch_init(); local_irq_restore(flags); if (ret) goto failed; -- cgit v1.2.3 From 7f11f5ecf4ae09815dc2de267c5e04d1de01d862 Mon Sep 17 00:00:00 2001 From: Petr Mladek Date: Mon, 24 Feb 2014 17:12:22 +0100 Subject: ftrace/x86: BUG when ftrace recovery fails Ftrace modifies function calls using Int3 breakpoints on x86. The breakpoints are handled only when the patching is in progress. If something goes wrong, there is a recovery code that removes the breakpoints. If this fails, the system might get silently rebooted when a remaining break is not handled or an invalid instruction is proceed. We should BUG() when the breakpoint could not be removed. Otherwise, the system silently crashes when the function finishes the Int3 handler is disabled. Note that we need to modify remove_breakpoint() to return non-zero value only when there is an error. The return value was ignored before, so it does not cause any troubles. Link: http://lkml.kernel.org/r/1393258342-29978-4-git-send-email-pmladek@suse.cz Signed-off-by: Petr Mladek Signed-off-by: Steven Rostedt --- arch/x86/kernel/ftrace.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 4b66adf17369..52819e816f87 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -425,7 +425,7 @@ static int remove_breakpoint(struct dyn_ftrace *rec) /* If this does not have a breakpoint, we are done */ if (ins[0] != brk) - return -1; + return 0; nop = ftrace_nop_replace(); @@ -625,7 +625,12 @@ void ftrace_replace_code(int enable) printk(KERN_WARNING "Failed on %s (%d):\n", report, count); for_ftrace_rec_iter(iter) { rec = ftrace_rec_iter_record(iter); - remove_breakpoint(rec); + /* + * Breakpoints are handled only when this function is in + * progress. The system could not work with them. + */ + if (remove_breakpoint(rec)) + BUG(); } run_sync(); } @@ -649,12 +654,19 @@ ftrace_modify_code(unsigned long ip, unsigned const char *old_code, run_sync(); ret = ftrace_write(ip, new_code, 1); + /* + * The breakpoint is handled only when this function is in progress. + * The system could not work if we could not remove it. + */ + BUG_ON(ret); out: run_sync(); return ret; fail_update: - ftrace_write(ip, old_code, 1); + /* Also here the system could not work with the breakpoint */ + if (ftrace_write(ip, old_code, 1)) + BUG(); goto out; } -- cgit v1.2.3