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>] [<thread-prev] [day] [month] [year] [list]
Date:	Wed, 8 Jul 2015 11:57:04 +0800
From:	Fengguang Wu <fengguang.wu@...el.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	LKML <linux-kernel@...r.kernel.org>, LKP <lkp@...org>
Subject: Re: [trace] kernel BUG at kernel/sched/core.c:2881!

On Tue, Jul 07, 2015 at 06:37:11PM -0400, Steven Rostedt wrote:
> On Tue, 7 Jul 2015 15:03:31 -0400
> Steven Rostedt <rostedt@...dmis.org> wrote:
> 
> > On Tue, 7 Jul 2015 11:06:27 -0400
> > Steven Rostedt <rostedt@...dmis.org> wrote:
> > 
> > > On Tue, 30 Jun 2015 22:18:03 +0800
> > > Fengguang Wu <fengguang.wu@...el.com> wrote:
> > > 
> > > > Hi Rostedt,
> > > > 
> > > > FYI, this merge changes kernel crash to some more obvious kernel BUG
> > > > message. If it's still not helpful, I can try bisect the old crash
> > > > or split up the merge and bisect into them.
> > > > 
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > > 
> > > 
> > > This has been a bug for some time. Does this fix it?
> > > 
> > 
> > Test this patch instead, this is the one I'm testing and will be
> > pushing to Linus.
> > 
> 
> Um, third time's a charm. Found yet another area that can recurse.

Steven, it works!

Tested-by: Fengguang Wu <fengguang.wu@...el.com>

Thanks,
Fengguang

> -- Steve
> 
> >From efd39d5241abf5bde995b1d324d619cca0a23b71 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Red Hat)" <rostedt@...dmis.org>
> Date: Tue, 7 Jul 2015 15:05:03 -0400
> Subject: [PATCH] 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>
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
> ---
>  kernel/trace/trace.h        |  1 +
>  kernel/trace/trace_branch.c | 17 ++++++++++-------
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> 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
> -- 
> 1.8.3.1
--
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/

Powered by blists - more mailing lists