Re: [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event()
From: Thomas Weißschuh <hidden>
Date: 2026-08-13 14:40:50
Also in:
sashiko-reviews
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:quoted
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 [off-list ref] 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.quoted
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