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

Re: [PATCH RFC 3/7] tracing: Stop modifying the input buffer in ftrace_set_clr_event()

From: Steven Rostedt <rostedt@goodmis.org>
Date: 2026-08-13 14:38:14
Also in: sashiko-reviews

On Thu, 13 Aug 2026 14:15:00 +0000
sashiko-bot@kernel.org wrote:
quoted hunk ↗ jump to hunk
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;

-- Steve

quoted
 	/* Modules events can be appended with :mod:<module> */
 	mod = strstr(buf, ":mod:");
 	if (mod) {  
[ ... ]
quoted
@@ -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);
 }  
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help