[PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs

[PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs

[Masami Hiramatsu (Google)]

Hi,

Here is a series of fixes for various bugs in the trace event filter
parser and predicate evaluation in kernel/trace/trace_events_filter.c,
along with a new ftracetest test case covering filter syntax checks.

Summary of the patches:

- [01/10] Compute the actual string length with strnlen()/strlen() in
  regex_match_end() so suffix matching ("*pattern") works on fixed-size
  string fields (such as comm) and dynamic strings.
- [02/10] Fix process_system_preds() so that "fail" is only cleared when
  at least one event in the subsystem succeeds, and use
  delay_free_filter() when freeing replaced filters on error paths.
- [03/10] Check "cpu >= nr_cpu_ids" before calling cpumask_test_cpu() in
  do_filter_cpumask_scalar() to avoid out-of-bounds bit tests when
  NR_CPUS > nr_cpu_ids.
- [04/10] Return "pred->op == OP_NE" when "cpu >= nr_cpu_ids" in
  DEFINE_CPUMASK_COMPARISON_PRED so "!=" against a cpumask evaluates to
  true for out-of-range CPU numbers.
- [05/10] Ensure parse_error() is called on all -EINVAL error paths in
  parse_pred(), predicate_parse(), and process_preds() instead of
  leaving lasterr as 0 ("Error: (0)").
- [06/10] Fix function event ("ip") operand parsing in parse_pred() to
  avoid reading past the NUL terminator on empty operands, check for
  missing closing quotes, advance past closing quotes, and reject empty
  operands or stray quotes/'(' in unquoted operands.
- [07/10] Advance past the second character of "&&" and "||" in
  calc_stack() so the second operator character is not miscounted as a
  new predicate.
- [08/10] Reject unsupported field sizes in parse_pred() when
  select_comparison_fn() or CPUS{} size matching leaves pred->fn_num as
  FILTER_PRED_FN_NOP, avoiding a kernel warning in update_pred_fn().
- [09/10] Terminate the input filter string at the first newline ('\n')
  in create_filter_start(), apply_event_filter(), and
  apply_subsystem_event_filter() so multi-line input cannot desynchronize
  cpulist_parse()/parse_pred() from calc_stack() or misalign error_log
  caret placement.
- [10/10] Add filter-syntax-check.tc to ftrace selftests to test valid
  filters, syntax error diagnostics and caret positions, multi-line
  input truncation, and subsystem filter error returns.

Note: These bugs were identified and the fixes and cover letter were
prepared with the assistance of an LLM coding assistant.

Also, note that [09/10] may need more discussion because it can break
user space tool which attempts to use line-break characters as a filter
for reasons that are not entirely clear. But as far as I can see, the
error_log does not support such multi-line filter input. So I think
we still need it. (But not sure we should backport it, maybe as an
improvement.)

Thank you,

---
base-commit: 1fdc4164e488916402badddbb1f51e9380dabb91

Masami Hiramatsu (Google) (10):
      tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings
      tracing/filters: Fix false success return in process_system_preds()
      tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar()
      tracing/filters: Handle out-of-range CPU for OP_NE in cpumask comparison
      tracing/filters: Report parse error on all -EINVAL paths in filter parsing
      tracing/filters: Fix function event operand parsing in parse_pred()
      tracing/filters: Skip second character of '&&' and '||' in calc_stack()
      tracing/filters: Reject unsupported field sizes in parse_pred()
      tracing/filters: Terminate filter string at first newline
      selftests/ftrace: Add event filter syntax check test case


 kernel/trace/trace_events_filter.c                 |  100 +++++++++++++++-----
 .../ftrace/test.d/filter/filter-syntax-check.tc    |   52 ++++++++++
 2 files changed, 129 insertions(+), 23 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

regex_match_end() calculated string length as "len - 1", assuming @len is
the string length including trailing '\0'. However, for fixed-size string
fields, filter_pred_string() passes field->size as @len, and for "comm",
filter_pred_comm() passes TASK_COMM_LEN as @len.

Because @len in these cases represents the buffer capacity rather than the
actual length of the string, regex_match_end() tests against the tail of
the buffer (typically trailing '\0' padding) rather than the end of the
string. Furthermore, if @len is 0 (denoting a dynamic string ending with
'\0'), "len - 1" underflows to -1 and never matches.

Compute the actual string length using strnlen(str, len) when @len is
non-zero, and strlen(str) when @len is zero.

Fixes: a3291c14ecf0 ("tracing/filters: Fix MATCH_END_ONLY filter matching")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_events_filter.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 5e74d2dae40a..18ce3ec00d34 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1075,10 +1075,10 @@ static int regex_match_middle(char *str, struct regex *r, int len)
 
 static int regex_match_end(char *str, struct regex *r, int len)
 {
-	int strlen = len - 1;
+	int slen = len ? strnlen(str, len) : strlen(str);
 
-	if (strlen >= r->len &&
-	    memcmp(str + strlen - r->len, r->pattern, r->len) == 0)
+	if (slen >= r->len &&
+	    memcmp(str + slen - r->len, r->pattern, r->len) == 0)
 		return 1;
 	return 0;
 }

[PATCH 02/10] tracing/filters: Fix false success return in process_system_preds()

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

In process_system_preds(), "fail" is initialized to true to indicate
whether any event in the subsystem successfully applied the filter.
However, "fail = false;" was placed at the end of the event loop outside
the error check, unconditionally clearing "fail" on every iteration even
when process_preds() returned an error.

As a result, if every event in the subsystem fails to match the filter
(e.g., an invalid field), process_system_preds() returns 0 (success)
instead of -EINVAL. Consequently, create_system_filter() considers the
operation successful and silently resets the subsystem filter display to
the default global filter message instead of reporting the parse error.

Move "fail = false;" into the else branch of the error check so that
"fail" is only cleared when at least one event filter succeeds. Because
file->filter is still replaced even when process_preds() fails, check
!list_empty(&filter_list->list) on the fail and fail_mem paths so any
replaced old filters are freed after an RCU grace period via
delay_free_filter().

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>
---
 kernel/trace/trace_events_filter.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 18ce3ec00d34..c2c35b4de85e 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2332,8 +2332,10 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
 			filter_disable(file);
 			parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
 			append_filter_err(tr, pe, filter);
-		} else
+		} else {
 			event_set_filtered_flag(file);
+			fail = false;
+		}
 
 
 		filter_item = kzalloc_obj(*filter_item);
@@ -2348,8 +2350,6 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
 		filter_item->filter = event_filter(file);
 		event_set_filter(file, filter);
 		filter = NULL;
-
-		fail = false;
 	}
 
 	if (fail)
@@ -2364,14 +2364,17 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
 	return 0;
  fail:
 	/* No call succeeded */
-	free_filter_list(filter_list);
+	if (!list_empty(&filter_list->list))
+		delay_free_filter(filter_list);
+	else
+		free_filter_list(filter_list);
 	parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
 	return -EINVAL;
  fail_mem:
 	__free_filter(filter);
 
-	/* If any call succeeded, we still need to sync */
-	if (!fail)
+	/* If any filter was replaced, we still need to sync */
+	if (!list_empty(&filter_list->list))
 		delay_free_filter(filter_list);
 	else
 		free_filter_list(filter_list);

[PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar()

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

In parse_pred(), single-CPU cpumask filters are parsed via cpulist_parse(),
which checks against nr_cpumask_bits (NR_CPUS). When NR_CPUS > nr_cpu_ids,
a user can provide a CPU number where cpu >= nr_cpu_ids.

When evaluating such a predicate in filter_pred_cpumask_cpu(),
do_filter_cpumask_scalar() calls cpumask_test_cpu(cpu, mask) without
checking cpu against nr_cpu_ids. This triggers a WARN_ON_ONCE under
CONFIG_DEBUG_PER_CPU_MAPS and causes an out-of-bounds bit test.

Check "cpu >= nr_cpu_ids" at the start of do_filter_cpumask_scalar(). When
the CPU index is out of bounds, return "op == OP_NE" since an out-of-range
CPU is never present in a valid cpumask.

Fixes: 38c6f6808389 ("tracing/filters: Further optimise scalar vs cpumask comparison")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_events_filter.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index c2c35b4de85e..69da95f42541 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -695,6 +695,9 @@ do_filter_scalar_cpumask(int op, unsigned int cpu, const struct cpumask *mask)
 static inline int
 do_filter_cpumask_scalar(int op, const struct cpumask *mask, unsigned int cpu)
 {
+	if (cpu >= nr_cpu_ids)
+		return op == OP_NE;
+
 	switch (op) {
 	case OP_EQ:
 		return cpumask_test_cpu(cpu, mask) &&

[PATCH 04/10] tracing/filters: Handle out-of-range CPU for OP_NE in cpumask comparison

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

In filter_pred_##size##_cpumask(), if the event's scalar CPU value is
out of range (cpu >= nr_cpu_ids), it unconditionally returns 0.

However, if the predicate operation is OP_NE (e.g., field != CPUS{0,1}),
an out-of-range CPU cannot match any valid CPU in the user-provided
cpumask and the condition is satisfied. Returning 0 produces a false
negative.

Return "pred->op == OP_NE" instead of 0 when cpu >= nr_cpu_ids.

Fixes: 3cbec9d7b93d ("tracing/filters: Enable filtering a scalar field by a cpumask")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_events_filter.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 69da95f42541..d9aeeec4ed91 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -762,7 +762,7 @@ static int filter_pred_##size##_cpumask(struct filter_pred *pred, void *event)	\
 	unsigned int cpu = *addr;						\
 										\
 	if (cpu >= nr_cpu_ids)							\
-		return 0;							\
+		return pred->op == OP_NE;					\
 										\
 	return do_filter_scalar_cpumask(pred->op, cpu, pred->mask);		\
 }

[PATCH 05/10] tracing/filters: Report parse error on all -EINVAL paths in filter parsing

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Several error paths in filter parsing return -EINVAL (or -1) without
calling parse_error() to set pe->lasterr:

1. In parse_pred(), when an invalid token is encountered where a field name
   is expected (e.g., "common_pid == 1 || (common_pid == 2 && )"), len is 0
   and it returns -1 without calling parse_error().
2. In process_preds(), when !nr_preds (e.g., empty or whitespace-only
   filter), it returns -EINVAL before predicate_parse() can set
   FILT_ERR_NO_FILTER.
3. In predicate_parse(), when op_stack overflows or the jump target check
   fails, it returns -EINVAL without calling parse_error().

Because pe->lasterr remains 0 on these paths, append_filter_err() falls
back to printing an uninformative "Error: (0)" instead of reporting the
diagnostic and error position.

Call parse_error() with the appropriate error code on all of these paths.

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>
---
 kernel/trace/trace_events_filter.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index d9aeeec4ed91..7e119c7164f0 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -515,6 +515,8 @@ predicate_parse(const char *str, int nr_parens, int nr_preds,
 		switch (*next) {
 		case '(':					/* #2 */
 			if (top - op_stack > nr_parens) {
+				parse_error(pe, FILT_ERR_TOO_MANY_OPEN,
+					    next - str);
 				ret = -EINVAL;
 				goto out_free;
 			}
@@ -638,6 +640,7 @@ predicate_parse(const char *str, int nr_parens, int nr_preds,
 		prog[i].when_to_branch = invert;
 		/* Make sure the program always moves forward */
 		if (WARN_ON(prog[i].target <= i)) {
+			parse_error(pe, FILT_ERR_INVALID_FILTER, 0);
 			ret = -EINVAL;
 			goto out_free;
 		}
@@ -1688,8 +1691,10 @@ static int parse_pred(const char *str, void *data,
 
 	len = i - s;
 
-	if (!len)
-		return -1;
+	if (!len) {
+		parse_error(pe, FILT_ERR_INVALID_FILTER, pos + i);
+		return -EINVAL;
+	}
 
 	field_name = kmemdup_nul(str + s, len, GFP_KERNEL);
 	if (!field_name)
@@ -2277,8 +2282,10 @@ static int process_preds(struct trace_event_call *call,
 		return ret;
 	}
 
-	if (!nr_preds)
+	if (!nr_preds) {
+		parse_error(pe, FILT_ERR_NO_FILTER, 0);
 		return -EINVAL;
+	}
 
 	prog = predicate_parse(filter_string, nr_parens, nr_preds,
 			       parse_pred, call, pe);

[PATCH 06/10] tracing/filters: Fix function event operand parsing in parse_pred()

[Masami Hiramatsu (Google)]

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.

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.

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.

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

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>
---
 kernel/trace/trace_events_filter.c |   37 ++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 7e119c7164f0..96eac7ccd488 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1833,22 +1833,39 @@ static int parse_pred(const char *str, void *data,
 		 * Quotes are not required, but if they exist then we need
 		 * to read them till we hit a matching one.
 		 */
-		if (str[i] == '\'' || str[i] == '"')
-			q = str[i];
-		else
+		if (str[i] == '\'' || str[i] == '"') {
+			q = str[i++];
+			s = i;
+		} else {
 			q = 0;
+		}
 
-		for (i++; str[i]; i++) {
+		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);
+			goto err_free;
 		}
-		/* Skip quotes */
-		if (q)
-			s++;
 		len = i - s;
+		if (q)
+			i++;
+		if (!len) {
+			parse_error(pe, FILT_ERR_INVALID_VALUE, pos + i);
+			goto err_free;
+		}
 		if (len >= MAX_FILTER_STR_VAL) {
 			parse_error(pe, FILT_ERR_OPERAND_TOO_LONG, pos + i);
 			goto err_free;

[PATCH 07/10] tracing/filters: Skip second character of '&&' and '||' in calc_stack()

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

In calc_stack(), when encountering '||' or '&&' (where str[i+1] == str[i]),
the loop sets 'is_pred = false' and continues without advancing 'i' past
the second character of the operator. On the next iteration, the second '|'
or '&' sees str[i+1] != str[i], breaks out of the switch statement, and is
counted as the start of a new predicate (incrementing nr_preds and setting
is_pred = true).

Consequently, a trailing logical operator (e.g. 'common_pid == 1 ||') is
counted as having 2 predicates instead of 1, and any subsequent non-space
character before a delimiter fails to increment nr_preds.

Increment 'i' when matching '||' or '&&' so the entire two-character
operator is skipped.

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>
---
 kernel/trace/trace_events_filter.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 96eac7ccd488..e734d8664444 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2207,6 +2207,7 @@ static int calc_stack(const char *str, int *parens, int *preds, int *err)
 		case '&':
 			if (str[i+1] != str[i])
 				break;
+			i++;
 			is_pred = false;
 			continue;
 		case '(':

[PATCH 08/10] tracing/filters: Reject unsupported field sizes in parse_pred()

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Prior to commit 80765597bc58 ("tracing: Rewrite filter logic to be simpler
and faster"), parse_pred() checked whether select_comparison_fn() returned
NULL and failed with FILT_ERR_INVALID_OP and -EINVAL. That check was lost
during the rewrite, and commit 3cbec9d7b93d ("tracing/filters: Enable
filtering a scalar field by a cpumask") similarly omitted a default error
case when matching field->size for CPUS{} comparisons.

If a non-string field (FILTER_OTHER) has a size other than 1, 2, 4, or 8
(such as a fixed-size integer array field), pred->fn_num remains
FILTER_PRED_FN_NOP (0) and parse_pred() returns success. Subsequently,
update_preds() rejects FILTER_PRED_FN_NOP via WARN_ON_ONCE() and returns
-EINVAL without setting a parse_error() message, triggering a kernel
warning from user-supplied filter input.

Restore the check for FILTER_PRED_FN_NOP in both the numeric scalar and
CPUS{} branches of parse_pred() and report FILT_ERR_INVALID_OP.

Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
Fixes: 3cbec9d7b93d ("tracing/filters: Enable filtering a scalar field by a cpumask")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_events_filter.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index e734d8664444..89f4e119628f 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1995,6 +1995,10 @@ static int parse_pred(const char *str, void *data,
 				break;
 			}
 		}
+		if (pred->fn_num == FILTER_PRED_FN_NOP) {
+			parse_error(pe, FILT_ERR_INVALID_OP, pos + i);
+			goto err_free;
+		}
 
 	/* This is either a string, or an integer */
 	} else if (str[i] == '\'' || str[i] == '"') {
@@ -2140,6 +2144,10 @@ static int parse_pred(const char *str, void *data,
 		else {
 			pred->fn_num = select_comparison_fn(pred->op, field->size,
 							    field->is_signed);
+			if (pred->fn_num == FILTER_PRED_FN_NOP) {
+				parse_error(pe, FILT_ERR_INVALID_OP, pos + i);
+				goto err_free;
+			}
 			if (pred->op == OP_NE)
 				pred->not = 1;
 		}

[PATCH 09/10] tracing/filters: Terminate filter string at first newline

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Each trace event or subsystem only holds a single filter, and
append_filter_err() / tracing_log_err() caret placement assumes a
single-line filter string. However, apply_event_filter() and
apply_subsystem_event_filter() only call strstrip(), which strips
trailing newlines but leaves embedded newlines intact.

An embedded newline in a filter string (for example inside 'CPUS{0\n"}')
causes cpulist_parse() / bitmap_parselist() to treat '\n' as the end of
the cpulist and ignore trailing characters before '}', desynchronizing
quote tracking between calc_stack() and parse_pred() and misaligning
error_log caret reporting.

Terminate filter_string at the first '\n' in apply_event_filter(),
apply_subsystem_event_filter(), and create_filter_start() so only the
first line is parsed.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_events_filter.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 89f4e119628f..213ed9bdf6f3 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2425,10 +2425,15 @@ static int create_filter_start(char *filter_string, bool set_str,
 	struct event_filter *filter;
 	struct filter_parse_error *pe = NULL;
 	int err = 0;
+	char *p;
 
 	if (WARN_ON_ONCE(*pse || *filterp))
 		return -EINVAL;
 
+	p = strchr(filter_string, '\n');
+	if (p)
+		*p = '\0';
+
 	filter = kzalloc_obj(*filter);
 	if (filter && set_str) {
 		filter->filter_string = kstrdup(filter_string, GFP_KERNEL);
@@ -2542,11 +2547,16 @@ int apply_event_filter(struct trace_event_file *file, char *filter_string)
 {
 	struct trace_event_call *call = file->event_call;
 	struct event_filter *filter = NULL;
+	char *p;
 	int err;
 
 	if (file->flags & EVENT_FILE_FL_FREED)
 		return -ENODEV;
 
+	p = strchr(filter_string, '\n');
+	if (p)
+		*p = '\0';
+
 	if (!strcmp(strstrip(filter_string), "0")) {
 		filter_disable(file);
 		filter = event_filter(file);
@@ -2594,6 +2604,7 @@ int apply_subsystem_event_filter(struct trace_subsystem_dir *dir,
 	struct trace_array *tr = dir->tr;
 	struct event_filter *filter = NULL;
 	int err = 0;
+	char *p;
 
 	guard(mutex)(&event_mutex);
 
@@ -2601,6 +2612,10 @@ int apply_subsystem_event_filter(struct trace_subsystem_dir *dir,
 	if (!dir->nr_events)
 		return -ENODEV;
 
+	p = strchr(filter_string, '\n');
+	if (p)
+		*p = '\0';
+
 	if (!strcmp(strstrip(filter_string), "0")) {
 		filter_free_subsystem_preds(dir, tr);
 		remove_filter_string(system->filter);

[PATCH 10/10] selftests/ftrace: Add event filter syntax check test case

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Add a new ftracetest test case (filter-syntax-check.tc) to verify valid
event filter expressions, error_log caret reporting for malformed filter
expressions (empty filters, missing fields, unbalanced parentheses and
quotes, dangling logical operators, invalid fields/values/cpulists),
newline truncation on multi-line filter input, and error returns on
invalid subsystem filters.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 .../ftrace/test.d/filter/filter-syntax-check.tc    |   52 ++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc

diff --git a/tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc b/tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc
new file mode 100644
index 000000000000..68c8accc5ffd
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc
@@ -0,0 +1,52 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: event filter - test filter syntax error check
+# requires: set_event events/sched/sched_switch/filter error_log
+# flags: instance
+
+check_error() { # command-with-error-pos-by-^
+    ftrace_errlog_check 'event filter parse error' "$1" 'events/sched/sched_switch/filter'
+}
+
+# Valid filter expressions
+echo '(common_pid == 0)' > events/sched/sched_switch/filter
+echo '((common_pid == 0) && (prev_pid == 0))' > events/sched/sched_switch/filter
+echo 'prev_comm ~ "*sh"' > events/sched/sched_switch/filter
+echo 'common_pid == CPUS{0}' > events/sched/sched_switch/filter
+echo 'common_pid != CPUS{0}' > events/sched/sched_switch/filter
+echo 0 > events/sched/sched_switch/filter
+
+# Empty or meaningless filter expressions
+check_error '^()'						# NO_FILTER
+check_error '=^= 1'						# INVALID_FILTER
+
+# Unbalanced parentheses and quotes
+check_error '^(common_pid == 1'					# TOO_MANY_OPEN
+check_error 'common_pid == 1^)'					# TOO_MANY_CLOSE
+check_error 'prev_comm == ^"bash'				# MISSING_QUOTE
+
+# Dangling logical operators
+check_error 'common_pid == 1 &&^'				# TOO_MANY_PREDS
+check_error 'common_pid == 1 ||^'				# TOO_MANY_PREDS
+check_error 'common_pid == 1 || (common_pid == 2 && )^'		# INVALID_FILTER
+
+# Invalid field, operator, or operand value
+check_error 'no_such_field ^== 1'				# FIELD_NOT_FOUND
+check_error 'common_pid == @^'					# INVALID_VALUE
+check_error 'common_pid == CPUS0}^'				# MISSING_BRACE_OPEN
+check_error 'common_pid == CPUS{}^'				# INVALID_CPULIST
+
+# Multi-line input must be truncated at the first newline
+echo > error_log
+! ( printf "common_pid == CPUS{0\n\"} && common_pid == 0 && common_pid == CPUS{0\n\"}" | \
+    dd of=events/sched/sched_switch/filter bs=4096 ) 2> /dev/null
+grep -q "Missing '}'" error_log
+! grep -q "Missing matching quote" error_log
+
+# Subsystem filter must fail on invalid filter expressions
+echo 'common_pid >= 0' > events/sched/filter
+! echo 'no_such_field == 1' > events/sched/filter
+! echo 'common_pid == 1 &&' > events/sched/filter
+echo 0 > events/sched/filter
+
+exit 0

Re: [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tracing/filters: out-of-bounds read in regex_match_end() when len is 0
--

commit 1023475f1fd5bcb5e8b46a2be2fb9fe7ea16d624
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings

This commit updates regex_match_end() to compute string length via strnlen()
when len is non-zero and strlen() when len is zero, fixing suffix filter
matching on fixed-size and dynamic string fields.

> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 5e74d2dae40a3..18ce3ec00d34f 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1075,10 +1075,10 @@ static int regex_match_middle(char *str, struct regex *r, int len)
>  
>  static int regex_match_end(char *str, struct regex *r, int len)
>  {
> -	int strlen = len - 1;
> +	int slen = len ? strnlen(str, len) : strlen(str);

[Severity: High]
Can calling strlen(str) when len == 0 cause an out-of-bounds read past the
end of the trace event record?

While filter_pred_string() passes pred->regex->field_len == 0 for
NUL-terminated dynamic strings, regex_match_end() is also called by
filter_pred_strloc() and filter_pred_strrelloc(), where len is the runtime
byte length extracted from the locator:

kernel/trace/trace_events_filter.c:filter_pred_strloc() {
	u32 str_item = *(u32 *)(event + pred->offset);
	int str_loc = str_item & 0xffff;
	int str_len = str_item >> 16;
	char *addr = (char *)(event + str_loc);
	int cmp, match;

	cmp = pred->regex->match(addr, pred->regex, str_len);
}

When a dynamic char array has a runtime length of 0, str_len is 0 and addr
points to the end of the event or uninitialized trailing memory without a
NUL terminator.

For example, in fs/xfs/xfs_trace.h, xfs_da_class defines a dynamic char array
that isclassified as FILTER_DYN_STRING, and when xfs_dir_init() ->
xfs_dir2_sf_create() runs with args->namelen == 0, 0 bytes are reserved and
nothing is written:

	__dynamic_array(char, name, args->namelen)
...
	TP_fast_assign(
		__entry->dev = VFS_I(args->dp)->i_sb->s_dev;
		__entry->ino = I_INO(args->dp);
		if (args->namelen)
			memcpy(__get_str(name), args->name, args->namelen);

Similarly, in kernel/trace/trace_events_user.c, user_event_validate() allows
a user_events string locator with size == 0 and pos == end as long as the
preceding byte *(char *)(end - 1) is '\0':

		pos += size;

		if (unlikely(pos > end))
			return -EFAULT;

		if (likely(validator->flags & VALIDATOR_ENSURE_NULL))
			if (unlikely(*(char *)(pos - 1) != '\0'))
				return -EFAULT;

In both cases, evaluating a MATCH_END_ONLY filter calls regex_match_end()
with len == 0 and str pointing at or past the end of the event buffer, where
strlen(str) and memcmp() will read out of bounds.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179285912318.466249.7425922682086130057.stgit@devnote2?part=1

Re: [PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tracing/filters: unreachable CPU bounds check in do_filter_cpumask_scalar()
--

commit 3eff3e01752bc54a2dfd5503608817b42e7186d2
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar()

This commit adds a bounds check comparing cpu against nr_cpu_ids at the
beginning of do_filter_cpumask_scalar() to avoid out-of-bounds bit tests
when evaluating single-CPU cpumask predicates.

> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index c2c35b4de85e0..69da95f425410 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -695,6 +695,9 @@ do_filter_scalar_cpumask(int op, unsigned int cpu, const struct cpumask *mask)
>  static inline int
>  do_filter_cpumask_scalar(int op, const struct cpumask *mask, unsigned int cpu)
>  {
> +	if (cpu >= nr_cpu_ids)
> +		return op == OP_NE;

[Severity: Medium]
Can cpu >= nr_cpu_ids ever be true in do_filter_cpumask_scalar()?

The commit description states that cpulist_parse() checks against
nr_cpumask_bits (NR_CPUS). However, since commit aa47a7c215e7 ("lib/cpumask:
deprecate nr_cpumask_bits"), include/linux/cpumask.h unconditionally defines
nr_cpumask_bits as nr_cpu_ids:

    #define nr_cpumask_bits nr_cpu_ids

    static __always_inline int cpulist_parse(const char *buf, struct cpumask *dstp)
    {
        return bitmap_parselist(buf, cpumask_bits(dstp), nr_cpumask_bits);
    }

When parse_pred() in kernel/trace/trace_events_filter.c parses a cpumask
filter via cpulist_parse()->bitmap_parselist()->bitmap_check_region(), any
CPU number >= nr_cpu_ids is rejected with -ERANGE:

    if (cpulist_parse(tmp, pred->mask)) {
        kfree(tmp);
        parse_error(pe, FILT_ERR_INVALID_CPULIST, pos + i);
        goto err_free;
    }
    ...
    single = cpumask_weight(pred->mask) == 1;
    if (single) {
        pred->val = cpumask_first(pred->mask);
        kfree(pred->mask);
        pred->mask = NULL;
    }

Since filter_pred_cpumask_cpu() is the only caller of
do_filter_cpumask_scalar() and passes pred->val as cpu, isn't cpu guaranteed
to be less than nr_cpu_ids here, making this check unreachable?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179285912318.466249.7425922682086130057.stgit@devnote2?part=3

Re: [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings

[Masami Hiramatsu]

On Sat, 24 Oct 2026 17:02:09 +0000, sashiko-bot@kernel.org <sashiko-bot@kernel.org> wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] tracing/filters: out-of-bounds read in regex_match_end() when len is 0
> --
> 
> commit 1023475f1fd5bcb5e8b46a2be2fb9fe7ea16d624
> Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings
> 
> This commit updates regex_match_end() to compute string length via strnlen()
> when len is non-zero and strlen() when len is zero, fixing suffix filter
> matching on fixed-size and dynamic string fields.
> 
> > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> > index 5e74d2dae40a3..18ce3ec00d34f 100644
> > --- a/kernel/trace/trace_events_filter.c
> > +++ b/kernel/trace/trace_events_filter.c
> > @@ -1075,10 +1075,10 @@ static int regex_match_middle(char *str, struct regex *r, int len)
> >  
> >  static int regex_match_end(char *str, struct regex *r, int len)
> >  {
> > -	int strlen = len - 1;
> > +	int slen = len ? strnlen(str, len) : strlen(str);
> 
> [Severity: High]
> Can calling strlen(str) when len == 0 cause an out-of-bounds read past the
> end of the trace event record?

Ah, good catch! Indeed. len == 0 means it is 0. Ok, let's fix it.
(maybe we should also reject len < 0 for safety?)

Thanks,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs

[Steven Rostedt]

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

> Hi,
> 
> Here is a series of fixes for various bugs in the trace event filter
> parser and predicate evaluation in kernel/trace/trace_events_filter.c,
> along with a new ftracetest test case covering filter syntax checks.
> 

FYI, even though the Assisted-by tag is considered obsolete, any AI usage
should still be stated in the change log.

-- Steve

P.S. I'm 100% *against* the removal of that tag :-p

(but I wasn't at the Maintainers Summit to argue my case :-( )

Re: [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings

[Steven Rostedt]

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

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> regex_match_end() calculated string length as "len - 1", assuming @len is
> the string length including trailing '\0'. However, for fixed-size string
> fields, filter_pred_string() passes field->size as @len, and for "comm",
> filter_pred_comm() passes TASK_COMM_LEN as @len.
> 
> Because @len in these cases represents the buffer capacity rather than the
> actual length of the string, regex_match_end() tests against the tail of
> the buffer (typically trailing '\0' padding) rather than the end of the
> string. Furthermore, if @len is 0 (denoting a dynamic string ending with
> '\0'), "len - 1" underflows to -1 and never matches.
> 
> Compute the actual string length using strnlen(str, len) when @len is
> non-zero, and strlen(str) when @len is zero.
> 
> Fixes: a3291c14ecf0 ("tracing/filters: Fix MATCH_END_ONLY filter matching")
> Cc: stable@vger.kernel.org
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  kernel/trace/trace_events_filter.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 5e74d2dae40a..18ce3ec00d34 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1075,10 +1075,10 @@ static int regex_match_middle(char *str, struct regex *r, int len)
>  
>  static int regex_match_end(char *str, struct regex *r, int len)
>  {
> -	int strlen = len - 1;
> +	int slen = len ? strnlen(str, len) : strlen(str);
>  
> -	if (strlen >= r->len &&
> -	    memcmp(str + strlen - r->len, r->pattern, r->len) == 0)
> +	if (slen >= r->len &&
> +	    memcmp(str + slen - r->len, r->pattern, r->len) == 0)
>  		return 1;

This slows down the code as it adds more strlen() calculations (it could
have already been done.

Let's fix the callers instead and have them pass in the real length.

-- Steve


>  	return 0;
>  }

Re: [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings

[Masami Hiramatsu (Google)]

On Mon, 26 Oct 2026 09:54:41 -0400, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sun, 25 Oct 2026 01:25:33 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > regex_match_end() calculated string length as "len - 1", assuming @len is
> > the string length including trailing '\0'. However, for fixed-size string
> > fields, filter_pred_string() passes field->size as @len, and for "comm",
> > filter_pred_comm() passes TASK_COMM_LEN as @len.
> > 
> > Because @len in these cases represents the buffer capacity rather than the
> > actual length of the string, regex_match_end() tests against the tail of
> > the buffer (typically trailing '\0' padding) rather than the end of the
> > string. Furthermore, if @len is 0 (denoting a dynamic string ending with
> > '\0'), "len - 1" underflows to -1 and never matches.
> > 
> > Compute the actual string length using strnlen(str, len) when @len is
> > non-zero, and strlen(str) when @len is zero.
> > 
> > Fixes: a3291c14ecf0 ("tracing/filters: Fix MATCH_END_ONLY filter matching")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  kernel/trace/trace_events_filter.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> > index 5e74d2dae40a..18ce3ec00d34 100644
> > --- a/kernel/trace/trace_events_filter.c
> > +++ b/kernel/trace/trace_events_filter.c
> > @@ -1075,10 +1075,10 @@ static int regex_match_middle(char *str, struct regex *r, int len)
> >  
> >  static int regex_match_end(char *str, struct regex *r, int len)
> >  {
> > -	int strlen = len - 1;
> > +	int slen = len ? strnlen(str, len) : strlen(str);
> >  
> > -	if (strlen >= r->len &&
> > -	    memcmp(str + strlen - r->len, r->pattern, r->len) == 0)
> > +	if (slen >= r->len &&
> > +	    memcmp(str + slen - r->len, r->pattern, r->len) == 0)
> >  		return 1;
> 
> This slows down the code as it adds more strlen() calculations (it could
> have already been done.

I sent v2 already, which is slightly lighter (but still using strnlen)

https://lore.kernel.org/all/179291743946.492959.9624495647021680807.stgit@devnote2/

(anyway, 3/10 is already fixed as Sashiko mentioned.)

> 
> Let's fix the callers instead and have them pass in the real length.

OK, let me check it.

Thanks,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 02/10] tracing/filters: Fix false success return in process_system_preds()

[Steven Rostedt]

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

> 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>
> ---
>  kernel/trace/trace_events_filter.c |   15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 18ce3ec00d34..c2c35b4de85e 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -2332,8 +2332,10 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
>  			filter_disable(file);
>  			parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
>  			append_filter_err(tr, pe, filter);
> -		} else
> +		} else {
>  			event_set_filtered_flag(file);
> +			fail = false;
> +		}
>  
>  
>  		filter_item = kzalloc_obj(*filter_item);
> @@ -2348,8 +2350,6 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
>  		filter_item->filter = event_filter(file);
>  		event_set_filter(file, filter);
>  		filter = NULL;
> -
> -		fail = false;
>  	}
>  
>  	if (fail)
> @@ -2364,14 +2364,17 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
>  	return 0;
>   fail:
>  	/* No call succeeded */
> -	free_filter_list(filter_list);
> +	if (!list_empty(&filter_list->list))
> +		delay_free_filter(filter_list);
> +	else
> +		free_filter_list(filter_list);
>  	parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
>  	return -EINVAL;
>   fail_mem:
>  	__free_filter(filter);
>  
> -	/* If any call succeeded, we still need to sync */
> -	if (!fail)
> +	/* If any filter was replaced, we still need to sync */
> +	if (!list_empty(&filter_list->list))
>  		delay_free_filter(filter_list);
>  	else
>  		free_filter_list(filter_list);

Hmm, I think this can be cleaned up a bit better.

 1) remove the fail boolean
 2) consolidate the if (!list_empty()) logic.

Like this:

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 5e74d2dae40a..4d763c55bb7b 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2305,7 +2305,7 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
 	struct filter_list *filter_item;
 	struct event_filter *filter = NULL;
 	struct filter_head *filter_list;
-	bool fail = true;
+	int ret = -ENOMEM;
 	int err;
 
 	filter_list = kmalloc_obj(*filter_list);
@@ -2332,8 +2332,10 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
 			filter_disable(file);
 			parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
 			append_filter_err(tr, pe, filter);
-		} else
+		} else {
 			event_set_filtered_flag(file);
+			ret = 0;
+		}
 
 
 		filter_item = kzalloc_obj(*filter_item);
@@ -2348,11 +2350,9 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
 		filter_item->filter = event_filter(file);
 		event_set_filter(file, filter);
 		filter = NULL;
-
-		fail = false;
 	}
 
-	if (fail)
+	if (ret)
 		goto fail;
 
 	/*
@@ -2364,19 +2364,18 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
 	return 0;
  fail:
 	/* No call succeeded */
-	free_filter_list(filter_list);
 	parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
-	return -EINVAL;
+	ret = -EINVAL;
  fail_mem:
 	__free_filter(filter);
 
-	/* If any call succeeded, we still need to sync */
-	if (!fail)
+	/* If any filter was replaced, we still need to sync */
+	if (!list_empty(&filter_list->list))
 		delay_free_filter(filter_list);
 	else
 		free_filter_list(filter_list);
 
-	return -ENOMEM;
+	return ret;
 }
 
 static int create_filter_start(char *filter_string, bool set_str,


-- Steve

Re: [PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar()

[Steven Rostedt]

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

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> In parse_pred(), single-CPU cpumask filters are parsed via cpulist_parse(),
> which checks against nr_cpumask_bits (NR_CPUS). When NR_CPUS > nr_cpu_ids,
> a user can provide a CPU number where cpu >= nr_cpu_ids.
> 
> When evaluating such a predicate in filter_pred_cpumask_cpu(),
> do_filter_cpumask_scalar() calls cpumask_test_cpu(cpu, mask) without
> checking cpu against nr_cpu_ids. This triggers a WARN_ON_ONCE under
> CONFIG_DEBUG_PER_CPU_MAPS and causes an out-of-bounds bit test.
> 
> Check "cpu >= nr_cpu_ids" at the start of do_filter_cpumask_scalar(). When
> the CPU index is out of bounds, return "op == OP_NE" since an out-of-range
> CPU is never present in a valid cpumask.
> 
> Fixes: 38c6f6808389 ("tracing/filters: Further optimise scalar vs cpumask comparison")
> Cc: stable@vger.kernel.org
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  kernel/trace/trace_events_filter.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index c2c35b4de85e..69da95f42541 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -695,6 +695,9 @@ do_filter_scalar_cpumask(int op, unsigned int cpu, const struct cpumask *mask)
>  static inline int
>  do_filter_cpumask_scalar(int op, const struct cpumask *mask, unsigned int cpu)
>  {
> +	if (cpu >= nr_cpu_ids)
> +		return op == OP_NE;
> +
>  	switch (op) {
>  	case OP_EQ:
>  		return cpumask_test_cpu(cpu, mask) &&

I wonder if we should just make the filter fail with -EINVAL, and not let
it get this far.

-- Steve

Re: [PATCH 04/10] tracing/filters: Handle out-of-range CPU for OP_NE in cpumask comparison

[Steven Rostedt]

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

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> In filter_pred_##size##_cpumask(), if the event's scalar CPU value is
> out of range (cpu >= nr_cpu_ids), it unconditionally returns 0.
> 
> However, if the predicate operation is OP_NE (e.g., field != CPUS{0,1}),
> an out-of-range CPU cannot match any valid CPU in the user-provided
> cpumask and the condition is satisfied. Returning 0 produces a false
> negative.
> 
> Return "pred->op == OP_NE" instead of 0 when cpu >= nr_cpu_ids.
> 
> Fixes: 3cbec9d7b93d ("tracing/filters: Enable filtering a scalar field by a cpumask")
> Cc: stable@vger.kernel.org

BTW, a lot of these are just "stupid input produces stupid results" and I
personally feel it doesn't deserve a stable tag. I don't even think it
deserves the "fixes" tag.

Now if the stupid input were to produce a crash or warning, then that's
different.

-- Steve


> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  kernel/trace/trace_events_filter.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 69da95f42541..d9aeeec4ed91 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -762,7 +762,7 @@ static int filter_pred_##size##_cpumask(struct filter_pred *pred, void *event)	\
>  	unsigned int cpu = *addr;						\
>  										\
>  	if (cpu >= nr_cpu_ids)							\
> -		return 0;							\
> +		return pred->op == OP_NE;					\
>  										\
>  	return do_filter_scalar_cpumask(pred->op, cpu, pred->mask);		\
>  }

Re: [PATCH 05/10] tracing/filters: Report parse error on all -EINVAL paths in filter parsing

[Steven Rostedt]

On Sun, 25 Oct 2026 01:26:10 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Several error paths in filter parsing return -EINVAL (or -1) without
> calling parse_error() to set pe->lasterr:
> 
> 1. In parse_pred(), when an invalid token is encountered where a field name
>    is expected (e.g., "common_pid == 1 || (common_pid == 2 && )"), len is 0
>    and it returns -1 without calling parse_error().
> 2. In process_preds(), when !nr_preds (e.g., empty or whitespace-only
>    filter), it returns -EINVAL before predicate_parse() can set
>    FILT_ERR_NO_FILTER.
> 3. In predicate_parse(), when op_stack overflows or the jump target check
>    fails, it returns -EINVAL without calling parse_error().
> 
> Because pe->lasterr remains 0 on these paths, append_filter_err() falls
> back to printing an uninformative "Error: (0)" instead of reporting the
> diagnostic and error position.
> 
> Call parse_error() with the appropriate error code on all of these paths.
> 
> 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>

This is more of a cleanup than fixes. You can keep the "fixes" tag but
remove the stable. It's not that important to backport. Sure, the stable
folks may backport it anyway, but I use the stable tag for what I feel is
important to backport.

-- Steve