Re: [PATCH v3 09/16] tracing/filters: Reject stray quotes and parentheses in unquoted function event operand

[Masami Hiramatsu (Google)]

On Wed, 28 Oct 2026 09:50:37 -0400, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> On Wed, 28 Oct 2026 09:44:48 +0900
> "Masami Hiramatsu (Google)" <mhiramat@xxxxxxxxxx> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> > 
> > In parse_pred(), when parsing an unquoted operand for function events,
> > the loop only stops at ')', '&', '|', or the NUL terminator.
> > 
> > However, calc_stack() parses '(', '\'', and '"' independently to
> > determine stack depth and string boundaries. If an unquoted function
> > operand contains stray quotes or an open parenthesis (e.g. "ip == foo'bar"
> > or "ip == foo(bar)"), parse_pred() consumes them as part of the function
> > name operand, causing parse_pred() and calc_stack() to desynchronize.
> > 
> > Reject unquoted function event operands containing '(', '\'', or '"'
> > by reporting FILT_ERR_INVALID_VALUE.
> > 
> > Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
> > Cc: stable@xxxxxxxxxxxxxxx
> 
> I wouldn't really call this a fix, as I can't see this breaking anything.
> Yes, the calc_stack may say there's more elements than what parse_pred()
> sees, but the parsing still fails.
> 
> This looks more of a clean up than a fix.

Agreed. I think fixin error_log UI issue will be a part of improvement.
OK, let me remove the Fixes and Cc: stable tags from those patches.

> 
> Unless, the current code allows something to actually be broken. Does it?

No. Hmm, should I split those "improving parse_error()" patches from
this series?

Thanks,

> 
> -- Steve
> 
> 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> > ---
> > Changes in v3:
> >  - Split from the monolithic function event parsing patch into a
> >    standalone patch rejecting stray quotes and '(' in unquoted operands.
> > ---
> >  kernel/trace/trace_events_filter.c |   14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> > index 96977eb9af85..48e16d75be74 100644
> > --- a/kernel/trace/trace_events_filter.c
> > +++ b/kernel/trace/trace_events_filter.c
> > @@ -1863,9 +1863,17 @@ static int parse_pred(const char *str, void *data,
> >  		for (; str[i]; i++) {
> >  			if (q && str[i] == q)
> >  				break;
> > -			if (!q && (str[i] == ')' || str[i] == '&' ||
> > -				   str[i] == '|'))
> > -				break;
> > +			if (!q) {
> > +				if (str[i] == ')' || str[i] == '&' ||
> > +				    str[i] == '|')
> > +					break;
> > +				if (str[i] == '(' || str[i] == '\'' ||
> > +				    str[i] == '"') {
> > +					parse_error(pe, FILT_ERR_INVALID_VALUE,
> > +						    pos + i);
> > +					goto err_free;
> > +				}
> > +			}
> >  		}
> >  		if (q && !str[i]) {
> >  			parse_error(pe, FILT_ERR_MISSING_QUOTE, pos + i);
> 
> 
>

-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>