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>
---
kernel/trace/trace_eprobe.c | 14 ++++++++------
kernel/trace/trace_fprobe.c | 13 ++++++++-----
kernel/trace/trace_kprobe.c | 10 +++++++---
kernel/trace/trace_probe.h | 9 +++++++++
kernel/trace/trace_uprobe.c | 15 +++++++++------
5 files changed, 41 insertions(+), 20 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..7bc4c84464e4 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -10,6 +10,7 @@
* Author: Srikar Dronamraju
*/
+#include <linux/cleanup.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/smp.h>
@@ -438,6 +439,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 (!IS_ERR_OR_NULL(_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..1fd479718d03 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -537,6 +537,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
*/
static int __trace_uprobe_create(int argc, const char **argv)
{
+ struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL;
struct trace_uprobe *tu;
const char *event = NULL, *group = UPROBE_EVENT_SYSTEM;
char *arg, *filename, *rctr, *rctr_end, *tmp;
@@ -693,15 +694,17 @@ static int __trace_uprobe_create(int argc, const char **argv)
tu->path = path;
tu->filename = filename;
+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+ if (!ctx) {
+ ret = -ENOMEM;
+ goto error;
+ }
+ ctx->flags = (is_return ? TPARG_FL_RETURN : 0) | TPARG_FL_USER;
+
/* parse arguments */
for (i = 0; i < argc; i++) {
- struct traceprobe_parse_context 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;
}On Fri, 18 Jul 2025 20:34:08 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 854e5668f5ee..7bc4c84464e4 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -10,6 +10,7 @@
> * Author: Srikar Dronamraju
> */
>
> +#include <linux/cleanup.h>
> #include <linux/seq_file.h>
> #include <linux/slab.h>
> #include <linux/smp.h>
Nit, but let's keep the "upside-down x-mas tree" format:
#include <linux/seq_file.h>
#include <linux/cleanup.h>
#include <linux/slab.h>
#include <linux/smp.h>
> @@ -438,6 +439,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 (!IS_ERR_OR_NULL(_T)) traceprobe_free_parse_ctx(_T))
ctx will either be allocated or NULL, I think the above could be:
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..1fd479718d03 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -537,6 +537,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
> */
> static int __trace_uprobe_create(int argc, const char **argv)
> {
> + struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL;
> struct trace_uprobe *tu;
> const char *event = NULL, *group = UPROBE_EVENT_SYSTEM;
> char *arg, *filename, *rctr, *rctr_end, *tmp;
> @@ -693,15 +694,17 @@ static int __trace_uprobe_create(int argc, const char **argv)
> tu->path = path;
> tu->filename = filename;
>
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx) {
> + ret = -ENOMEM;
> + goto error;
> + }
> + ctx->flags = (is_return ? TPARG_FL_RETURN : 0) | TPARG_FL_USER;
> +
> /* parse arguments */
> for (i = 0; i < argc; i++) {
> - struct traceprobe_parse_context 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);
Doesn't this change the semantics a bit?
Before this change, traceprobe_finish_parse(&ctx) is called for every
iteration of the loop. Now we only do it when it exits the function.
-- Steve
> if (ret)
> goto error;
> }
On Fri, 18 Jul 2025 12:58:20 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 18 Jul 2025 20:34:08 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
>
> > diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> > index 854e5668f5ee..7bc4c84464e4 100644
> > --- a/kernel/trace/trace_probe.h
> > +++ b/kernel/trace/trace_probe.h
> > @@ -10,6 +10,7 @@
> > * Author: Srikar Dronamraju
> > */
> >
> > +#include <linux/cleanup.h>
> > #include <linux/seq_file.h>
> > #include <linux/slab.h>
> > #include <linux/smp.h>
>
> Nit, but let's keep the "upside-down x-mas tree" format:
>
> #include <linux/seq_file.h>
> #include <linux/cleanup.h>
> #include <linux/slab.h>
> #include <linux/smp.h>
Isn't it for variable rules?
I saw some examples of sorting headers by A-Z.
>
>
> > @@ -438,6 +439,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 (!IS_ERR_OR_NULL(_T)) traceprobe_free_parse_ctx(_T))
>
> ctx will either be allocated or NULL, I think the above could be:
>
> if (_T) traceprobe_free_parse_ctx(_T))
OK.
>
>
> >
> > 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..1fd479718d03 100644
> > --- a/kernel/trace/trace_uprobe.c
> > +++ b/kernel/trace/trace_uprobe.c
> > @@ -537,6 +537,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
> > */
> > static int __trace_uprobe_create(int argc, const char **argv)
> > {
> > + struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL;
> > struct trace_uprobe *tu;
> > const char *event = NULL, *group = UPROBE_EVENT_SYSTEM;
> > char *arg, *filename, *rctr, *rctr_end, *tmp;
> > @@ -693,15 +694,17 @@ static int __trace_uprobe_create(int argc, const char **argv)
> > tu->path = path;
> > tu->filename = filename;
> >
> > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> > + if (!ctx) {
> > + ret = -ENOMEM;
> > + goto error;
> > + }
> > + ctx->flags = (is_return ? TPARG_FL_RETURN : 0) | TPARG_FL_USER;
> > +
> > /* parse arguments */
> > for (i = 0; i < argc; i++) {
> > - struct traceprobe_parse_context 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);
>
> Doesn't this change the semantics a bit?
Yes, and we don't need to allocate ctx each time because probe target
point is always same (not different for each field). In this case,
we don't need to allocate/free each time.
>
> Before this change, traceprobe_finish_parse(&ctx) is called for every
> iteration of the loop. Now we only do it when it exits the function.
Yes, but that is not a good way to use the ctx. As same as kprobe and
fprobe events, it is designed to be the same through parsing one probe,
not each field.
For the uprobe case, this is just passing ctx->flags, others are mostly
unused or temporarily used in field parsing. So allocating from stack
frame, it is OK. But allocating from heap, it involves slab allocation
and free each time. I think it is just inefficient.
Hmm, but eprobe seems doing the same mistake. Let me split that part
to fix to keep using the same ctx through parsing one probe.
Thank you,
>
> -- Steve
>
>
> > if (ret)
> > goto error;
> > }
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
© 2016 - 2025 Red Hat, Inc.