On Thu, 13 Aug 2026 14:15:00 +0000
[email protected] 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 <[email protected]>
>
> 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);
> > }
>
