[PATCH v2 0/9] tracing/filters: Fix event filter parsing and matching bugs

[PATCH v2 0/9] tracing/filters: Fix event filter parsing and matching bugs

[Masami Hiramatsu (Google)]

Hi,

Here is the second version 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.

The previous version is here:

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

Changes in v2:
[01/09]
 - Guard against negative length and avoid calling strnlen() when the
   buffer capacity is smaller than the search pattern.
 - Use strnlen(str, len) directly instead of calling strlen(str) when
   len is 0, avoiding an out-of-bounds read on 0-length dynamic arrays.
[03/10] (in v1)
 - Dropped because out-of-range CPU numbers are already rejected at
   filter parse time by cpulist_parse() checking against nr_cpu_ids,
   making the runtime check in do_filter_cpumask_scalar() unreachable.

Summary of the patches:

- [01/09] Compute the actual string length with strnlen() in
  regex_match_end() so suffix matching ("*pattern") works on fixed-size
  string fields (such as comm) and dynamic strings.
- [02/09] 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/09] 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.
- [04/09] 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)").
- [05/09] 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.
- [06/09] Advance past the second character of "&&" and "||" in
  calc_stack() so the second operator character is not miscounted as a
  new predicate.
- [07/09] 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().
- [08/09] 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. (as an improvement)
- [09/09] 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.

Notes:

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

- Also, note that [08/09] 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) (9):
      tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings
      tracing/filters: Fix false success return in process_system_preds()
      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                 |  101 +++++++++++++++-----
 .../ftrace/test.d/filter/filter-syntax-check.tc    |   52 ++++++++++
 2 files changed, 130 insertions(+), 23 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc

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

[PATCH v2 1/9] 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 an empty dynamic array without
trailing '\0'), "len - 1" underflows to -1.

First check "len < r->len" and return 0, which safely rejects negative
lengths and cases where the buffer capacity is smaller than the pattern.
Then compute the actual string length using strnlen(str, len), which
bounds the scan by @len (stopping at the first '\0') without dereferencing
past the buffer.

Fixes: a3291c14ecf0 ("tracing/filters: Fix MATCH_END_ONLY filter matching")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v2:
 - Check "len < r->len" to guard against negative lengths and avoid
   calling strnlen() when the buffer capacity is smaller than the pattern.
 - Use strnlen(str, len) directly instead of calling strlen(str) when
   len is 0, avoiding an out-of-bounds read on 0-length dynamic arrays.
---
 kernel/trace/trace_events_filter.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 5e74d2dae40a..fac51a2953dd 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1075,10 +1075,14 @@ 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;
 
-	if (strlen >= r->len &&
-	    memcmp(str + strlen - r->len, r->pattern, r->len) == 0)
+	if (len < r->len)
+		return 0;
+
+	slen = strnlen(str, len);
+	if (slen >= r->len &&
+	    memcmp(str + slen - r->len, r->pattern, r->len) == 0)
 		return 1;
 	return 0;
 }

[PATCH v2 2/9] 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 fac51a2953dd..54144ef9d0db 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2336,8 +2336,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);
@@ -2352,8 +2354,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)
@@ -2368,14 +2368,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 v2 3/9] 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 54144ef9d0db..44ca7772ed2c 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -759,7 +759,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 v2 4/9] 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 44ca7772ed2c..cc3a53e7cdbd 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;
 		}
@@ -1689,8 +1692,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)
@@ -2278,8 +2283,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 v2 5/9] 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 cc3a53e7cdbd..3bd0b47a627f 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1834,22 +1834,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 v2 6/9] 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 3bd0b47a627f..bcd37fd6c109 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2208,6 +2208,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 v2 7/9] 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 bcd37fd6c109..8d7fb6e9d7f8 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1996,6 +1996,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] == '"') {
@@ -2141,6 +2145,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 v2 8/9] 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 8d7fb6e9d7f8..14afcca7a444 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2426,10 +2426,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);
@@ -2543,11 +2548,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);
@@ -2595,6 +2605,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);
 
@@ -2602,6 +2613,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 v2 9/9] 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