[PATCH] tracing/core: fix early free of cpumasks

[PATCH] tracing/core: fix early free of cpumasks

[Frederic Weisbecker]

Impact: fix weird bugs with tracing cpumasks

While ring-buffer allocation, the cpumasks are allocated too, including
the tracing cpumask and the per-cpu file mask handler.
But these cpumasks are freed accidentally just after.
Fix it.

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/trace.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c0cf946..ae32d3b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4125,7 +4125,8 @@ __init static int tracer_alloc_buffers(void)
 				       &trace_panic_notifier);
 
 	register_die_notifier(&trace_die_notifier);
-	ret = 0;
+
+	return 0;
 
 out_free_cpumask:
 	free_cpumask_var(tracing_reader_cpumask);
-- 
1.6.1

[tip:tracing/syscalls] tracing/core: fix early free of cpumasks

[Frederic Weisbecker]

Commit-ID:  2fc1dfbe17e7705c55b7a99da995fa565e26f151
Gitweb:     http://git.kernel.org/tip/2fc1dfbe17e7705c55b7a99da995fa565e26f151
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Mon, 16 Mar 2009 01:45:03 +0100
Commit:     Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 16 Mar 2009 09:13:18 +0100

tracing/core: fix early free of cpumasks

Impact: fix crashes when tracing cpumasks

While ring-buffer allocation, the cpumasks are allocated too,
including the tracing cpumask and the per-cpu file mask handler.
But these cpumasks are freed accidentally just after.
Fix it.

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <1237164303-11476-1-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/trace/trace.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c0cf946..ae32d3b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4125,7 +4125,8 @@ __init static int tracer_alloc_buffers(void)
 				       &trace_panic_notifier);
 
 	register_die_notifier(&trace_die_notifier);
-	ret = 0;
+
+	return 0;
 
 out_free_cpumask:
 	free_cpumask_var(tracing_reader_cpumask);

Re: [tip:tracing/syscalls] tracing/core: fix early free of cpumasks

[Steven Rostedt]

This never made it into 2.6.29, nor do I see it yet in mainline.

This can cause unpredictable results with ftrace. We are freeing a cpu mask
that will be later used.

This is definitely stable material.

Thanks to Johannes Berg for patiently bisecting his branch with a merge of my
branch to locate the patch that was missing from mainline.

He was seeing empty traces, but in reality, the changing of these masks might
have more dire consequences.

-- Steve

On Mon, Mar 16, 2009 at 08:19:33AM +0000, Frederic Weisbecker wrote:
> Commit-ID:  2fc1dfbe17e7705c55b7a99da995fa565e26f151
> Gitweb:     http://git.kernel.org/tip/2fc1dfbe17e7705c55b7a99da995fa565e26f151
> Author:     Frederic Weisbecker <fweisbec@gmail.com>
> AuthorDate: Mon, 16 Mar 2009 01:45:03 +0100
> Commit:     Ingo Molnar <mingo@elte.hu>
> CommitDate: Mon, 16 Mar 2009 09:13:18 +0100
> 
> tracing/core: fix early free of cpumasks
> 
> Impact: fix crashes when tracing cpumasks
> 
> While ring-buffer allocation, the cpumasks are allocated too,
> including the tracing cpumask and the per-cpu file mask handler.
> But these cpumasks are freed accidentally just after.
> Fix it.
> 
> Reported-by: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> LKML-Reference: <1237164303-11476-1-git-send-email-fweisbec@gmail.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> 
> ---
>  kernel/trace/trace.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index c0cf946..ae32d3b 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -4125,7 +4125,8 @@ __init static int tracer_alloc_buffers(void)
>  				       &trace_panic_notifier);
>  
>  	register_die_notifier(&trace_die_notifier);
> -	ret = 0;
> +
> +	return 0;
>  
>  out_free_cpumask:
>  	free_cpumask_var(tracing_reader_cpumask);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [tip:tracing/syscalls] tracing/core: fix early free of cpumasks

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> This never made it into 2.6.29, nor do I see it yet in mainline.
> 
> This can cause unpredictable results with ftrace. We are freeing a 
> cpu mask that will be later used.

Yes ... but CONFIG_OFFSTACK is only possible as a debug option right 
now. So it's far less serious of an issue, it needs 2-3 unlikely 
features to be combined.

The fix looks simple enough, so once the tracing tree is upstream, 
we can forward it to -stable.

	Ingo

Re: [tip:tracing/syscalls] tracing/core: fix early free of cpumasks

[Steven Rostedt]

On Thu, 2 Apr 2009, Ingo Molnar wrote:

> 
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > This never made it into 2.6.29, nor do I see it yet in mainline.
> > 
> > This can cause unpredictable results with ftrace. We are freeing a 
> > cpu mask that will be later used.
> 
> Yes ... but CONFIG_OFFSTACK is only possible as a debug option right 
> now. So it's far less serious of an issue, it needs 2-3 unlikely 
> features to be combined.

And unfortunately for Johannes, he must have had all those features 
configured :-/  He spent pretty much the entire day trying to figure out 
why ftrace was broken.

-- Steve

> 
> The fix looks simple enough, so once the tracing tree is upstream, 
> we can forward it to -stable.
> 
> 	Ingo
> 

Re: [tip:tracing/syscalls] tracing/core: fix early free of cpumasks

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> On Thu, 2 Apr 2009, Ingo Molnar wrote:
> 
> > 
> > * Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > This never made it into 2.6.29, nor do I see it yet in mainline.
> > > 
> > > This can cause unpredictable results with ftrace. We are freeing a 
> > > cpu mask that will be later used.
> > 
> > Yes ... but CONFIG_OFFSTACK is only possible as a debug option right 
> > now. So it's far less serious of an issue, it needs 2-3 unlikely 
> > features to be combined.
> 
> And unfortunately for Johannes, he must have had all those 
> features configured :-/ He spent pretty much the entire day trying 
> to figure out why ftrace was broken.

Okay - will forward the fix to -stable once it's upstream.

	Ingo