Thread (15 messages) flat view 15 messages, 4 authors, 2d ago

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help