[PATCH] tracing: fix invalid function_graph entry

[PATCH] tracing: fix invalid function_graph entry

[Lai Jiangshan]

print_graph_entry() consumes current event, if this event
is the last event in the page, the ring_buffer may reuse
the page. It will become invalid.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index abf7c4a..02102a3 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -835,9 +835,16 @@ print_graph_function(struct trace_iterator *iter)
 
 	switch (entry->type) {
 	case TRACE_GRAPH_ENT: {
-		struct ftrace_graph_ent_entry *field;
+		/*
+		 * print_graph_entry() may consume the current event,
+		 * thus @field may become invalid, so we need to save it.
+		 * sizeof(struct ftrace_graph_ent_entry) is very small,
+		 * it is safely saved at the stack.
+		 */
+		struct ftrace_graph_ent_entry *field, saved;
 		trace_assign_type(field, entry);
-		return print_graph_entry(field, s, iter);
+		saved = *field;
+		return print_graph_entry(&saved, s, iter);
 	}
 	case TRACE_GRAPH_RET: {
 		struct ftrace_graph_ret_entry *field;

Re: [PATCH] tracing: fix invalid function_graph entry

[Frederic Weisbecker]

On Tue, Jul 28, 2009 at 08:11:24PM +0800, Lai Jiangshan wrote:
> 
> print_graph_entry() consumes current event, if this event
> is the last event in the page, the ring_buffer may reuse
> the page. It will become invalid.
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---


Queued for .31, thanks Lai!


> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index abf7c4a..02102a3 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -835,9 +835,16 @@ print_graph_function(struct trace_iterator *iter)
>  
>  	switch (entry->type) {
>  	case TRACE_GRAPH_ENT: {
> -		struct ftrace_graph_ent_entry *field;
> +		/*
> +		 * print_graph_entry() may consume the current event,
> +		 * thus @field may become invalid, so we need to save it.
> +		 * sizeof(struct ftrace_graph_ent_entry) is very small,
> +		 * it is safely saved at the stack.
> +		 */
> +		struct ftrace_graph_ent_entry *field, saved;
>  		trace_assign_type(field, entry);
> -		return print_graph_entry(field, s, iter);
> +		saved = *field;
> +		return print_graph_entry(&saved, s, iter);
>  	}
>  	case TRACE_GRAPH_RET: {
>  		struct ftrace_graph_ret_entry *field;
> 

[tip:tracing/urgent] tracing: Fix invalid function_graph entry

[tip-bot for Lai Jiangshan]

Commit-ID:  38ceb592fcac9110c6b3c87ea0a27bff68c43486
Gitweb:     http://git.kernel.org/tip/38ceb592fcac9110c6b3c87ea0a27bff68c43486
Author:     Lai Jiangshan <laijs@cn.fujitsu.com>
AuthorDate: Tue, 28 Jul 2009 20:11:24 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Tue, 28 Jul 2009 23:17:23 +0200

tracing: Fix invalid function_graph entry

When print_graph_entry() computes a function call entry event, it needs
to also check the next entry to guess if it matches the return event of
the current function entry.
In order to look at this next event, it needs to consume the current
entry before going ahead in the ring buffer.

However, if the current event that gets consumed is the last one in the
ring buffer head page, the ring_buffer may reuse the page for writers.
The consumed entry will then become invalid because of possible
racy overwriting.

Me must then handle this entry by making a copy of it.

The fix also applies on 2.6.30

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: stable@kernel.org
LKML-Reference: <4A6EEAEC.3050508@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>


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

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index d2249ab..420ec34 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -843,9 +843,16 @@ print_graph_function(struct trace_iterator *iter)
 
 	switch (entry->type) {
 	case TRACE_GRAPH_ENT: {
-		struct ftrace_graph_ent_entry *field;
+		/*
+		 * print_graph_entry() may consume the current event,
+		 * thus @field may become invalid, so we need to save it.
+		 * sizeof(struct ftrace_graph_ent_entry) is very small,
+		 * it can be safely saved at the stack.
+		 */
+		struct ftrace_graph_ent_entry *field, saved;
 		trace_assign_type(field, entry);
-		return print_graph_entry(field, s, iter);
+		saved = *field;
+		return print_graph_entry(&saved, s, iter);
 	}
 	case TRACE_GRAPH_RET: {
 		struct ftrace_graph_ret_entry *field;