| 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
| ||
|
Message-ID: <20150708035704.GA22002@wfg-t540p.sh.intel.com>
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