* [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
@ 2026-10-24 16:25 ` Masami Hiramatsu (Google)
2026-10-24 17:02 ` sashiko-bot
2026-10-26 13:54 ` Steven Rostedt
2026-10-24 16:25 ` [PATCH 02/10] tracing/filters: Fix false success return in process_system_preds() Masami Hiramatsu (Google)
` (9 subsequent siblings)
10 siblings, 2 replies; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-24 16:25 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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;
}
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings
2026-10-24 16:25 ` [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings Masami Hiramatsu (Google)
@ 2026-10-24 17:02 ` sashiko-bot
2026-10-25 2:11 ` Masami Hiramatsu
2026-10-26 13:54 ` Steven Rostedt
1 sibling, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-10-24 17:02 UTC (permalink / raw)
To: Masami Hiramatsu (Google); +Cc: linux-trace-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings
2026-10-24 17:02 ` sashiko-bot
@ 2026-10-25 2:11 ` Masami Hiramatsu
0 siblings, 0 replies; 21+ messages in thread
From: Masami Hiramatsu @ 2026-10-25 2:11 UTC (permalink / raw)
To: sashiko-bot; +Cc: linux-trace-kernel
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>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings
2026-10-24 16:25 ` [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings Masami Hiramatsu (Google)
2026-10-24 17:02 ` sashiko-bot
@ 2026-10-26 13:54 ` Steven Rostedt
2026-10-26 15:37 ` Masami Hiramatsu (Google)
1 sibling, 1 reply; 21+ messages in thread
From: Steven Rostedt @ 2026-10-26 13:54 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Shuah Khan, Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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;
> }
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings
2026-10-26 13:54 ` Steven Rostedt
@ 2026-10-26 15:37 ` Masami Hiramatsu (Google)
0 siblings, 0 replies; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-26 15:37 UTC (permalink / raw)
To: Steven Rostedt
Cc: Shuah Khan, Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 02/10] tracing/filters: Fix false success return in process_system_preds()
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
2026-10-24 16:25 ` [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings Masami Hiramatsu (Google)
@ 2026-10-24 16:25 ` Masami Hiramatsu (Google)
2026-10-26 19:33 ` Steven Rostedt
2026-10-24 16:25 ` [PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar() Masami Hiramatsu (Google)
` (8 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-24 16:25 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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);
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 02/10] tracing/filters: Fix false success return in process_system_preds()
2026-10-24 16:25 ` [PATCH 02/10] tracing/filters: Fix false success return in process_system_preds() Masami Hiramatsu (Google)
@ 2026-10-26 19:33 ` Steven Rostedt
0 siblings, 0 replies; 21+ messages in thread
From: Steven Rostedt @ 2026-10-26 19:33 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Shuah Khan, Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar()
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
2026-10-24 16:25 ` [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings Masami Hiramatsu (Google)
2026-10-24 16:25 ` [PATCH 02/10] tracing/filters: Fix false success return in process_system_preds() Masami Hiramatsu (Google)
@ 2026-10-24 16:25 ` Masami Hiramatsu (Google)
2026-10-24 17:04 ` sashiko-bot
2026-10-26 19:36 ` Steven Rostedt
2026-10-24 16:26 ` [PATCH 04/10] tracing/filters: Handle out-of-range CPU for OP_NE in cpumask comparison Masami Hiramatsu (Google)
` (7 subsequent siblings)
10 siblings, 2 replies; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-24 16:25 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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) &&
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar()
2026-10-24 16:25 ` [PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar() Masami Hiramatsu (Google)
@ 2026-10-24 17:04 ` sashiko-bot
2026-10-26 19:36 ` Steven Rostedt
1 sibling, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-10-24 17:04 UTC (permalink / raw)
To: Masami Hiramatsu (Google); +Cc: linux-trace-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar()
2026-10-24 16:25 ` [PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar() Masami Hiramatsu (Google)
2026-10-24 17:04 ` sashiko-bot
@ 2026-10-26 19:36 ` Steven Rostedt
1 sibling, 0 replies; 21+ messages in thread
From: Steven Rostedt @ 2026-10-26 19:36 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Shuah Khan, Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 04/10] tracing/filters: Handle out-of-range CPU for OP_NE in cpumask comparison
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
` (2 preceding siblings ...)
2026-10-24 16:25 ` [PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar() Masami Hiramatsu (Google)
@ 2026-10-24 16:26 ` Masami Hiramatsu (Google)
2026-10-26 19:38 ` Steven Rostedt
2026-10-24 16:26 ` [PATCH 05/10] tracing/filters: Report parse error on all -EINVAL paths in filter parsing Masami Hiramatsu (Google)
` (6 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-24 16:26 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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); \
}
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 04/10] tracing/filters: Handle out-of-range CPU for OP_NE in cpumask comparison
2026-10-24 16:26 ` [PATCH 04/10] tracing/filters: Handle out-of-range CPU for OP_NE in cpumask comparison Masami Hiramatsu (Google)
@ 2026-10-26 19:38 ` Steven Rostedt
0 siblings, 0 replies; 21+ messages in thread
From: Steven Rostedt @ 2026-10-26 19:38 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Shuah Khan, Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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); \
> }
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 05/10] tracing/filters: Report parse error on all -EINVAL paths in filter parsing
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
` (3 preceding siblings ...)
2026-10-24 16:26 ` [PATCH 04/10] tracing/filters: Handle out-of-range CPU for OP_NE in cpumask comparison Masami Hiramatsu (Google)
@ 2026-10-24 16:26 ` Masami Hiramatsu (Google)
2026-10-26 19:40 ` Steven Rostedt
2026-10-24 16:26 ` [PATCH 06/10] tracing/filters: Fix function event operand parsing in parse_pred() Masami Hiramatsu (Google)
` (5 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-24 16:26 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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);
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 05/10] tracing/filters: Report parse error on all -EINVAL paths in filter parsing
2026-10-24 16:26 ` [PATCH 05/10] tracing/filters: Report parse error on all -EINVAL paths in filter parsing Masami Hiramatsu (Google)
@ 2026-10-26 19:40 ` Steven Rostedt
0 siblings, 0 replies; 21+ messages in thread
From: Steven Rostedt @ 2026-10-26 19:40 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Shuah Khan, Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 06/10] tracing/filters: Fix function event operand parsing in parse_pred()
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
` (4 preceding siblings ...)
2026-10-24 16:26 ` [PATCH 05/10] tracing/filters: Report parse error on all -EINVAL paths in filter parsing Masami Hiramatsu (Google)
@ 2026-10-24 16:26 ` Masami Hiramatsu (Google)
2026-10-24 16:26 ` [PATCH 07/10] tracing/filters: Skip second character of '&&' and '||' in calc_stack() Masami Hiramatsu (Google)
` (4 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-24 16:26 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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;
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 07/10] tracing/filters: Skip second character of '&&' and '||' in calc_stack()
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
` (5 preceding siblings ...)
2026-10-24 16:26 ` [PATCH 06/10] tracing/filters: Fix function event operand parsing in parse_pred() Masami Hiramatsu (Google)
@ 2026-10-24 16:26 ` Masami Hiramatsu (Google)
2026-10-24 16:26 ` [PATCH 08/10] tracing/filters: Reject unsupported field sizes in parse_pred() Masami Hiramatsu (Google)
` (3 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-24 16:26 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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 '(':
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 08/10] tracing/filters: Reject unsupported field sizes in parse_pred()
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
` (6 preceding siblings ...)
2026-10-24 16:26 ` [PATCH 07/10] tracing/filters: Skip second character of '&&' and '||' in calc_stack() Masami Hiramatsu (Google)
@ 2026-10-24 16:26 ` Masami Hiramatsu (Google)
2026-10-24 16:26 ` [PATCH 09/10] tracing/filters: Terminate filter string at first newline Masami Hiramatsu (Google)
` (2 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-24 16:26 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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;
}
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 09/10] tracing/filters: Terminate filter string at first newline
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
` (7 preceding siblings ...)
2026-10-24 16:26 ` [PATCH 08/10] tracing/filters: Reject unsupported field sizes in parse_pred() Masami Hiramatsu (Google)
@ 2026-10-24 16:26 ` Masami Hiramatsu (Google)
2026-10-24 16:26 ` [PATCH 10/10] selftests/ftrace: Add event filter syntax check test case Masami Hiramatsu (Google)
2026-10-26 13:44 ` [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Steven Rostedt
10 siblings, 0 replies; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-24 16:26 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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);
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 10/10] selftests/ftrace: Add event filter syntax check test case
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
` (8 preceding siblings ...)
2026-10-24 16:26 ` [PATCH 09/10] tracing/filters: Terminate filter string at first newline Masami Hiramatsu (Google)
@ 2026-10-24 16:26 ` Masami Hiramatsu (Google)
2026-10-26 13:44 ` [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Steven Rostedt
10 siblings, 0 replies; 21+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-10-24 16:26 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs
2026-10-24 16:25 [PATCH 00/10] tracing/filters: Fix event filter parsing and matching bugs Masami Hiramatsu (Google)
` (9 preceding siblings ...)
2026-10-24 16:26 ` [PATCH 10/10] selftests/ftrace: Add event filter syntax check test case Masami Hiramatsu (Google)
@ 2026-10-26 13:44 ` Steven Rostedt
10 siblings, 0 replies; 21+ messages in thread
From: Steven Rostedt @ 2026-10-26 13:44 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Shuah Khan, Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
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 :-( )
^ permalink raw reply [flat|nested] 21+ messages in thread