* FAILED: patch "[PATCH] ftrace: Fix removing of second function probe" failed to apply to 4.11-stable tree
@ 2017-05-22 16:55 gregkh
2017-05-23 22:32 ` Steven Rostedt
0 siblings, 1 reply; 2+ messages in thread
From: gregkh @ 2017-05-22 16:55 UTC (permalink / raw)
To: rostedt; +Cc: stable
The patch below does not apply to the 4.11-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From acceb72e90624ec6c3f2fc62e72dab892cd075da Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Date: Fri, 14 Apr 2017 17:45:45 -0400
Subject: [PATCH] ftrace: Fix removing of second function probe
When two function probes are added to set_ftrace_filter, and then one of
them is removed, the update to the function locations is not performed, and
the record keeping of the function states are corrupted, and causes an
ftrace_bug() to occur.
This is easily reproducable by adding two probes, removing one, and then
adding it back again.
# cd /sys/kernel/debug/tracing
# echo schedule:traceoff > set_ftrace_filter
# echo do_IRQ:traceoff > set_ftrace_filter
# echo \!do_IRQ:traceoff > /debug/tracing/set_ftrace_filter
# echo do_IRQ:traceoff > set_ftrace_filter
Causes:
------------[ cut here ]------------
WARNING: CPU: 2 PID: 1098 at kernel/trace/ftrace.c:2369 ftrace_get_addr_curr+0x143/0x220
Modules linked in: [...]
CPU: 2 PID: 1098 Comm: bash Not tainted 4.10.0-test+ #405
Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v02.05 05/07/2012
Call Trace:
dump_stack+0x68/0x9f
__warn+0x111/0x130
? trace_irq_work_interrupt+0xa0/0xa0
warn_slowpath_null+0x1d/0x20
ftrace_get_addr_curr+0x143/0x220
? __fentry__+0x10/0x10
ftrace_replace_code+0xe3/0x4f0
? ftrace_int3_handler+0x90/0x90
? printk+0x99/0xb5
? 0xffffffff81000000
ftrace_modify_all_code+0x97/0x110
arch_ftrace_update_code+0x10/0x20
ftrace_run_update_code+0x1c/0x60
ftrace_run_modify_code.isra.48.constprop.62+0x8e/0xd0
register_ftrace_function_probe+0x4b6/0x590
? ftrace_startup+0x310/0x310
? debug_lockdep_rcu_enabled.part.4+0x1a/0x30
? update_stack_state+0x88/0x110
? ftrace_regex_write.isra.43.part.44+0x1d3/0x320
? preempt_count_sub+0x18/0xd0
? mutex_lock_nested+0x104/0x800
? ftrace_regex_write.isra.43.part.44+0x1d3/0x320
? __unwind_start+0x1c0/0x1c0
? _mutex_lock_nest_lock+0x800/0x800
ftrace_trace_probe_callback.isra.3+0xc0/0x130
? func_set_flag+0xe0/0xe0
? __lock_acquire+0x642/0x1790
? __might_fault+0x1e/0x20
? trace_get_user+0x398/0x470
? strcmp+0x35/0x60
ftrace_trace_onoff_callback+0x48/0x70
ftrace_regex_write.isra.43.part.44+0x251/0x320
? match_records+0x420/0x420
ftrace_filter_write+0x2b/0x30
__vfs_write+0xd7/0x330
? do_loop_readv_writev+0x120/0x120
? locks_remove_posix+0x90/0x2f0
? do_lock_file_wait+0x160/0x160
? __lock_is_held+0x93/0x100
? rcu_read_lock_sched_held+0x5c/0xb0
? preempt_count_sub+0x18/0xd0
? __sb_start_write+0x10a/0x230
? vfs_write+0x222/0x240
vfs_write+0xef/0x240
SyS_write+0xab/0x130
? SyS_read+0x130/0x130
? trace_hardirqs_on_caller+0x182/0x280
? trace_hardirqs_on_thunk+0x1a/0x1c
entry_SYSCALL_64_fastpath+0x18/0xad
RIP: 0033:0x7fe61c157c30
RSP: 002b:00007ffe87890258 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: ffffffff8114a410 RCX: 00007fe61c157c30
RDX: 0000000000000010 RSI: 000055814798f5e0 RDI: 0000000000000001
RBP: ffff8800c9027f98 R08: 00007fe61c422740 R09: 00007fe61ca53700
R10: 0000000000000073 R11: 0000000000000246 R12: 0000558147a36400
R13: 00007ffe8788f160 R14: 0000000000000024 R15: 00007ffe8788f15c
? trace_hardirqs_off_caller+0xc0/0x110
---[ end trace 99fa09b3d9869c2c ]---
Bad trampoline accounting at: ffffffff81cc3b00 (do_IRQ+0x0/0x150)
Cc: stable@vger.kernel.org
Fixes: 59df055f1991 ("ftrace: trace different functions with a different tracer")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 34f63e78d661..4b6459a57fbc 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3780,23 +3780,24 @@ static void __enable_ftrace_function_probe(struct ftrace_ops_hash *old_hash)
ftrace_probe_registered = 1;
}
-static void __disable_ftrace_function_probe(void)
+static bool __disable_ftrace_function_probe(void)
{
int i;
if (!ftrace_probe_registered)
- return;
+ return false;
for (i = 0; i < FTRACE_FUNC_HASHSIZE; i++) {
struct hlist_head *hhd = &ftrace_func_hash[i];
if (hhd->first)
- return;
+ return false;
}
/* no more funcs left */
ftrace_shutdown(&trace_probe_ops, 0);
ftrace_probe_registered = 0;
+ return true;
}
@@ -3926,6 +3927,7 @@ static void
__unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
void *data, int flags)
{
+ struct ftrace_ops_hash old_hash_ops;
struct ftrace_func_entry *rec_entry;
struct ftrace_func_probe *entry;
struct ftrace_func_probe *p;
@@ -3937,6 +3939,7 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
struct hlist_node *tmp;
char str[KSYM_SYMBOL_LEN];
int i, ret;
+ bool disabled;
if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
func_g.search = NULL;
@@ -3955,6 +3958,10 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
mutex_lock(&trace_probe_ops.func_hash->regex_lock);
+ old_hash_ops.filter_hash = old_hash;
+ /* Probes only have filters */
+ old_hash_ops.notrace_hash = NULL;
+
hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, *orig_hash);
if (!hash)
/* Hmm, should report this somehow */
@@ -3992,12 +3999,17 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
}
}
mutex_lock(&ftrace_lock);
- __disable_ftrace_function_probe();
+ disabled = __disable_ftrace_function_probe();
/*
* Remove after the disable is called. Otherwise, if the last
* probe is removed, a null hash means *all enabled*.
*/
ret = ftrace_hash_move(&trace_probe_ops, 1, orig_hash, hash);
+
+ /* still need to update the function call sites */
+ if (ftrace_enabled && !disabled)
+ ftrace_run_modify_code(&trace_probe_ops, FTRACE_UPDATE_CALLS,
+ &old_hash_ops);
synchronize_sched();
if (!ret)
free_ftrace_hash_rcu(old_hash);
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: FAILED: patch "[PATCH] ftrace: Fix removing of second function probe" failed to apply to 4.11-stable tree
2017-05-22 16:55 FAILED: patch "[PATCH] ftrace: Fix removing of second function probe" failed to apply to 4.11-stable tree gregkh
@ 2017-05-23 22:32 ` Steven Rostedt
0 siblings, 0 replies; 2+ messages in thread
From: Steven Rostedt @ 2017-05-23 22:32 UTC (permalink / raw)
To: gregkh; +Cc: stable
On Mon, 22 May 2017 18:55:33 +0200
<gregkh@linuxfoundation.org> wrote:
> The patch below does not apply to the 4.11-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git
> commit id to <stable@vger.kernel.org>.
>
Hi Greg,
This fails because it was already applied to 4.11 with a stable tag.
But I had to cherry pick it into my development tree for 4.12 because
new code depended on it as well as code I already pushed to linux-next
and did not want to rebase. See this thread here:
http://lkml.kernel.org/r/20170414180547.182f859c@gandalf.local.home
Sorry for the confusion, maybe I should have removed the stable tag
from the cherry picked commit.
-- Steve
> thanks,
>
> greg k-h
>
> ------------------ original commit in Linus's tree ------------------
>
> >From acceb72e90624ec6c3f2fc62e72dab892cd075da Mon Sep 17 00:00:00
> >2001
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> Date: Fri, 14 Apr 2017 17:45:45 -0400
> Subject: [PATCH] ftrace: Fix removing of second function probe
>
> When two function probes are added to set_ftrace_filter, and then one
> of them is removed, the update to the function locations is not
> performed, and the record keeping of the function states are
> corrupted, and causes an ftrace_bug() to occur.
>
> This is easily reproducable by adding two probes, removing one, and
> then adding it back again.
>
> # cd /sys/kernel/debug/tracing
> # echo schedule:traceoff > set_ftrace_filter
> # echo do_IRQ:traceoff > set_ftrace_filter
> # echo \!do_IRQ:traceoff > /debug/tracing/set_ftrace_filter
> # echo do_IRQ:traceoff > set_ftrace_filter
>
> Causes:
> ------------[ cut here ]------------
> WARNING: CPU: 2 PID: 1098 at kernel/trace/ftrace.c:2369
> ftrace_get_addr_curr+0x143/0x220 Modules linked in: [...]
> CPU: 2 PID: 1098 Comm: bash Not tainted 4.10.0-test+ #405
> Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01
> v02.05 05/07/2012 Call Trace:
> dump_stack+0x68/0x9f
> __warn+0x111/0x130
> ? trace_irq_work_interrupt+0xa0/0xa0
> warn_slowpath_null+0x1d/0x20
> ftrace_get_addr_curr+0x143/0x220
> ? __fentry__+0x10/0x10
> ftrace_replace_code+0xe3/0x4f0
> ? ftrace_int3_handler+0x90/0x90
> ? printk+0x99/0xb5
> ? 0xffffffff81000000
> ftrace_modify_all_code+0x97/0x110
> arch_ftrace_update_code+0x10/0x20
> ftrace_run_update_code+0x1c/0x60
> ftrace_run_modify_code.isra.48.constprop.62+0x8e/0xd0
> register_ftrace_function_probe+0x4b6/0x590
> ? ftrace_startup+0x310/0x310
> ? debug_lockdep_rcu_enabled.part.4+0x1a/0x30
> ? update_stack_state+0x88/0x110
> ? ftrace_regex_write.isra.43.part.44+0x1d3/0x320
> ? preempt_count_sub+0x18/0xd0
> ? mutex_lock_nested+0x104/0x800
> ? ftrace_regex_write.isra.43.part.44+0x1d3/0x320
> ? __unwind_start+0x1c0/0x1c0
> ? _mutex_lock_nest_lock+0x800/0x800
> ftrace_trace_probe_callback.isra.3+0xc0/0x130
> ? func_set_flag+0xe0/0xe0
> ? __lock_acquire+0x642/0x1790
> ? __might_fault+0x1e/0x20
> ? trace_get_user+0x398/0x470
> ? strcmp+0x35/0x60
> ftrace_trace_onoff_callback+0x48/0x70
> ftrace_regex_write.isra.43.part.44+0x251/0x320
> ? match_records+0x420/0x420
> ftrace_filter_write+0x2b/0x30
> __vfs_write+0xd7/0x330
> ? do_loop_readv_writev+0x120/0x120
> ? locks_remove_posix+0x90/0x2f0
> ? do_lock_file_wait+0x160/0x160
> ? __lock_is_held+0x93/0x100
> ? rcu_read_lock_sched_held+0x5c/0xb0
> ? preempt_count_sub+0x18/0xd0
> ? __sb_start_write+0x10a/0x230
> ? vfs_write+0x222/0x240
> vfs_write+0xef/0x240
> SyS_write+0xab/0x130
> ? SyS_read+0x130/0x130
> ? trace_hardirqs_on_caller+0x182/0x280
> ? trace_hardirqs_on_thunk+0x1a/0x1c
> entry_SYSCALL_64_fastpath+0x18/0xad
> RIP: 0033:0x7fe61c157c30
> RSP: 002b:00007ffe87890258 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000001 RAX: ffffffffffffffda RBX: ffffffff8114a410 RCX:
> 00007fe61c157c30 RDX: 0000000000000010 RSI: 000055814798f5e0 RDI:
> 0000000000000001 RBP: ffff8800c9027f98 R08: 00007fe61c422740 R09:
> 00007fe61ca53700 R10: 0000000000000073 R11: 0000000000000246 R12:
> 0000558147a36400 R13: 00007ffe8788f160 R14: 0000000000000024 R15:
> 00007ffe8788f15c ? trace_hardirqs_off_caller+0xc0/0x110
> ---[ end trace 99fa09b3d9869c2c ]---
> Bad trampoline accounting at: ffffffff81cc3b00 (do_IRQ+0x0/0x150)
>
> Cc: stable@vger.kernel.org
> Fixes: 59df055f1991 ("ftrace: trace different functions with a
> different tracer") Signed-off-by: Steven Rostedt (VMware)
> <rostedt@goodmis.org>
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 34f63e78d661..4b6459a57fbc 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -3780,23 +3780,24 @@ static void
> __enable_ftrace_function_probe(struct ftrace_ops_hash *old_hash)
> ftrace_probe_registered = 1; }
>
> -static void __disable_ftrace_function_probe(void)
> +static bool __disable_ftrace_function_probe(void)
> {
> int i;
>
> if (!ftrace_probe_registered)
> - return;
> + return false;
>
> for (i = 0; i < FTRACE_FUNC_HASHSIZE; i++) {
> struct hlist_head *hhd = &ftrace_func_hash[i];
> if (hhd->first)
> - return;
> + return false;
> }
>
> /* no more funcs left */
> ftrace_shutdown(&trace_probe_ops, 0);
>
> ftrace_probe_registered = 0;
> + return true;
> }
>
>
> @@ -3926,6 +3927,7 @@ static void
> __unregister_ftrace_function_probe(char *glob, struct
> ftrace_probe_ops *ops, void *data, int flags)
> {
> + struct ftrace_ops_hash old_hash_ops;
> struct ftrace_func_entry *rec_entry;
> struct ftrace_func_probe *entry;
> struct ftrace_func_probe *p;
> @@ -3937,6 +3939,7 @@ __unregister_ftrace_function_probe(char *glob,
> struct ftrace_probe_ops *ops, struct hlist_node *tmp;
> char str[KSYM_SYMBOL_LEN];
> int i, ret;
> + bool disabled;
>
> if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
> func_g.search = NULL;
> @@ -3955,6 +3958,10 @@ __unregister_ftrace_function_probe(char *glob,
> struct ftrace_probe_ops *ops,
> mutex_lock(&trace_probe_ops.func_hash->regex_lock);
>
> + old_hash_ops.filter_hash = old_hash;
> + /* Probes only have filters */
> + old_hash_ops.notrace_hash = NULL;
> +
> hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS,
> *orig_hash); if (!hash)
> /* Hmm, should report this somehow */
> @@ -3992,12 +3999,17 @@ __unregister_ftrace_function_probe(char
> *glob, struct ftrace_probe_ops *ops, }
> }
> mutex_lock(&ftrace_lock);
> - __disable_ftrace_function_probe();
> + disabled = __disable_ftrace_function_probe();
> /*
> * Remove after the disable is called. Otherwise, if the last
> * probe is removed, a null hash means *all enabled*.
> */
> ret = ftrace_hash_move(&trace_probe_ops, 1, orig_hash, hash);
> +
> + /* still need to update the function call sites */
> + if (ftrace_enabled && !disabled)
> + ftrace_run_modify_code(&trace_probe_ops,
> FTRACE_UPDATE_CALLS,
> + &old_hash_ops);
> synchronize_sched();
> if (!ret)
> free_ftrace_hash_rcu(old_hash);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-05-23 22:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-22 16:55 FAILED: patch "[PATCH] ftrace: Fix removing of second function probe" failed to apply to 4.11-stable tree gregkh
2017-05-23 22:32 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).