lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [day] [month] [year] [list]
Date:   Sat, 15 Apr 2017 17:11:35 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: [GIT PULL] ftrace: Fix removing of second function probe


Linus,

While rewriting the function probe code, I stumbled over a long standing
bug. This bug has been there since function tracing was added way back
when. But my new development depends on this bug being fixed, and it
should be fixed regardless as it causes ftrace to disable itself when
triggered, and a reboot is required to enable it again.

The bug is that the function probe does not disable itself properly
if there's another probe of its type still enabled. For example:

     # 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

The above registers two traceoff probes (one for schedule and one for
do_IRQ, and then removes do_IRQ. But since there still exists one for
schedule, it is not done properly. When adding do_IRQ back, the breakage
in the accounting is noticed by the ftrace self tests, and it causes
a warning and disables ftrace.


Please pull the latest trace-v4.11-rc5-2 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v4.11-rc5-2

Tag SHA1: fe8a1251ab0940f3807c2cc1228906681b3880da
Head SHA1: 82cc4fc2e70ec5baeff8f776f2773abc8b2cc0ae


Steven Rostedt (VMware) (1):
      ftrace: Fix removing of second function probe

----
 kernel/trace/ftrace.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
---------------------------
commit 82cc4fc2e70ec5baeff8f776f2773abc8b2cc0ae
Author: Steven Rostedt (VMware) <rostedt@...dmis.org>
Date:   Fri Apr 14 17:45:45 2017 -0400

    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@...r.kernel.org
    Fixes: 59df055f1991 ("ftrace: trace different functions with a different tracer")
    Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index b9691ee8f6c1..27bb2e61276e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3755,23 +3755,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;
 }
 
 
@@ -3901,6 +3902,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;
@@ -3912,6 +3914,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;
@@ -3930,6 +3933,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 */
@@ -3967,12 +3974,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);

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ