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)
Showing the changes to kernel (1 of 1 file). Show all
Unified · Split
| @@ -3755,23 +3755,24 @@ static void __enable_ftrace_function_probe(struct ftrace_ops_hash *old_hash) · 3754 unmodified lines | |||
| 3755 | ftrace_probe_registered = 1; | 3755 | ftrace_probe_registered = 1; |
| 3756 | } | 3756 | } |
| 3757 | 3757 | ||
| 3758 | static void __disable_ftrace_function_probe(void) | 3758 | static bool __disable_ftrace_function_probe(void) |
| 3759 | { | 3759 | { |
| 3760 | int i; | 3760 | int i; |
| 3761 | 3761 | ||
| 3762 | if (!ftrace_probe_registered) | 3762 | if (!ftrace_probe_registered) |
| 3763 | return; | 3763 | return false; |
| 3764 | 3764 | ||
| 3765 | for (i = 0; i < FTRACE_FUNC_HASHSIZE; i++) { | 3765 | for (i = 0; i < FTRACE_FUNC_HASHSIZE; i++) { |
| 3766 | struct hlist_head *hhd = &ftrace_func_hash[i]; | 3766 | struct hlist_head *hhd = &ftrace_func_hash[i]; |
| 3767 | if (hhd->first) | 3767 | if (hhd->first) |
| 3768 | return; | 3768 | return false; |
| 3769 | } | 3769 | } |
| 3770 | 3770 | ||
| 3771 | /* no more funcs left */ | 3771 | /* no more funcs left */ |
| 3772 | ftrace_shutdown(&trace_probe_ops, 0); | 3772 | ftrace_shutdown(&trace_probe_ops, 0); |
| 3773 | 3773 | ||
| 3774 | ftrace_probe_registered = 0; | 3774 | ftrace_probe_registered = 0; |
| 3775 | return true; | ||
| 3775 | } | 3776 | } |
| 3776 | 3777 | ||
| 3777 | 3778 | ||
| @@ -3901,6 +3902,7 @@ static void · 123 unmodified lines | |||
| 3901 | __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops, | 3902 | __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops, |
| 3902 | void *data, int flags) | 3903 | void *data, int flags) |
| 3903 | { | 3904 | { |
| 3905 | struct ftrace_ops_hash old_hash_ops; | ||
| 3904 | struct ftrace_func_entry *rec_entry; | 3906 | struct ftrace_func_entry *rec_entry; |
| 3905 | struct ftrace_func_probe *entry; | 3907 | struct ftrace_func_probe *entry; |
| 3906 | struct ftrace_func_probe *p; | 3908 | struct ftrace_func_probe *p; |
| @@ -3912,6 +3914,7 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops, · 5 unmodified lines | |||
| 3912 | struct hlist_node *tmp; | 3914 | struct hlist_node *tmp; |
| 3913 | char str[KSYM_SYMBOL_LEN]; | 3915 | char str[KSYM_SYMBOL_LEN]; |
| 3914 | int i, ret; | 3916 | int i, ret; |
| 3917 | bool disabled; | ||
| 3915 | 3918 | ||
| 3916 | if (glob && (strcmp(glob, "*") == 0 || !strlen(glob))) | 3919 | if (glob && (strcmp(glob, "*") == 0 || !strlen(glob))) |
| 3917 | func_g.search = NULL; | 3920 | func_g.search = NULL; |
| @@ -3930,6 +3933,10 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops, · 12 unmodified lines | |||
| 3930 | 3933 | ||
| 3931 | mutex_lock(&trace_probe_ops.func_hash->regex_lock); | 3934 | mutex_lock(&trace_probe_ops.func_hash->regex_lock); |
| 3932 | 3935 | ||
| 3936 | old_hash_ops.filter_hash = old_hash; | ||
| 3937 | /* Probes only have filters */ | ||
| 3938 | old_hash_ops.notrace_hash = NULL; | ||
| 3939 | |||
| 3933 | hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, *orig_hash); | 3940 | hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, *orig_hash); |
| 3934 | if (!hash) | 3941 | if (!hash) |
| 3935 | /* Hmm, should report this somehow */ | 3942 | /* Hmm, should report this somehow */ |
| @@ -3967,12 +3974,17 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops, · 31 unmodified lines | |||
| 3967 | } | 3974 | } |
| 3968 | } | 3975 | } |
| 3969 | mutex_lock(&ftrace_lock); | 3976 | mutex_lock(&ftrace_lock); |
| 3970 | __disable_ftrace_function_probe(); | 3977 | disabled = __disable_ftrace_function_probe(); |
| 3971 | /* | 3978 | /* |
| 3972 | * Remove after the disable is called. Otherwise, if the last | 3979 | * Remove after the disable is called. Otherwise, if the last |
| 3973 | * probe is removed, a null hash means *all enabled*. | 3980 | * probe is removed, a null hash means *all enabled*. |
| 3974 | */ | 3981 | */ |
| 3975 | ret = ftrace_hash_move(&trace_probe_ops, 1, orig_hash, hash); | 3982 | ret = ftrace_hash_move(&trace_probe_ops, 1, orig_hash, hash); |
| 3983 | |||
| 3984 | /* still need to update the function call sites */ | ||
| 3985 | if (ftrace_enabled && !disabled) | ||
| 3986 | ftrace_run_modify_code(&trace_probe_ops, FTRACE_UPDATE_CALLS, | ||
| 3987 | &old_hash_ops); | ||
| 3976 | synchronize_sched(); | 3988 | synchronize_sched(); |
| 3977 | if (!ret) | 3989 | if (!ret) |
| 3978 | free_ftrace_hash_rcu(old_hash); | 3990 | free_ftrace_hash_rcu(old_hash); |