Re: [PATCH v2 5/9] tracing/filters: Fix function event operand parsing in parse_pred()

[Steven Rostedt <rostedt@goodmis.org>]

On Sun, 25 Oct 2026 17:37:56 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> 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);

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

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

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

-- 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: stable@vger.kernel.org
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---