On Mon, 26 Oct 2026 16:32:57 -0400, Steven Rostedt <[email protected]> wrote:
> On Sun, 25 Oct 2026 17:37:56 +0900
> "Masami Hiramatsu (Google)" <[email protected]> wrote:
>
> > From: Masami Hiramatsu (Google) <[email protected]>
> >
> > In parse_pred(), the ftrace_event_is_function(call) branch has several
> > bugs when parsing the string operand:
> >
> > 1. The loop 'for (i++; str[i]; i++)' unconditionally increments 'i' before
> > checking whether str[i] is the NUL terminator. When the operand is empty
> > (e.g. 'ip == '), this advances 'i' past the NUL terminator and reads
> > out-of-bounds memory until another zero byte is encountered.
>
> Can't this part simply be fixed with:
>
> diff --git a/kernel/trace/trace_events_filter.c
> b/kernel/trace/trace_events_filter.c
> index 5e74d2dae40a..6a14c964c115 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1733,6 +1733,11 @@ static int parse_pred(const char *str, void *data,
> while (isspace(str[i]))
> i++;
>
> + if (!str[i]) {
> + parse_error(pe, FILT_ERR_INVALID_FILTER, pos + i);
> + goto err_free;
> + }
> +
> s = i;
>
> pred = kzalloc_obj(*pred);
Yeah, looks good the me.
>
> >
> > 2. When the operand is quoted (q != 0), the loop breaks with str[i] at the
> > closing quote, but 'i' is never incremented past it (unlike the regular
> > string literal parser which does 'i++'). Furthermore, it does not check
> > for a missing closing quote (!str[i]). Returning 'i' at the closing
> > quote causes predicate_parse() to immediately re-invoke parse_pred() on
> > the closing quote and fail with FILT_ERR_TOO_MANY_PREDS.
>
> This is a separate bug and needs to be a separate patch.
OK. Let's make it a separate patch.
>
> >
> > 3. If the operand length is 0 (e.g. 'ip == )' or 'ip == ""'), it accepts an
> > empty pattern instead of rejecting it with FILT_ERR_INVALID_VALUE.
>
> Well, empty value ("") is actually valid. May be meaningless, but it's not
> wrong.
OK, I will also add this case in the test.
>
> >
> > 4. When the operand is unquoted (q == 0), stray quotes or '(' inside the
> > operand are consumed as part of the pattern, desynchronizing
> > parse_pred() from calc_stack().
>
> This is a separate bug and needs to be a separate patch, although it may be
> the same as #2 above.
OK.
Thanks for your comments!
>
> -- Steve
>
> >
> > Fix this by advancing 'i' and 's' past the opening quote before the loop,
> > starting the loop at the current 'i' without a blind increment, checking
> > for missing closing quotes, advancing 'i' past the closing quote, and
> > rejecting empty operands or stray quotes/'(' in unquoted operands.
> >
> > Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and
> > faster")
> > Cc: [email protected]
> > Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
> > ---
>
--
Masami Hiramatsu (Google) <[email protected]>
