* [PATCH RFC 1/7] tracing: Restore :mod: trailer after parsing in ftrace_set_clr_event()
2026-08-13 14:07 [PATCH RFC 0/7] tracing: Parse filter from event string Thomas Weißschuh
@ 2026-08-13 14:07 ` Thomas Weißschuh
2026-08-13 14:07 ` [PATCH RFC 2/7] tracing: Remove duplicate declaration of ftrace_set_clr_event() Thomas Weißschuh
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2026-08-13 14:07 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel, Thomas Weißschuh, stable
While ftrace_set_clr_event() modifies its input buffer during parsing,
before returning to the caller the buffer is supposed to be restored
to its original state.
This works correctly for the colon between the subsystem and event
but not the colon at the beginning of :mod:.
Restore the colon, so the :mod: trailer is not stripped after
ftrace_set_clr_event().
Fixes: 4c86bc531e60 ("tracing: Add :mod: command to enabled module events")
Cc: stable@vger.kernel.org
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
kernel/trace/trace_events.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c46e623e7e0d..6aa32c492280 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1460,6 +1460,8 @@ int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
/* Put back the colon to allow this to be called again */
if (buf)
*(buf - 1) = ':';
+ if (mod)
+ *(mod - 5) = ':';
return ret;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH RFC 2/7] tracing: Remove duplicate declaration of ftrace_set_clr_event()
2026-08-13 14:07 [PATCH RFC 0/7] tracing: Parse filter from event string Thomas Weißschuh
2026-08-13 14:07 ` [PATCH RFC 1/7] tracing: Restore :mod: trailer after parsing in ftrace_set_clr_event() Thomas Weißschuh
@ 2026-08-13 14:07 ` Thomas Weißschuh
2026-08-13 14:07 ` [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event() Thomas Weißschuh
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2026-08-13 14:07 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel, Thomas Weißschuh
The function is also declared in include/linux/trace_events.h which is
even included from kernel/trace/trace.h.
Having two declarations creates churn when changing the signature.
Remove the unnecessary, duplicate declaration.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
kernel/trace/trace.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 80fe152af1dd..c00e4741f815 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -2286,7 +2286,6 @@ static inline const char *get_syscall_name(int syscall)
void trace_event_init(void);
void trace_event_update_all(struct trace_eval_map **map, int len);
/* Used from boot time tracer */
-extern int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
extern int trigger_process_regex(struct trace_event_file *file, char *buff);
#else
static inline void __init trace_event_init(void) { }
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event()
2026-08-13 14:07 [PATCH RFC 0/7] tracing: Parse filter from event string Thomas Weißschuh
2026-08-13 14:07 ` [PATCH RFC 1/7] tracing: Restore :mod: trailer after parsing in ftrace_set_clr_event() Thomas Weißschuh
2026-08-13 14:07 ` [PATCH RFC 2/7] tracing: Remove duplicate declaration of ftrace_set_clr_event() Thomas Weißschuh
@ 2026-08-13 14:07 ` Thomas Weißschuh
2026-08-13 14:15 ` sashiko-bot
2026-08-13 14:07 ` [PATCH RFC 4/7] tracing: Split the event string parsing logic into a dedicated function Thomas Weißschuh
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Thomas Weißschuh @ 2026-08-13 14:07 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel, Thomas Weißschuh
ftrace_set_clr_event() modifies its input buffer during parsing.
Before returning to the caller these modifications are undone.
This is restoration is error prone and was forgotten before.
Upcoming changes will also make it more complicated.
Copy the input argument and modify that copy and drop the
restoration logic.
Also mark the input argument as const.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
include/linux/trace_events.h | 2 +-
kernel/trace/trace_events.c | 17 ++++++-----------
2 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 308c76b57d13..460d820fe7a4 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -873,7 +873,7 @@ extern int trace_add_event_call(struct trace_event_call *call);
extern int trace_remove_event_call(struct trace_event_call *call);
extern int trace_event_get_offsets(struct trace_event_call *call);
-int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
+int ftrace_set_clr_event(struct trace_array *tr, const char *buf, int set);
int trace_set_clr_event(const char *system, const char *event, int set);
int trace_array_set_clr_event(struct trace_array *tr, const char *system,
const char *event, bool enable);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 6aa32c492280..c75a7f5b55de 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1411,14 +1411,17 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
return ret;
}
-int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
+int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
{
char *event = NULL, *sub = NULL, *match, *mod;
- int ret;
if (!tr)
return -ENOENT;
+ char *buf __free(kfree) = kstrdup(_buf, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
/* Modules events can be appended with :mod:<module> */
mod = strstr(buf, ":mod:");
if (mod) {
@@ -1455,15 +1458,7 @@ int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
match = NULL;
}
- ret = __ftrace_set_clr_event(tr, match, sub, event, set, mod);
-
- /* Put back the colon to allow this to be called again */
- if (buf)
- *(buf - 1) = ':';
- if (mod)
- *(mod - 5) = ':';
-
- return ret;
+ return __ftrace_set_clr_event(tr, match, sub, event, set, mod);
}
/**
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event()
2026-08-13 14:07 ` [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event() Thomas Weißschuh
@ 2026-08-13 14:15 ` sashiko-bot
2026-08-13 14:38 ` Steven Rostedt
0 siblings, 1 reply; 15+ messages in thread
From: sashiko-bot @ 2026-08-13 14:15 UTC (permalink / raw)
To: Thomas Weißschuh; +Cc: linux-trace-kernel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Invalid free (causing kernel panic) and memory leak due to `strsep()` modifying a pointer managed by `__free(kfree)`.
--
--- Patch [3]: [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event() ---
commit c968a27c39638474537de841b7e2191407463142
Author: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
tracing: Stop modifying the input buffer in ftrace_set_clr_event()
This commit changes ftrace_set_clr_event() to copy its input buffer using
kstrdup() and manage it with __free(kfree). This removes the need to
restore the original buffer modifications before returning.
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 6aa32c4922807..c75a7f5b55def 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1411,14 +1411,17 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
> return ret;
> }
>
> -int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
> +int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
> {
> char *event = NULL, *sub = NULL, *match, *mod;
>
> if (!tr)
> return -ENOENT;
>
> + char *buf __free(kfree) = kstrdup(_buf, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
[Severity: High]
Will this lead to an invalid free or memory leak when buf is later modified?
Further down in ftrace_set_clr_event(), buf is passed to strsep(), which
modifies the pointer in place:
match = strsep(&buf, ":");
If strsep() advances the pointer to the next token, the cleanup function
will call kfree() on an interior pointer, causing a slab corruption bug or
kernel panic upon scope exit. If buf is advanced to NULL (when no colon is
found), it will result in a memory leak of the duplicated string since
kfree(NULL) is a no-op.
Could a separate pointer be used to track the allocation for cleanup,
leaving buf free to be safely advanced by strsep()?
> /* Modules events can be appended with :mod:<module> */
> mod = strstr(buf, ":mod:");
> if (mod) {
[ ... ]
> @@ -1455,15 +1458,7 @@ int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
> match = NULL;
> }
>
> - ret = __ftrace_set_clr_event(tr, match, sub, event, set, mod);
> -
> - /* Put back the colon to allow this to be called again */
> - if (buf)
> - *(buf - 1) = ':';
> - if (mod)
> - *(mod - 5) = ':';
> -
> - return ret;
> + return __ftrace_set_clr_event(tr, match, sub, event, set, mod);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813-tracing-cli-event-filter-v1-0-57c4e8029c86@linutronix.de?part=3
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event()
2026-08-13 14:15 ` sashiko-bot
@ 2026-08-13 14:38 ` Steven Rostedt
2026-08-13 14:40 ` Thomas Weißschuh
0 siblings, 1 reply; 15+ messages in thread
From: Steven Rostedt @ 2026-08-13 14:38 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, Thomas Weißschuh, linux-trace-kernel
On Thu, 13 Aug 2026 14:15:00 +0000
sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Invalid free (causing kernel panic) and memory leak due to `strsep()` modifying a pointer managed by `__free(kfree)`.
> --
>
> --- Patch [3]: [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event() ---
> commit c968a27c39638474537de841b7e2191407463142
> Author: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>
> tracing: Stop modifying the input buffer in ftrace_set_clr_event()
>
> This commit changes ftrace_set_clr_event() to copy its input buffer using
> kstrdup() and manage it with __free(kfree). This removes the need to
> restore the original buffer modifications before returning.
>
> > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> > index 6aa32c4922807..c75a7f5b55def 100644
> > --- a/kernel/trace/trace_events.c
> > +++ b/kernel/trace/trace_events.c
> > @@ -1411,14 +1411,17 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
> > return ret;
> > }
> >
> > -int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
> > +int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
> > {
> > char *event = NULL, *sub = NULL, *match, *mod;
> >
> > if (!tr)
> > return -ENOENT;
> >
> > + char *buf __free(kfree) = kstrdup(_buf, GFP_KERNEL);
> > + if (!buf)
> > + return -ENOMEM;
> > +
>
> [Severity: High]
> Will this lead to an invalid free or memory leak when buf is later modified?
>
> Further down in ftrace_set_clr_event(), buf is passed to strsep(), which
> modifies the pointer in place:
>
> match = strsep(&buf, ":");
>
> If strsep() advances the pointer to the next token, the cleanup function
> will call kfree() on an interior pointer, causing a slab corruption bug or
> kernel panic upon scope exit. If buf is advanced to NULL (when no colon is
> found), it will result in a memory leak of the duplicated string since
> kfree(NULL) is a no-op.
>
> Could a separate pointer be used to track the allocation for cleanup,
> leaving buf free to be safely advanced by strsep()?
Yes, this is a legitimate bug.
I guess you need to have:
int ftrace_set_clr_event(struct trace_array *tr, const char *buffer, int set)
{
char *event = NULL, *sub = NULL, *match, *mod;
char *buf;
int ret;
if (!tr)
return -ENOENT;
char *_buf __free(kfree) = kstrdup(_buf, GFP_KERNEL);
if (!_buf)
return -ENOMEM;
/* Need to keep _buf pointing to the start of the buffer */
buf = _buf;
-- Steve
>
> > /* Modules events can be appended with :mod:<module> */
> > mod = strstr(buf, ":mod:");
> > if (mod) {
>
> [ ... ]
>
> > @@ -1455,15 +1458,7 @@ int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
> > match = NULL;
> > }
> >
> > - ret = __ftrace_set_clr_event(tr, match, sub, event, set, mod);
> > -
> > - /* Put back the colon to allow this to be called again */
> > - if (buf)
> > - *(buf - 1) = ':';
> > - if (mod)
> > - *(mod - 5) = ':';
> > -
> > - return ret;
> > + return __ftrace_set_clr_event(tr, match, sub, event, set, mod);
> > }
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event()
2026-08-13 14:38 ` Steven Rostedt
@ 2026-08-13 14:40 ` Thomas Weißschuh
2026-08-13 14:45 ` Steven Rostedt
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Weißschuh @ 2026-08-13 14:40 UTC (permalink / raw)
To: Steven Rostedt; +Cc: sashiko-bot, sashiko-reviews, linux-trace-kernel
On Thu, Aug 13, 2026 at 10:38:21AM -0400, Steven Rostedt wrote:
> On Thu, 13 Aug 2026 14:15:00 +0000
> sashiko-bot@kernel.org wrote:
>
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [High] Invalid free (causing kernel panic) and memory leak due to `strsep()` modifying a pointer managed by `__free(kfree)`.
> > --
> >
> > --- Patch [3]: [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event() ---
> > commit c968a27c39638474537de841b7e2191407463142
> > Author: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> >
> > tracing: Stop modifying the input buffer in ftrace_set_clr_event()
> >
> > This commit changes ftrace_set_clr_event() to copy its input buffer using
> > kstrdup() and manage it with __free(kfree). This removes the need to
> > restore the original buffer modifications before returning.
> >
> > > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> > > index 6aa32c4922807..c75a7f5b55def 100644
> > > --- a/kernel/trace/trace_events.c
> > > +++ b/kernel/trace/trace_events.c
> > > @@ -1411,14 +1411,17 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
> > > return ret;
> > > }
> > >
> > > -int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
> > > +int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
> > > {
> > > char *event = NULL, *sub = NULL, *match, *mod;
> > >
> > > if (!tr)
> > > return -ENOENT;
> > >
> > > + char *buf __free(kfree) = kstrdup(_buf, GFP_KERNEL);
> > > + if (!buf)
> > > + return -ENOMEM;
> > > +
> >
> > [Severity: High]
> > Will this lead to an invalid free or memory leak when buf is later modified?
> >
> > Further down in ftrace_set_clr_event(), buf is passed to strsep(), which
> > modifies the pointer in place:
> >
> > match = strsep(&buf, ":");
> >
> > If strsep() advances the pointer to the next token, the cleanup function
> > will call kfree() on an interior pointer, causing a slab corruption bug or
> > kernel panic upon scope exit. If buf is advanced to NULL (when no colon is
> > found), it will result in a memory leak of the duplicated string since
> > kfree(NULL) is a no-op.
> >
> > Could a separate pointer be used to track the allocation for cleanup,
> > leaving buf free to be safely advanced by strsep()?
>
> Yes, this is a legitimate bug.
>
> I guess you need to have:
>
> int ftrace_set_clr_event(struct trace_array *tr, const char *buffer, int set)
> {
> char *event = NULL, *sub = NULL, *match, *mod;
> char *buf;
> int ret;
>
> if (!tr)
> return -ENOENT;
>
> char *_buf __free(kfree) = kstrdup(_buf, GFP_KERNEL);
> if (!_buf)
> return -ENOMEM;
>
> /* Need to keep _buf pointing to the start of the buffer */
> buf = _buf;
Agreed.
I'll deal with the Sashiko fallout if you are fine with the general idea.
Thomas
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event()
2026-08-13 14:40 ` Thomas Weißschuh
@ 2026-08-13 14:45 ` Steven Rostedt
0 siblings, 0 replies; 15+ messages in thread
From: Steven Rostedt @ 2026-08-13 14:45 UTC (permalink / raw)
To: Thomas Weißschuh; +Cc: sashiko-bot, sashiko-reviews, linux-trace-kernel
On Thu, 13 Aug 2026 16:40:48 +0200
Thomas Weißschuh <thomas.weissschuh@linutronix.de> wrote:
> I'll deal with the Sashiko fallout if you are fine with the general idea.
I'm fine with the general idea of adding filtering logic to kernel command
line, but can you show some examples of how it would work? That is,
examples of the kernel command line.
This should be in the cover letter.
Thanks,
-- Steve
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC 4/7] tracing: Split the event string parsing logic into a dedicated function
2026-08-13 14:07 [PATCH RFC 0/7] tracing: Parse filter from event string Thomas Weißschuh
` (2 preceding siblings ...)
2026-08-13 14:07 ` [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event() Thomas Weißschuh
@ 2026-08-13 14:07 ` Thomas Weißschuh
2026-08-13 14:07 ` [PATCH RFC 5/7] tracing: Add a test for ftrace_parse_event_string() Thomas Weißschuh
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2026-08-13 14:07 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel, Thomas Weißschuh
That new function can be tested with a KUnit test to prevent regressions
when adding new fields to the format.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
kernel/trace/trace_events.c | 59 ++++++++++++++++++++++++++-------------------
1 file changed, 34 insertions(+), 25 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c75a7f5b55de..4ff3f9214894 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1411,23 +1411,18 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
return ret;
}
-int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
+static void
+ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, char **mod)
{
- char *event = NULL, *sub = NULL, *match, *mod;
-
- if (!tr)
- return -ENOENT;
-
- char *buf __free(kfree) = kstrdup(_buf, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
+ *event = NULL;
+ *sub = NULL;
/* Modules events can be appended with :mod:<module> */
- mod = strstr(buf, ":mod:");
- if (mod) {
- *mod = '\0';
+ *mod = strstr(buf, ":mod:");
+ if (*mod) {
+ **mod = '\0';
/* move to the module name */
- mod += 5;
+ *mod += 5;
}
/*
@@ -1442,21 +1437,35 @@ int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
* the name <name> or any event that matches <name>
*/
- match = strsep(&buf, ":");
+ *match = strsep(&buf, ":");
if (buf) {
- sub = match;
- event = buf;
- match = NULL;
-
- if (!strlen(sub) || strcmp(sub, "*") == 0)
- sub = NULL;
- if (!strlen(event) || strcmp(event, "*") == 0)
- event = NULL;
- } else if (mod) {
+ *sub = *match;
+ *event = buf;
+ *match = NULL;
+
+ if (!strlen(*sub) || strcmp(*sub, "*") == 0)
+ *sub = NULL;
+ if (!strlen(*event) || strcmp(*event, "*") == 0)
+ *event = NULL;
+ } else if (*mod) {
/* Allow wildcard for no length or star */
- if (!strlen(match) || strcmp(match, "*") == 0)
- match = NULL;
+ if (!strlen(*match) || strcmp(*match, "*") == 0)
+ *match = NULL;
}
+}
+
+int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
+{
+ char *event, *sub, *match, *mod;
+
+ if (!tr)
+ return -ENOENT;
+
+ char *buf __free(kfree) = kstrdup(_buf, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ftrace_parse_event_string(buf, &match, &sub, &event, &mod);
return __ftrace_set_clr_event(tr, match, sub, event, set, mod);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH RFC 5/7] tracing: Add a test for ftrace_parse_event_string()
2026-08-13 14:07 [PATCH RFC 0/7] tracing: Parse filter from event string Thomas Weißschuh
` (3 preceding siblings ...)
2026-08-13 14:07 ` [PATCH RFC 4/7] tracing: Split the event string parsing logic into a dedicated function Thomas Weißschuh
@ 2026-08-13 14:07 ` Thomas Weißschuh
2026-08-13 14:14 ` sashiko-bot
2026-08-13 14:07 ` [PATCH RFC 6/7] tracing: Add a filter argument to __ftrace_set_clr_event() Thomas Weißschuh
2026-08-13 14:07 ` [PATCH RFC 7/7] tracing: Parse filter from event string Thomas Weißschuh
6 siblings, 1 reply; 15+ messages in thread
From: Thomas Weißschuh @ 2026-08-13 14:07 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel, Thomas Weißschuh
The parsing logic is a bit complicated and about to become more so.
Add a unit test to avoid regressions.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
kernel/trace/Kconfig | 8 ++++
kernel/trace/Makefile | 1 +
kernel/trace/trace.h | 4 ++
kernel/trace/trace_events.c | 5 +-
kernel/trace/trace_events_test.c | 101 +++++++++++++++++++++++++++++++++++++++
5 files changed, 118 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 084f34dc6c9f..e5eb26780abf 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -151,6 +151,14 @@ config EVENT_TRACING
select GLOB
bool
+config EVENT_TRACING_TEST
+ tristate "Test for event tracing" if !KUNIT_ALL_TESTS
+ depends on EVENT_TRACING
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ KUnit test for the event tracing implementation.
+
config CONTEXT_SWITCH_TRACER
bool
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index f934ff586bd4..7bef5a84ce47 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -96,6 +96,7 @@ obj-$(CONFIG_EVENT_TRACING) += blktrace.o
endif
obj-$(CONFIG_EVENT_TRACING) += trace_events.o
obj-$(CONFIG_EVENT_TRACING) += trace_export.o
+obj-$(CONFIG_EVENT_TRACING_TEST) += trace_events_test.o
obj-$(CONFIG_FTRACE_SYSCALLS) += trace_syscalls.o
ifeq ($(CONFIG_PERF_EVENTS),y)
obj-$(CONFIG_EVENT_TRACING) += trace_event_perf.o
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index c00e4741f815..6bf4f031efa8 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -2505,3 +2505,7 @@ static inline int rv_init_interface(void)
})
#endif /* _LINUX_KERNEL_TRACE_H */
+
+#if IS_ENABLED(CONFIG_KUNIT)
+void ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, char **mod);
+#endif
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 4ff3f9214894..098a5aee5ec7 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -23,6 +23,8 @@
#include <linux/slab.h>
#include <linux/delay.h>
+#include <kunit/visibility.h>
+
#include <trace/events/sched.h>
#include <trace/syscall.h>
@@ -1411,7 +1413,7 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
return ret;
}
-static void
+VISIBLE_IF_KUNIT void
ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, char **mod)
{
*event = NULL;
@@ -1453,6 +1455,7 @@ ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, cha
*match = NULL;
}
}
+EXPORT_SYMBOL_IF_KUNIT(ftrace_parse_event_string);
int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
{
diff --git a/kernel/trace/trace_events_test.c b/kernel/trace/trace_events_test.c
new file mode 100644
index 000000000000..e090a699b8d5
--- /dev/null
+++ b/kernel/trace/trace_events_test.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/module.h>
+
+#include <kunit/test.h>
+
+#include "trace.h"
+
+struct parse_event_test_case {
+ const char *input;
+
+ const char *match, *sub, *event, *mod;
+};
+
+static const struct parse_event_test_case parse_event_test_cases[] = {
+ {
+ "",
+ .match = "",
+ },
+ {
+ "*:event",
+ .event = "event",
+ },
+ {
+ ":event",
+ .event = "event",
+ },
+ {
+ "sub:*",
+ .sub = "sub",
+ },
+ {
+ "sub:",
+ .sub = "sub",
+ },
+ {
+ "sub:event",
+ .sub = "sub",
+ .event = "event",
+ },
+ {
+ "match",
+ .match = "match",
+ },
+ {
+ ":mod:module",
+ .mod = "module",
+ },
+ {
+ "sub:event:mod:module",
+ .sub = "sub",
+ .event = "event",
+ .mod = "module",
+ },
+};
+
+static void parse_event_test_desc(const struct parse_event_test_case *params, char *desc)
+{
+ snprintf(desc, KUNIT_PARAM_DESC_SIZE, "'%s'", params->input);
+}
+
+KUNIT_ARRAY_PARAM(parse_event, parse_event_test_cases, parse_event_test_desc);
+
+#define EXPECT_NULL_OR_STR_EQ(test, left, right) \
+({ \
+ if ((left) != NULL) \
+ KUNIT_EXPECT_STREQ(test, (left), (right)); \
+ else \
+ KUNIT_EXPECT_NULL(test, (right)); \
+})
+
+static void parse_event(struct kunit *test)
+{
+ const struct parse_event_test_case *params = test->param_value;
+ char *input, *match, *sub, *event, *mod;
+
+ input = kunit_kstrdup(test, params->input, GFP_KERNEL);
+ if (!input)
+ kunit_skip(test, "ENOMEM");
+
+ ftrace_parse_event_string(input, &match, &sub, &event, &mod);
+
+ EXPECT_NULL_OR_STR_EQ(test, match, params->match);
+ EXPECT_NULL_OR_STR_EQ(test, sub, params->sub);
+ EXPECT_NULL_OR_STR_EQ(test, event, params->event);
+ EXPECT_NULL_OR_STR_EQ(test, mod, params->mod);
+}
+
+static struct kunit_case trace_events_test_cases[] = {
+ KUNIT_CASE_PARAM(parse_event, parse_event_gen_params),
+ {}
+};
+
+static struct kunit_suite trace_events_test_suite = {
+ .name = "trace-events",
+ .test_cases = trace_events_test_cases,
+};
+kunit_test_suite(trace_events_test_suite);
+
+
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH RFC 5/7] tracing: Add a test for ftrace_parse_event_string()
2026-08-13 14:07 ` [PATCH RFC 5/7] tracing: Add a test for ftrace_parse_event_string() Thomas Weißschuh
@ 2026-08-13 14:14 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-13 14:14 UTC (permalink / raw)
To: Thomas Weißschuh; +Cc: linux-trace-kernel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly added module `trace_events_test.c` lacks a `MODULE_DESCRIPTION()` declaration. Since `CONFIG_EVENT_TRACING_TEST` is a tristate config, the code can be compiled as a module. Recent Linux kernels strictly enforce module descriptions, and omitting it results in a modpost warning (`WARNING: modpost: missing MODULE_DESCRIPTION() in kernel/trace/trace_events_test.o`). In automated CI environments that enforce zero warnings or build with `-Werror`, this will cause build failures.
--
--- Patch [5]: [PATCH RFC 5/7] tracing: Add a test for ftrace_parse_event_string() ---
commit 509c596a834391e59b26327382138635d44f58c9
Author: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
tracing: Add a test for ftrace_parse_event_string()
This commit adds a KUnit test for the ftrace_parse_event_string() function
to prevent regressions as the parsing logic is updated.
> diff --git a/kernel/trace/trace_events_test.c b/kernel/trace/trace_events_test.c
> new file mode 100644
> --- /dev/null
> +++ b/kernel/trace/trace_events_test.c
[ ... ]
> +static struct kunit_suite trace_events_test_suite = {
> + .name = "trace-events",
> + .test_cases = trace_events_test_cases,
> +};
> +kunit_test_suite(trace_events_test_suite);
> +
> +
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
[Severity: Low]
Since CONFIG_EVENT_TRACING_TEST is a tristate config and this can be compiled
as a module, should a MODULE_DESCRIPTION() be added here?
Recent kernels emit a modpost warning (WARNING: modpost: missing
MODULE_DESCRIPTION() in kernel/trace/trace_events_test.o) when this is
omitted, which could cause build failures in automated CI environments that
enforce zero warnings.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813-tracing-cli-event-filter-v1-0-57c4e8029c86@linutronix.de?part=5
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC 6/7] tracing: Add a filter argument to __ftrace_set_clr_event()
2026-08-13 14:07 [PATCH RFC 0/7] tracing: Parse filter from event string Thomas Weißschuh
` (4 preceding siblings ...)
2026-08-13 14:07 ` [PATCH RFC 5/7] tracing: Add a test for ftrace_parse_event_string() Thomas Weißschuh
@ 2026-08-13 14:07 ` Thomas Weißschuh
2026-08-13 14:07 ` [PATCH RFC 7/7] tracing: Parse filter from event string Thomas Weißschuh
6 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2026-08-13 14:07 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel, Thomas Weißschuh
Make it possible to directly enable a filter when enabling an event.
For now this is unused, an upcoming patch will wire it up.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
kernel/trace/trace_events.c | 47 +++++++++++++++++++++++++++++----------------
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 098a5aee5ec7..c24929d003dd 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -916,6 +916,7 @@ struct event_mod_load {
char *match;
char *system;
char *event;
+ char *filter;
};
static void free_event_mod(struct event_mod_load *event_mod)
@@ -925,6 +926,7 @@ static void free_event_mod(struct event_mod_load *event_mod)
kfree(event_mod->match);
kfree(event_mod->system);
kfree(event_mod->event);
+ kfree(event_mod->filter);
kfree(event_mod);
}
@@ -966,7 +968,7 @@ static int remove_cache_mod(struct trace_array *tr, const char *mod,
}
static int cache_mod(struct trace_array *tr, const char *mod, int set,
- const char *match, const char *system, const char *event)
+ const char *match, const char *system, const char *event, const char *filter)
{
struct event_mod_load *event_mod;
@@ -1005,6 +1007,12 @@ static int cache_mod(struct trace_array *tr, const char *mod, int set,
goto out_free;
}
+ if (filter) {
+ event_mod->filter = kstrdup(filter, GFP_KERNEL);
+ if (!event_mod->filter)
+ goto out_free;
+ }
+
list_add(&event_mod->list, &tr->mod_events);
return 0;
@@ -1017,7 +1025,7 @@ static int cache_mod(struct trace_array *tr, const char *mod, int set,
#else /* CONFIG_MODULES */
static inline void clear_mod_events(struct trace_array *tr) { }
static int cache_mod(struct trace_array *tr, const char *mod, int set,
- const char *match, const char *system, const char *event)
+ const char *match, const char *system, const char *event, const char *filter)
{
return -EINVAL;
}
@@ -1326,7 +1334,7 @@ static void remove_event_file_dir(struct trace_event_file *file)
static int
__ftrace_set_clr_event_nolock(struct trace_array *tr, const char *match,
const char *sub, const char *event, int set,
- const char *mod)
+ const char *mod, char *filter)
{
struct trace_event_file *file;
struct trace_event_call *call;
@@ -1374,7 +1382,11 @@ __ftrace_set_clr_event_nolock(struct trace_array *tr, const char *match,
if (event && strcmp(event, name) != 0)
continue;
- ret = ftrace_event_enable_disable(file, set);
+ if (filter)
+ ret = apply_event_filter(file, filter);
+
+ if (!filter || !ret)
+ ret = ftrace_event_enable_disable(file, set);
/*
* Save the first error and return that. Some events
@@ -1392,14 +1404,14 @@ __ftrace_set_clr_event_nolock(struct trace_array *tr, const char *match,
* check if the module was loaded. If it wasn't cache it.
*/
if (module && ret == -EINVAL && !eret)
- ret = cache_mod(tr, module, set, match, sub, event);
+ ret = cache_mod(tr, module, set, match, sub, event, filter);
return ret;
}
static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
const char *sub, const char *event, int set,
- const char *mod)
+ const char *mod, char *filter)
{
int ret;
@@ -1407,7 +1419,7 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
return -EACCES;
mutex_lock(&event_mutex);
- ret = __ftrace_set_clr_event_nolock(tr, match, sub, event, set, mod);
+ ret = __ftrace_set_clr_event_nolock(tr, match, sub, event, set, mod, filter);
mutex_unlock(&event_mutex);
return ret;
@@ -1470,7 +1482,7 @@ int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
ftrace_parse_event_string(buf, &match, &sub, &event, &mod);
- return __ftrace_set_clr_event(tr, match, sub, event, set, mod);
+ return __ftrace_set_clr_event(tr, match, sub, event, set, mod, NULL);
}
/**
@@ -1492,7 +1504,7 @@ int trace_set_clr_event(const char *system, const char *event, int set)
if (!tr)
return -ENODEV;
- return __ftrace_set_clr_event(tr, NULL, system, event, set, NULL);
+ return __ftrace_set_clr_event(tr, NULL, system, event, set, NULL, NULL);
}
EXPORT_SYMBOL_GPL(trace_set_clr_event);
@@ -1518,7 +1530,7 @@ int trace_array_set_clr_event(struct trace_array *tr, const char *system,
return -ENOENT;
set = (enable == true) ? 1 : 0;
- return __ftrace_set_clr_event(tr, NULL, system, event, set, NULL);
+ return __ftrace_set_clr_event(tr, NULL, system, event, set, NULL, NULL);
}
EXPORT_SYMBOL_GPL(trace_array_set_clr_event);
@@ -2038,7 +2050,7 @@ system_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
if (system)
name = system->name;
- ret = __ftrace_set_clr_event(dir->tr, NULL, name, NULL, val, NULL);
+ ret = __ftrace_set_clr_event(dir->tr, NULL, name, NULL, val, NULL, NULL);
if (ret)
goto out;
@@ -3909,7 +3921,8 @@ static void update_mod_cache(struct trace_array *tr, struct module *mod)
__ftrace_set_clr_event_nolock(tr, event_mod->match,
event_mod->system,
- event_mod->event, 1, mod->name);
+ event_mod->event, 1, mod->name,
+ event_mod->filter);
free_event_mod(event_mod);
}
}
@@ -4676,7 +4689,7 @@ int event_trace_del_tracer(struct trace_array *tr)
__ftrace_clear_event_pids(tr, TRACE_PIDS | TRACE_NO_PIDS);
/* Disable any running events */
- __ftrace_set_clr_event_nolock(tr, NULL, NULL, NULL, 0, NULL);
+ __ftrace_set_clr_event_nolock(tr, NULL, NULL, NULL, 0, NULL, NULL);
/* Make sure no more events are being executed */
tracepoint_synchronize_unregister();
@@ -4982,7 +4995,7 @@ static __init void event_trace_self_tests(void)
pr_info("Testing event system %s: ", system->name);
- ret = __ftrace_set_clr_event(tr, NULL, system->name, NULL, 1, NULL);
+ ret = __ftrace_set_clr_event(tr, NULL, system->name, NULL, 1, NULL, NULL);
if (WARN_ON_ONCE(ret)) {
pr_warn("error enabling system %s\n",
system->name);
@@ -4991,7 +5004,7 @@ static __init void event_trace_self_tests(void)
event_test_stuff();
- ret = __ftrace_set_clr_event(tr, NULL, system->name, NULL, 0, NULL);
+ ret = __ftrace_set_clr_event(tr, NULL, system->name, NULL, 0, NULL, NULL);
if (WARN_ON_ONCE(ret)) {
pr_warn("error disabling system %s\n",
system->name);
@@ -5006,7 +5019,7 @@ static __init void event_trace_self_tests(void)
pr_info("Running tests on all trace events:\n");
pr_info("Testing all events: ");
- ret = __ftrace_set_clr_event(tr, NULL, NULL, NULL, 1, NULL);
+ ret = __ftrace_set_clr_event(tr, NULL, NULL, NULL, 1, NULL, NULL);
if (WARN_ON_ONCE(ret)) {
pr_warn("error enabling all events\n");
return;
@@ -5015,7 +5028,7 @@ static __init void event_trace_self_tests(void)
event_test_stuff();
/* reset sysname */
- ret = __ftrace_set_clr_event(tr, NULL, NULL, NULL, 0, NULL);
+ ret = __ftrace_set_clr_event(tr, NULL, NULL, NULL, 0, NULL, NULL);
if (WARN_ON_ONCE(ret)) {
pr_warn("error disabling all events\n");
return;
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH RFC 7/7] tracing: Parse filter from event string
2026-08-13 14:07 [PATCH RFC 0/7] tracing: Parse filter from event string Thomas Weißschuh
` (5 preceding siblings ...)
2026-08-13 14:07 ` [PATCH RFC 6/7] tracing: Add a filter argument to __ftrace_set_clr_event() Thomas Weißschuh
@ 2026-08-13 14:07 ` Thomas Weißschuh
2026-08-13 14:22 ` sashiko-bot
2026-08-13 16:22 ` Masami Hiramatsu
6 siblings, 2 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2026-08-13 14:07 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel, Thomas Weißschuh
Make it possible to specify a trace event filter from the
kernel command line.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
For the non-RFC patch this should probably be limited to only apply to
the cmdline path.
---
kernel/trace/trace.h | 3 ++-
kernel/trace/trace_events.c | 13 +++++++++----
kernel/trace/trace_events_test.c | 20 +++++++++++++++++---
3 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 6bf4f031efa8..9102bf0a45a9 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -2507,5 +2507,6 @@ static inline int rv_init_interface(void)
#endif /* _LINUX_KERNEL_TRACE_H */
#if IS_ENABLED(CONFIG_KUNIT)
-void ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, char **mod);
+void ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, char **mod,
+ char **filter);
#endif
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c24929d003dd..d4c46b24c382 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1426,10 +1426,12 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
}
VISIBLE_IF_KUNIT void
-ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, char **mod)
+ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, char **mod,
+ char **filter)
{
*event = NULL;
*sub = NULL;
+ *filter = NULL;
/* Modules events can be appended with :mod:<module> */
*mod = strstr(buf, ":mod:");
@@ -1453,6 +1455,9 @@ ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, cha
*match = strsep(&buf, ":");
if (buf) {
+ *filter = buf;
+ strsep(filter, ":");
+
*sub = *match;
*event = buf;
*match = NULL;
@@ -1471,7 +1476,7 @@ EXPORT_SYMBOL_IF_KUNIT(ftrace_parse_event_string);
int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
{
- char *event, *sub, *match, *mod;
+ char *event, *sub, *match, *mod, *filter;
if (!tr)
return -ENOENT;
@@ -1480,9 +1485,9 @@ int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
if (!buf)
return -ENOMEM;
- ftrace_parse_event_string(buf, &match, &sub, &event, &mod);
+ ftrace_parse_event_string(buf, &match, &sub, &event, &mod, &filter);
- return __ftrace_set_clr_event(tr, match, sub, event, set, mod, NULL);
+ return __ftrace_set_clr_event(tr, match, sub, event, set, mod, filter);
}
/**
diff --git a/kernel/trace/trace_events_test.c b/kernel/trace/trace_events_test.c
index e090a699b8d5..fd54eb03b58f 100644
--- a/kernel/trace/trace_events_test.c
+++ b/kernel/trace/trace_events_test.c
@@ -8,7 +8,7 @@
struct parse_event_test_case {
const char *input;
- const char *match, *sub, *event, *mod;
+ const char *match, *sub, *event, *mod, *filter;
};
static const struct parse_event_test_case parse_event_test_cases[] = {
@@ -37,6 +37,12 @@ static const struct parse_event_test_case parse_event_test_cases[] = {
.sub = "sub",
.event = "event",
},
+ {
+ "sub:event:filter",
+ .sub = "sub",
+ .event = "event",
+ .filter = "filter",
+ },
{
"match",
.match = "match",
@@ -51,6 +57,13 @@ static const struct parse_event_test_case parse_event_test_cases[] = {
.event = "event",
.mod = "module",
},
+ {
+ "sub:event:filter:mod:module",
+ .sub = "sub",
+ .event = "event",
+ .mod = "module",
+ .filter = "filter",
+ },
};
static void parse_event_test_desc(const struct parse_event_test_case *params, char *desc)
@@ -71,18 +84,19 @@ KUNIT_ARRAY_PARAM(parse_event, parse_event_test_cases, parse_event_test_desc);
static void parse_event(struct kunit *test)
{
const struct parse_event_test_case *params = test->param_value;
- char *input, *match, *sub, *event, *mod;
+ char *input, *match, *sub, *event, *mod, *filter;
input = kunit_kstrdup(test, params->input, GFP_KERNEL);
if (!input)
kunit_skip(test, "ENOMEM");
- ftrace_parse_event_string(input, &match, &sub, &event, &mod);
+ ftrace_parse_event_string(input, &match, &sub, &event, &mod, &filter);
EXPECT_NULL_OR_STR_EQ(test, match, params->match);
EXPECT_NULL_OR_STR_EQ(test, sub, params->sub);
EXPECT_NULL_OR_STR_EQ(test, event, params->event);
EXPECT_NULL_OR_STR_EQ(test, mod, params->mod);
+ EXPECT_NULL_OR_STR_EQ(test, filter, params->filter);
}
static struct kunit_case trace_events_test_cases[] = {
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH RFC 7/7] tracing: Parse filter from event string
2026-08-13 14:07 ` [PATCH RFC 7/7] tracing: Parse filter from event string Thomas Weißschuh
@ 2026-08-13 14:22 ` sashiko-bot
2026-08-13 16:22 ` Masami Hiramatsu
1 sibling, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-13 14:22 UTC (permalink / raw)
To: Thomas Weißschuh; +Cc: linux-trace-kernel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `:mod:` suffix parsing using `strstr` is broken by the introduction of filter expressions, which can legitimately contain `:mod:` within string literals.
- [Medium] Unintended userspace ABI change for the debugfs `set_event` interface by exposing event string filter parsing.
--
--- Patch [7]: [PATCH RFC 7/7] tracing: Parse filter from event string ---
commit d99b1133b250a61ac85f36416d5a9f5e175d37f9
Author: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
tracing: Parse filter from event string
This commit makes it possible to specify a trace event filter from the kernel
command line by extracting it from the event string.
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index c24929d003dd6..d4c46b24c3829 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1426,10 +1426,12 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
> }
>
> VISIBLE_IF_KUNIT void
> -ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, char **mod)
> +ftrace_parse_event_string(char *buf, char **match, char **sub, char **event, char **mod,
> + char **filter)
> {
> *event = NULL;
> *sub = NULL;
> + *filter = NULL;
>
> /* Modules events can be appended with :mod:<module> */
> *mod = strstr(buf, ":mod:");
[Severity: High]
Does this code incorrectly match the module string inside the filter string?
If a filter expression is provided that legitimately contains the literal
string ":mod:" (for example, comm == "my:mod:app"), strstr() will match it
here before the buffer is tokenized.
This could truncate the filter and falsely extract a module name, causing
event enablement to fail.
[ ... ]
> @@ -1471,7 +1476,7 @@ EXPORT_SYMBOL_IF_KUNIT(ftrace_parse_event_string);
>
> int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
> {
> - char *event, *sub, *match, *mod;
> + char *event, *sub, *match, *mod, *filter;
>
> if (!tr)
> return -ENOENT;
> @@ -1480,9 +1485,9 @@ int ftrace_set_clr_event(struct trace_array *tr, const char *_buf, int set)
> if (!buf)
> return -ENOMEM;
>
> - ftrace_parse_event_string(buf, &match, &sub, &event, &mod);
> + ftrace_parse_event_string(buf, &match, &sub, &event, &mod, &filter);
>
> - return __ftrace_set_clr_event(tr, match, sub, event, set, mod, NULL);
> + return __ftrace_set_clr_event(tr, match, sub, event, set, mod, filter);
> }
[Severity: Medium]
Will this unintentionally alter the ABI for the set_event debugfs file?
Since ftrace_set_clr_event() is called by the debugfs set_event file write
handler, this change exposes the filter parsing to userspace writes.
Writing strings with colons directly to set_event will now attempt to parse
them as filters, changing the expected ABI behavior and error semantics of
the interface.
As mentioned in the patch notes, shouldn't this be limited to only apply to
the command line path?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813-tracing-cli-event-filter-v1-0-57c4e8029c86@linutronix.de?part=7
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH RFC 7/7] tracing: Parse filter from event string
2026-08-13 14:07 ` [PATCH RFC 7/7] tracing: Parse filter from event string Thomas Weißschuh
2026-08-13 14:22 ` sashiko-bot
@ 2026-08-13 16:22 ` Masami Hiramatsu
1 sibling, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2026-08-13 16:22 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Steven Rostedt, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
On Thu, 13 Aug 2026 16:07:20 +0200
Thomas Weißschuh <thomas.weissschuh@linutronix.de> wrote:
> Make it possible to specify a trace event filter from the
> kernel command line.
>
But how to use it? Could you give an example and update the document?
Thank you,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 15+ messages in thread