linux-kernel - [GIT PULL] tracing: Have branch tracer use recursive field of task struct

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Sun, 12 Jul 2015 19:34:15 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Fengguang Wu <fengguang.wu@...el.com>,
	Ingo Molnar <mingo@...nel.org>
Subject: [GIT PULL] tracing: Have branch tracer use recursive field of task
 struct


Linus,

Fengguang Wu discovered a crash that happened to be because of the branch
tracer (traces unlikely and likely branches) when enabled with certain
debug options.

What happened was that various debug options like lockdep and DEBUG_PREEMPT
can cause parts of the branch tracer to recurse outside its recursion
protection. In fact, part of its recursion protection used these features
that caused the lockup. This cleans up the code a little and makes the
recursion protection a bit more robust.

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


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v4.2-rc1-fix

Tag SHA1: fc00e43412e3155cb9f23b9ab9ddaa07d1304b22
Head SHA1: 6224beb12e190ff11f3c7d4bf50cb2922878f600


Steven Rostedt (Red Hat) (1):
      tracing: Have branch tracer use recursive field of task struct

----
 kernel/trace/trace.h        |  1 +
 kernel/trace/trace_branch.c | 17 ++++++++++-------
 2 files changed, 11 insertions(+), 7 deletions(-)
---------------------------
commit 6224beb12e190ff11f3c7d4bf50cb2922878f600
Author: Steven Rostedt (Red Hat) <rostedt@...dmis.org>
Date:   Tue Jul 7 15:05:03 2015 -0400

    tracing: Have branch tracer use recursive field of task struct
    
    Fengguang Wu's tests triggered a bug in the branch tracer's start up
    test when CONFIG_DEBUG_PREEMPT set. This was because that config
    adds some debug logic in the per cpu field, which calls back into
    the branch tracer.
    
    The branch tracer has its own recursive checks, but uses a per cpu
    variable to implement it. If retrieving the per cpu variable calls
    back into the branch tracer, you can see how things will break.
    
    Instead of using a per cpu variable, use the trace_recursion field
    of the current task struct. Simply set a bit when entering the
    branch tracing and clear it when leaving. If the bit is set on
    entry, just don't do the tracing.
    
    There's also the case with lockdep, as the local_irq_save() called
    before the recursion can also trigger code that can call back into
    the function. Changing that to a raw_local_irq_save() will protect
    that as well.
    
    This prevents the recursion and the inevitable crash that follows.
    
    Link: http://lkml.kernel.org/r/20150630141803.GA28071@wfg-t540p.sh.intel.com
    
    Cc: stable@...r.kernel.org # 3.10+
    Reported-by: Fengguang Wu <fengguang.wu@...el.com>
    Tested-by: Fengguang Wu <fengguang.wu@...el.com>
    Signed-off-by: Steven Rostedt <rostedt@...dmis.org>

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index f060716b02ae..74bde81601a9 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -444,6 +444,7 @@ enum {
 
 	TRACE_CONTROL_BIT,
 
+	TRACE_BRANCH_BIT,
 /*
  * Abuse of the trace_recursion.
  * As we need a way to maintain state if we are tracing the function
diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c
index a87b43f49eb4..e2e12ad3186f 100644
--- a/kernel/trace/trace_branch.c
+++ b/kernel/trace/trace_branch.c
@@ -36,9 +36,12 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect)
 	struct trace_branch *entry;
 	struct ring_buffer *buffer;
 	unsigned long flags;
-	int cpu, pc;
+	int pc;
 	const char *p;
 
+	if (current->trace_recursion & TRACE_BRANCH_BIT)
+		return;
+
 	/*
 	 * I would love to save just the ftrace_likely_data pointer, but
 	 * this code can also be used by modules. Ugly things can happen
@@ -49,10 +52,10 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect)
 	if (unlikely(!tr))
 		return;
 
-	local_irq_save(flags);
-	cpu = raw_smp_processor_id();
-	data = per_cpu_ptr(tr->trace_buffer.data, cpu);
-	if (atomic_inc_return(&data->disabled) != 1)
+	raw_local_irq_save(flags);
+	current->trace_recursion |= TRACE_BRANCH_BIT;
+	data = this_cpu_ptr(tr->trace_buffer.data);
+	if (atomic_read(&data->disabled))
 		goto out;
 
 	pc = preempt_count();
@@ -81,8 +84,8 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect)
 		__buffer_unlock_commit(buffer, event);
 
  out:
-	atomic_dec(&data->disabled);
-	local_irq_restore(flags);
+	current->trace_recursion &= ~TRACE_BRANCH_BIT;
+	raw_local_irq_restore(flags);
 }
 
 static inline
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/