Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1171843 > unrolled thread

Re: [GIT PULL] tracing: Have filter check for balanced ops

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [GIT PULL] tracing: Have filter check for balanced ops Steven Rostedt <rostedt@goodmis.org> - 2015-06-25 06:40 +0200
    Re: [GIT PULL] tracing: Have filter check for balanced ops Steven Rostedt <rostedt@goodmis.org> - 2015-06-26 01:10 +0200

#1171843 — Re: [GIT PULL] tracing: Have filter check for balanced ops

On Thu, 25 Jun 2015 00:03:02 -0400
Sasha Levin <sasha.levin@oracle.com> wrote:

> On 06/17/2015 08:36 AM, Steven Rostedt wrote:
> > Linus,
> > 
> > Vince Weaver reported a warning when he added perf event filters
> > into his fuzzer tests. There's a missing check of balanced
> > operations when parenthesis are used, and this triggers a WARN_ON()
> > and when reading the failure, the filter reports no failure occurred.
> 
> Hey Steven,
> 
> My fuzzings are hitting the warning added by this patch:

Yes, Vince said he was able to hit it as well. But the warning itself
is useless if you don't supply what filter was used to trigger it.

-- Steve


> 
> [2175114.187536] WARNING: CPU: 16 PID: 10388 at kernel/trace/trace_events_filter.c:1388 replace_preds+0x814/0x2140()
> [2175114.190213] Modules linked in:
> [2175114.191111] CPU: 16 PID: 10388 Comm: trinity-c48 Not tainted 4.1.0-next-20150623-sasha-00039-ga1eb83a-dirty #2280
> [2175114.194463]  ffff880a23350000 000000006a8e22d4 ffff880a2335f878 ffffffffabc8cfa3
> [2175114.196547]  0000000000000000 0000000000000000 ffff880a2335f8c8 ffffffffa21ebd36
> [2175114.198604]  ffff880e60fe09e0 ffffffffa24608f4 ffff880e61b14830 ffff880e60fe09d8
> [2175114.200666] Call Trace:
> [2175114.201377]  [<ffffffffabc8cfa3>] dump_stack+0x4f/0x7b
> [2175114.202793]  [<ffffffffa21ebd36>] warn_slowpath_common+0xc6/0x120
> [2175114.206235]  [<ffffffffa21ebf7a>] warn_slowpath_null+0x1a/0x20
> [2175114.207819]  [<ffffffffa24608f4>] replace_preds+0x814/0x2140
> [2175114.216433]  [<ffffffffa24638aa>] create_filter+0x15a/0x210
> [2175114.231529]  [<ffffffffa246406b>] apply_event_filter+0x28b/0x780
> [2175114.241196]  [<ffffffffa2450306>] event_filter_write+0x106/0x1c0
> [2175114.242823]  [<ffffffffa260dd48>] do_loop_readv_writev+0x128/0x1e0
> [2175114.248901]  [<ffffffffa261051e>] do_readv_writev+0x5ae/0x6c0
> [2175114.256760]  [<ffffffffa2610742>] vfs_writev+0x72/0xb0
> [2175114.258134]  [<ffffffffa2613bb4>] SyS_pwritev+0x1b4/0x220
> [2175114.261291]  [<ffffffffabcf9962>] tracesys_phase2+0x88/0x8d
> 
> 
> Thanks,
> Sasha

--
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/

[toc] | [next] | [standalone]

#1172510

On Thu, 25 Jun 2015 16:11:46 -0400
Sasha Levin <sasha.levin@oracle.com> wrote:


> # echo ">" > events/ext4/ext4_truncate_exit/filter

Thanks. Seems that the WARN_ON is frivolous. We should just handle 
cnt < 0 as a bad filter.

Can you run you tests with this patch. Thanks!

-- Steve

From b4875bbe7e68f139bd3383828ae8e994a0df6d28 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Date: Thu, 25 Jun 2015 18:02:29 -0400
Subject: [PATCH] tracing/filter: Do not WARN on operand count going below zero

When testing the fix for the trace filter, I could not come up with
a scenario where the operand count goes below zero, so I added a
WARN_ON_ONCE(cnt < 0) to the logic. But there is legitimate case
that it can happen (although the filter would be wrong).

 # echo '>' > /sys/kernel/debug/events/ext4/ext4_truncate_exit/filter

That is, a single operation without any operands will hit the path
where the WARN_ON_ONCE() can trigger. Although this is harmless,
and the filter is reported as a error. But instead of spitting out
a warning to the kernel dmesg, just fail nicely and report it via
the proper channels.

Link: http://lkml.kernel.org/r/558C6082.90608@oracle.com

Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Cc: stable@vger.kernel.org # 2.6.33+
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_events_filter.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 7f2e97ce71a7..2900d7723d97 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1385,7 +1385,9 @@ static int check_preds(struct filter_parse_state *ps)
 		if (elt->op != OP_NOT)
 			cnt--;
 		n_normal_preds++;
-		WARN_ON_ONCE(cnt < 0);
+		/* all ops should have operands */
+		if (cnt < 0)
+			break;
 	}
 
 	if (cnt != 1 || !n_normal_preds || n_logical_preds >= n_normal_preds) {
-- 
2.1.4


--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web