Re: [PATCH] tracing: Fix oops caused by graph notrace filter

[Steven Rostedt]

On Tue, 21 Jun 2016 18:57:46 +0800
Chunyu Hu <ch...@redhat.com> wrote:

> wakeup tracer can use function_graph trace when display_graph trace
> option is setup by user via tracefs, and bypass the set_graph_function
> and set_graph_notrace. But the bypass of set_graph_notrace is not clean.
> Although wakeup_graph_entry does most of the bypass, and both the enry
> and exit event will be submitted to the trace ring buffer, the ret_stack
> index, which will be assigned to depth field of graph enrty event is not
> handled. The issue is that the depth is used as the array index of
> fgraph_cpu_data and can cause an oops when it's negative. irqsoff tracer
> has same issue. To see the oops:
> 
> echo 1 > options/display_graph
> echo schedule > set_graph_notrace
> echo wakeup > current_tracer
> cat trace
> cat trace

I'm not able to trigger an oops, but I do see them being traced.

> 
> Making ftrace_graph_notrace_addr always return false when tracers need
> to bypass it is a proposed fix.

There's a lot of things wrong with this patch, but before we go there,
why should the latency tracers ignore set_graph_notrace?

What about this patch?

-- Steve

diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 03cdff84d026..68fcee5097d2 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -175,6 +175,9 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent 
*trace)
        int ret;
        int pc;
 
+       if (trace->depth < 0)
+               return 0;
+
        if (!func_prolog_dec(tr, &data, &flags))
                return 0;
 
diff --git a/kernel/trace/trace_sched_wakeup.c 
b/kernel/trace/trace_sched_wakeup.c
index 9d4399b553a3..ce9889bff8b4 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -239,6 +239,9 @@ static int wakeup_graph_entry(struct ftrace_graph_ent 
*trace)
        unsigned long flags;
        int pc, ret = 0;
 
+       if (trace->depth < 0)
+               return 0;
+
        if (!func_prolog_preempt_disable(tr, &data, &pc))
                return 0;