On Thu, Aug 13, 2026 at 10:38:21AM -0400, Steven Rostedt wrote: > 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;
Agreed. I'll deal with the Sashiko fallout if you are fine with the general idea. Thomas
