Patch "tracing: Have filter check for balanced ops" has been added to the 3.14-stable tree

Patch "tracing: Have filter check for balanced ops" has been added to the 3.14-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    tracing: Have filter check for balanced ops

to the 3.14-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     tracing-have-filter-check-for-balanced-ops.patch
and it can be found in the queue-3.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From 2cf30dc180cea808077f003c5116388183e54f9e Mon Sep 17 00:00:00 2001
From: Steven Rostedt <rostedt@goodmis.org>
Date: Mon, 15 Jun 2015 17:50:25 -0400
Subject: tracing: Have filter check for balanced ops

From: Steven Rostedt <rostedt@goodmis.org>

commit 2cf30dc180cea808077f003c5116388183e54f9e upstream.

When the following filter is used it causes a warning to trigger:

 # cd /sys/kernel/debug/tracing
 # echo "((dev==1)blocks==2)" > events/ext4/ext4_truncate_exit/filter
-bash: echo: write error: Invalid argument
 # cat events/ext4/ext4_truncate_exit/filter
((dev==1)blocks==2)
^
parse_error: No error

 ------------[ cut here ]------------
 WARNING: CPU: 2 PID: 1223 at kernel/trace/trace_events_filter.c:1640 replace_preds+0x3c5/0x990()
 Modules linked in: bnep lockd grace bluetooth  ...
 CPU: 3 PID: 1223 Comm: bash Tainted: G        W       4.1.0-rc3-test+ #450
 Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v02.05 05/07/2012
  0000000000000668 ffff8800c106bc98 ffffffff816ed4f9 ffff88011ead0cf0
  0000000000000000 ffff8800c106bcd8 ffffffff8107fb07 ffffffff8136b46c
  ffff8800c7d81d48 ffff8800d4c2bc00 ffff8800d4d4f920 00000000ffffffea
 Call Trace:
  [<ffffffff816ed4f9>] dump_stack+0x4c/0x6e
  [<ffffffff8107fb07>] warn_slowpath_common+0x97/0xe0
  [<ffffffff8136b46c>] ? _kstrtoull+0x2c/0x80
  [<ffffffff8107fb6a>] warn_slowpath_null+0x1a/0x20
  [<ffffffff81159065>] replace_preds+0x3c5/0x990
  [<ffffffff811596b2>] create_filter+0x82/0xb0
  [<ffffffff81159944>] apply_event_filter+0xd4/0x180
  [<ffffffff81152bbf>] event_filter_write+0x8f/0x120
  [<ffffffff811db2a8>] __vfs_write+0x28/0xe0
  [<ffffffff811dda43>] ? __sb_start_write+0x53/0xf0
  [<ffffffff812e51e0>] ? security_file_permission+0x30/0xc0
  [<ffffffff811dc408>] vfs_write+0xb8/0x1b0
  [<ffffffff811dc72f>] SyS_write+0x4f/0xb0
  [<ffffffff816f5217>] system_call_fastpath+0x12/0x6a
 ---[ end trace e11028bd95818dcd ]---

Worse yet, reading the error message (the filter again) it says that
there was no error, when there clearly was. The issue is that the
code that checks the input does not check for balanced ops. That is,
having an op between a closed parenthesis and the next token.

This would only cause a warning, and fail out before doing any real
harm, but it should still not caues a warning, and the error reported
should work:

 # cd /sys/kernel/debug/tracing
 # echo "((dev==1)blocks==2)" > events/ext4/ext4_truncate_exit/filter
-bash: echo: write error: Invalid argument
 # cat events/ext4/ext4_truncate_exit/filter
((dev==1)blocks==2)
^
parse_error: Meaningless filter expression

And give no kernel warning.

Link: http://lkml.kernel.org/r/20150615175025.7e809215@gandalf.local.home

Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Tested-by: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1399,19 +1399,26 @@ static int check_preds(struct filter_par
 {
 	int n_normal_preds = 0, n_logical_preds = 0;
 	struct postfix_elt *elt;
+	int cnt = 0;
 
 	list_for_each_entry(elt, &ps->postfix, list) {
-		if (elt->op == OP_NONE)
+		if (elt->op == OP_NONE) {
+			cnt++;
 			continue;
+		}
 
 		if (elt->op == OP_AND || elt->op == OP_OR) {
 			n_logical_preds++;
+			cnt--;
 			continue;
 		}
+		if (elt->op != OP_NOT)
+			cnt--;
 		n_normal_preds++;
+		WARN_ON_ONCE(cnt < 0);
 	}
 
-	if (!n_normal_preds || n_logical_preds >= n_normal_preds) {
+	if (cnt != 1 || !n_normal_preds || n_logical_preds >= n_normal_preds) {
 		parse_error(ps, FILT_ERR_INVALID_FILTER, 0);
 		return -EINVAL;
 	}


Patches currently in stable-queue which might be from rostedt@goodmis.org are

queue-3.14/tracing-have-filter-check-for-balanced-ops.patch
--
To unsubscribe from this list: send the line "unsubscribe stable" in

Re: Patch "tracing: Have filter check for balanced ops" has been added to the 3.14-stable tree

[Jiri Slaby]

On 06/23/2015, 05:22 AM, gregkh@linuxfoundation.org wrote:
> 
> This is a note to let you know that I've just added the patch titled
> 
>     tracing: Have filter check for balanced ops
> 
> to the 3.14-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      tracing-have-filter-check-for-balanced-ops.patch
> and it can be found in the queue-3.14 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
> 
> 
> From 2cf30dc180cea808077f003c5116388183e54f9e Mon Sep 17 00:00:00 2001
> From: Steven Rostedt <rostedt@goodmis.org>
> Date: Mon, 15 Jun 2015 17:50:25 -0400
> Subject: tracing: Have filter check for balanced ops
> 
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> commit 2cf30dc180cea808077f003c5116388183e54f9e upstream.

...

> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1399,19 +1399,26 @@ static int check_preds(struct filter_par
>  {
>  	int n_normal_preds = 0, n_logical_preds = 0;
>  	struct postfix_elt *elt;
> +	int cnt = 0;
>  
>  	list_for_each_entry(elt, &ps->postfix, list) {
> -		if (elt->op == OP_NONE)
> +		if (elt->op == OP_NONE) {
> +			cnt++;
>  			continue;
> +		}
>  
>  		if (elt->op == OP_AND || elt->op == OP_OR) {
>  			n_logical_preds++;
> +			cnt--;
>  			continue;
>  		}
> +		if (elt->op != OP_NOT)

This breaks build, OP_NOT is not in 3.14 (or 3.10) yet. I dropped this
line for 3.12.

> +			cnt--;
>  		n_normal_preds++;
> +		WARN_ON_ONCE(cnt < 0);
>  	}
>  
> -	if (!n_normal_preds || n_logical_preds >= n_normal_preds) {
> +	if (cnt != 1 || !n_normal_preds || n_logical_preds >= n_normal_preds) {
>  		parse_error(ps, FILT_ERR_INVALID_FILTER, 0);
>  		return -EINVAL;
>  	}
> 
> 

thanks,
-- 
js
suse labs

Re: Patch "tracing: Have filter check for balanced ops" has been added to the 3.14-stable tree

[Steven Rostedt]

On Tue, 23 Jun 2015 15:22:50 +0200
Jiri Slaby <jslaby@suse.cz> wrote:


> > --- a/kernel/trace/trace_events_filter.c
> > +++ b/kernel/trace/trace_events_filter.c
> > @@ -1399,19 +1399,26 @@ static int check_preds(struct filter_par
> >  {
> >  	int n_normal_preds = 0, n_logical_preds = 0;
> >  	struct postfix_elt *elt;
> > +	int cnt = 0;
> >  
> >  	list_for_each_entry(elt, &ps->postfix, list) {
> > -		if (elt->op == OP_NONE)
> > +		if (elt->op == OP_NONE) {
> > +			cnt++;
> >  			continue;
> > +		}
> >  
> >  		if (elt->op == OP_AND || elt->op == OP_OR) {
> >  			n_logical_preds++;
> > +			cnt--;
> >  			continue;
> >  		}
> > +		if (elt->op != OP_NOT)
> 
> This breaks build, OP_NOT is not in 3.14 (or 3.10) yet. I dropped this
> line for 3.12.

Luis has a version he used for his stable tree, which should also work
for yours.

See this:

  http://lkml.kernel.org/r/20150622144940.GD2036@ares

-- Steve

> 
> > +			cnt--;
> >  		n_normal_preds++;
> > +		WARN_ON_ONCE(cnt < 0);
> >  	}
> >  
> > -	if (!n_normal_preds || n_logical_preds >= n_normal_preds) {
> > +	if (cnt != 1 || !n_normal_preds || n_logical_preds >= n_normal_preds) {
> >  		parse_error(ps, FILT_ERR_INVALID_FILTER, 0);
> >  		return -EINVAL;
> >  	}
> > 
> > 
> 
> thanks,

Re: Patch "tracing: Have filter check for balanced ops" has been added to the 3.14-stable tree

[Greg KH]

On Tue, Jun 23, 2015 at 09:40:59AM -0400, Steven Rostedt wrote:
> On Tue, 23 Jun 2015 15:22:50 +0200
> Jiri Slaby <jslaby@suse.cz> wrote:
> 
> 
> > > --- a/kernel/trace/trace_events_filter.c
> > > +++ b/kernel/trace/trace_events_filter.c
> > > @@ -1399,19 +1399,26 @@ static int check_preds(struct filter_par
> > >  {
> > >  	int n_normal_preds = 0, n_logical_preds = 0;
> > >  	struct postfix_elt *elt;
> > > +	int cnt = 0;
> > >  
> > >  	list_for_each_entry(elt, &ps->postfix, list) {
> > > -		if (elt->op == OP_NONE)
> > > +		if (elt->op == OP_NONE) {
> > > +			cnt++;
> > >  			continue;
> > > +		}
> > >  
> > >  		if (elt->op == OP_AND || elt->op == OP_OR) {
> > >  			n_logical_preds++;
> > > +			cnt--;
> > >  			continue;
> > >  		}
> > > +		if (elt->op != OP_NOT)
> > 
> > This breaks build, OP_NOT is not in 3.14 (or 3.10) yet. I dropped this
> > line for 3.12.
> 
> Luis has a version he used for his stable tree, which should also work
> for yours.
> 
> See this:
> 
>   http://lkml.kernel.org/r/20150622144940.GD2036@ares

Yes, now used, thanks.

greg k-h