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

From
Steven Rostedt <rostedt@goodmis.org>
Date
2026-10-26 20:33:01
Also in
linux-kselftest, lkml
Reply to
Masami Hiramatsu (Google), 2026-10-25
On Sun, 25 Oct 2026 17:37:56 +0900
"Masami Hiramatsu (Google)" [off-list ref] 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: <redacted>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Thread (20 messages) 20 messages, 3 authors, last reply 16d ago
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help