[PATCH v2 1/6] tracing: probe: Allocate traceprobe_parse_context from heap

Masami Hiramatsu (Google) posted 6 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 1/6] tracing: probe: Allocate traceprobe_parse_context from heap
Posted by Masami Hiramatsu (Google) 2 months, 2 weeks ago
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Instead of allocating traceprobe_parse_context on stack, allocate it
dynamically from heap (slab).

This change is likely intended to prevent potential stack overflow
issues, which can be a concern in the kernel environment where stack
space is limited.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506240416.nZIhDXoO-lkp@intel.com/
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v2:
  - Sort #include alphabetically.
  - Just NULL check for freeing traceprobe_parse_context.
  - Do not change the semantics of traceprobe_parse_context for uprobe
    event. (alloc/free in loop)
---
 kernel/trace/trace_eprobe.c |   14 ++++++++------
 kernel/trace/trace_fprobe.c |   13 ++++++++-----
 kernel/trace/trace_kprobe.c |   10 +++++++---
 kernel/trace/trace_probe.h  |   26 ++++++++++++++++++--------
 kernel/trace/trace_uprobe.c |   13 ++++++++-----
 5 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index 916555f0de81..1e18a8619b40 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -797,18 +797,20 @@ find_and_get_event(const char *system, const char *event_name)
 
 static int trace_eprobe_tp_update_arg(struct trace_eprobe *ep, const char *argv[], int i)
 {
-	struct traceprobe_parse_context ctx = {
-		.event = ep->event,
-		.flags = TPARG_FL_KERNEL | TPARG_FL_TEVENT,
-	};
+	struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL;
 	int ret;
 
-	ret = traceprobe_parse_probe_arg(&ep->tp, i, argv[i], &ctx);
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	ctx->event = ep->event;
+	ctx->flags = TPARG_FL_KERNEL | TPARG_FL_TEVENT;
+
+	ret = traceprobe_parse_probe_arg(&ep->tp, i, argv[i], ctx);
 	/* Handle symbols "@" */
 	if (!ret)
 		ret = traceprobe_update_arg(&ep->tp.args[i]);
 
-	traceprobe_finish_parse(&ctx);
 	return ret;
 }
 
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index dbf9d413125a..264cf7fc9a1d 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -1383,14 +1383,17 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
 
 static int trace_fprobe_create_cb(int argc, const char *argv[])
 {
-	struct traceprobe_parse_context ctx = {
-		.flags = TPARG_FL_KERNEL | TPARG_FL_FPROBE,
-	};
+	struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL;
 	int ret;
 
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->flags = TPARG_FL_KERNEL | TPARG_FL_FPROBE,
+
 	trace_probe_log_init("trace_fprobe", argc, argv);
-	ret = trace_fprobe_create_internal(argc, argv, &ctx);
-	traceprobe_finish_parse(&ctx);
+	ret = trace_fprobe_create_internal(argc, argv, ctx);
 	trace_probe_log_clear();
 	return ret;
 }
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 3e5c47b6d7b2..15d7a381a128 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1065,14 +1065,18 @@ static int trace_kprobe_create_internal(int argc, const char *argv[],
 
 static int trace_kprobe_create_cb(int argc, const char *argv[])
 {
-	struct traceprobe_parse_context ctx = { .flags = TPARG_FL_KERNEL };
+	struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL;
 	int ret;
 
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	ctx->flags = TPARG_FL_KERNEL;
+
 	trace_probe_log_init("trace_kprobe", argc, argv);
 
-	ret = trace_kprobe_create_internal(argc, argv, &ctx);
+	ret = trace_kprobe_create_internal(argc, argv, ctx);
 
-	traceprobe_finish_parse(&ctx);
 	trace_probe_log_clear();
 	return ret;
 }
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 854e5668f5ee..842383fbc03b 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -10,20 +10,22 @@
  * Author:     Srikar Dronamraju
  */
 
+#include <linux/bitops.h>
+#include <linux/btf.h>
+#include <linux/cleanup.h>
+#include <linux/kprobes.h>
+#include <linux/limits.h>
+#include <linux/perf_event.h>
+#include <linux/ptrace.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/smp.h>
-#include <linux/tracefs.h>
-#include <linux/types.h>
 #include <linux/string.h>
-#include <linux/ptrace.h>
-#include <linux/perf_event.h>
-#include <linux/kprobes.h>
 #include <linux/stringify.h>
-#include <linux/limits.h>
+#include <linux/tracefs.h>
+#include <linux/types.h>
 #include <linux/uaccess.h>
-#include <linux/bitops.h>
-#include <linux/btf.h>
+
 #include <asm/bitsperlong.h>
 
 #include "trace.h"
@@ -438,6 +440,14 @@ extern void traceprobe_free_probe_arg(struct probe_arg *arg);
  * this MUST be called for clean up the context and return a resource.
  */
 void traceprobe_finish_parse(struct traceprobe_parse_context *ctx);
+static inline void traceprobe_free_parse_ctx(struct traceprobe_parse_context *ctx)
+{
+	traceprobe_finish_parse(ctx);
+	kfree(ctx);
+}
+
+DEFINE_FREE(traceprobe_parse_context, struct traceprobe_parse_context *,
+	if (_T) traceprobe_free_parse_ctx(_T))
 
 extern int traceprobe_split_symbol_offset(char *symbol, long *offset);
 int traceprobe_parse_event_name(const char **pevent, const char **pgroup,
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index f95a2c3d5b1b..797ac1250956 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -695,13 +695,16 @@ static int __trace_uprobe_create(int argc, const char **argv)
 
 	/* parse arguments */
 	for (i = 0; i < argc; i++) {
-		struct traceprobe_parse_context ctx = {
-			.flags = (is_return ? TPARG_FL_RETURN : 0) | TPARG_FL_USER,
-		};
+		struct traceprobe_parse_context *ctx __free(traceprobe_parse_context)
+			= kzalloc(sizeof(*ctx), GFP_KERNEL);
 
+		if (!ctx) {
+			ret = -ENOMEM;
+			goto error;
+		}
+		ctx->flags = (is_return ? TPARG_FL_RETURN : 0) | TPARG_FL_USER;
 		trace_probe_log_set_index(i + 2);
-		ret = traceprobe_parse_probe_arg(&tu->tp, i, argv[i], &ctx);
-		traceprobe_finish_parse(&ctx);
+		ret = traceprobe_parse_probe_arg(&tu->tp, i, argv[i], ctx);
 		if (ret)
 			goto error;
 	}
Re: [PATCH v2 1/6] tracing: probe: Allocate traceprobe_parse_context from heap
Posted by Steven Rostedt 2 months, 2 weeks ago
On Sun, 20 Jul 2025 15:21:47 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

>  
> +#include <linux/bitops.h>
> +#include <linux/btf.h>
> +#include <linux/cleanup.h>
> +#include <linux/kprobes.h>
> +#include <linux/limits.h>
> +#include <linux/perf_event.h>
> +#include <linux/ptrace.h>
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
>  #include <linux/smp.h>
> -#include <linux/tracefs.h>
> -#include <linux/types.h>
>  #include <linux/string.h>
> -#include <linux/ptrace.h>
> -#include <linux/perf_event.h>
> -#include <linux/kprobes.h>
>  #include <linux/stringify.h>
> -#include <linux/limits.h>
> +#include <linux/tracefs.h>
> +#include <linux/types.h>
>  #include <linux/uaccess.h>
> -#include <linux/bitops.h>
> -#include <linux/btf.h>
> +

A re-sort should be a separate patch, as I can't tell what changed in
the header for this patch.

-- Steve
Re: [PATCH v2 1/6] tracing: probe: Allocate traceprobe_parse_context from heap
Posted by Masami Hiramatsu (Google) 2 months, 2 weeks ago
On Mon, 21 Jul 2025 13:19:07 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Sun, 20 Jul 2025 15:21:47 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> >  
> > +#include <linux/bitops.h>
> > +#include <linux/btf.h>
> > +#include <linux/cleanup.h>
> > +#include <linux/kprobes.h>
> > +#include <linux/limits.h>
> > +#include <linux/perf_event.h>
> > +#include <linux/ptrace.h>
> >  #include <linux/seq_file.h>
> >  #include <linux/slab.h>
> >  #include <linux/smp.h>
> > -#include <linux/tracefs.h>
> > -#include <linux/types.h>
> >  #include <linux/string.h>
> > -#include <linux/ptrace.h>
> > -#include <linux/perf_event.h>
> > -#include <linux/kprobes.h>
> >  #include <linux/stringify.h>
> > -#include <linux/limits.h>
> > +#include <linux/tracefs.h>
> > +#include <linux/types.h>
> >  #include <linux/uaccess.h>
> > -#include <linux/bitops.h>
> > -#include <linux/btf.h>
> > +
> 
> A re-sort should be a separate patch, as I can't tell what changed in
> the header for this patch.

OK, let me make it a separate patch.

Thanks!

> 
> -- Steve


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>