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

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 22 Jun 2016 09:08:14 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Chunyu Hu <chuhu@...hat.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tracing: Fix oops caused by graph notrace filter

On Tue, 21 Jun 2016 18:57:46 +0800
Chunyu Hu <chuhu@...hat.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;