From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Allocate temporary string buffers for parsing eprobe-events
from heap instead of stack.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace_eprobe.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index 1e18a8619b40..75d8208cd859 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -9,6 +9,7 @@
* Copyright (C) 2021, VMware Inc, Tzvetomir Stoyanov tz.stoyanov@gmail.com>
*
*/
+#include <linux/cleanup.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/ftrace.h>
@@ -871,10 +872,10 @@ static int __trace_eprobe_create(int argc, const char *argv[])
const char *event = NULL, *group = EPROBE_EVENT_SYSTEM;
const char *sys_event = NULL, *sys_name = NULL;
struct trace_event_call *event_call;
+ char *buf1 __free(kfree) = NULL;
+ char *buf2 __free(kfree) = NULL;
+ char *gbuf __free(kfree) = NULL;
struct trace_eprobe *ep = NULL;
- char buf1[MAX_EVENT_NAME_LEN];
- char buf2[MAX_EVENT_NAME_LEN];
- char gbuf[MAX_EVENT_NAME_LEN];
int ret = 0, filter_idx = 0;
int i, filter_cnt;
@@ -885,6 +886,11 @@ static int __trace_eprobe_create(int argc, const char *argv[])
event = strchr(&argv[0][1], ':');
if (event) {
+ gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
+ if (!gbuf) {
+ ret = -ENOMEM;
+ goto parse_error;
+ }
event++;
ret = traceprobe_parse_event_name(&event, &group, gbuf,
event - argv[0]);
@@ -894,6 +900,12 @@ static int __trace_eprobe_create(int argc, const char *argv[])
trace_probe_log_set_index(1);
sys_event = argv[1];
+
+ buf2 = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
+ if (!buf2) {
+ ret = -ENOMEM;
+ goto parse_error;
+ }
ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, 0);
if (ret || !sys_event || !sys_name) {
trace_probe_log_err(0, NO_EVENT_INFO);
@@ -901,7 +913,11 @@ static int __trace_eprobe_create(int argc, const char *argv[])
}
if (!event) {
- strscpy(buf1, sys_event, MAX_EVENT_NAME_LEN);
+ buf1 = kstrdup(sys_event, GFP_KERNEL);
+ if (!buf1) {
+ ret = -ENOMEM;
+ goto error;
+ }
event = buf1;
}
On Fri, 18 Jul 2025 20:34:40 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Allocate temporary string buffers for parsing eprobe-events > from heap instead of stack. > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > --- > kernel/trace/trace_eprobe.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c > index 1e18a8619b40..75d8208cd859 100644 > --- a/kernel/trace/trace_eprobe.c > +++ b/kernel/trace/trace_eprobe.c > @@ -9,6 +9,7 @@ > * Copyright (C) 2021, VMware Inc, Tzvetomir Stoyanov tz.stoyanov@gmail.com> > * > */ > +#include <linux/cleanup.h> > #include <linux/module.h> > #include <linux/mutex.h> > #include <linux/ftrace.h> > @@ -871,10 +872,10 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > const char *event = NULL, *group = EPROBE_EVENT_SYSTEM; > const char *sys_event = NULL, *sys_name = NULL; > struct trace_event_call *event_call; > + char *buf1 __free(kfree) = NULL; > + char *buf2 __free(kfree) = NULL; > + char *gbuf __free(kfree) = NULL; > struct trace_eprobe *ep = NULL; > - char buf1[MAX_EVENT_NAME_LEN]; > - char buf2[MAX_EVENT_NAME_LEN]; > - char gbuf[MAX_EVENT_NAME_LEN]; > int ret = 0, filter_idx = 0; > int i, filter_cnt; > > @@ -885,6 +886,11 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > > event = strchr(&argv[0][1], ':'); > if (event) { > + gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL); > + if (!gbuf) { > + ret = -ENOMEM; > + goto parse_error; > + } > event++; > ret = traceprobe_parse_event_name(&event, &group, gbuf, > event - argv[0]); > @@ -894,6 +900,12 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > > trace_probe_log_set_index(1); > sys_event = argv[1]; > + > + buf2 = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL); > + if (!buf2) { > + ret = -ENOMEM; > + goto parse_error; > + } > ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, 0); > if (ret || !sys_event || !sys_name) { > trace_probe_log_err(0, NO_EVENT_INFO); > @@ -901,7 +913,11 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > } > > if (!event) { > - strscpy(buf1, sys_event, MAX_EVENT_NAME_LEN); > + buf1 = kstrdup(sys_event, GFP_KERNEL); > + if (!buf1) { > + ret = -ENOMEM; > + goto error; > + } I kinda hate all these updating of "ret" before jumping to error. > event = buf1; > } > What about this: -- Steve diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c index 916555f0de81..48cc1079a1dd 100644 --- a/kernel/trace/trace_eprobe.c +++ b/kernel/trace/trace_eprobe.c @@ -9,6 +9,7 @@ * Copyright (C) 2021, VMware Inc, Tzvetomir Stoyanov tz.stoyanov@gmail.com> * */ +#include <linux/cleanup.h> #include <linux/module.h> #include <linux/mutex.h> #include <linux/ftrace.h> @@ -869,10 +870,10 @@ static int __trace_eprobe_create(int argc, const char *argv[]) const char *event = NULL, *group = EPROBE_EVENT_SYSTEM; const char *sys_event = NULL, *sys_name = NULL; struct trace_event_call *event_call; + char *buf1 __free(kfree) = NULL; + char *buf2 __free(kfree) = NULL; + char *gbuf __free(kfree) = NULL; struct trace_eprobe *ep = NULL; - char buf1[MAX_EVENT_NAME_LEN]; - char buf2[MAX_EVENT_NAME_LEN]; - char gbuf[MAX_EVENT_NAME_LEN]; int ret = 0, filter_idx = 0; int i, filter_cnt; @@ -883,6 +884,9 @@ static int __trace_eprobe_create(int argc, const char *argv[]) event = strchr(&argv[0][1], ':'); if (event) { + gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL); + if (!gbuf) + goto mem_error; event++; ret = traceprobe_parse_event_name(&event, &group, gbuf, event - argv[0]); @@ -892,6 +896,10 @@ static int __trace_eprobe_create(int argc, const char *argv[]) trace_probe_log_set_index(1); sys_event = argv[1]; + + buf2 = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL); + if (!buf2) + goto mem_error; ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, 0); if (ret || !sys_event || !sys_name) { trace_probe_log_err(0, NO_EVENT_INFO); @@ -899,7 +907,9 @@ static int __trace_eprobe_create(int argc, const char *argv[]) } if (!event) { - strscpy(buf1, sys_event, MAX_EVENT_NAME_LEN); + buf1 = kstrdup(sys_event, GFP_KERNEL); + if (!buf1) + goto mem_error; event = buf1; } @@ -972,6 +982,9 @@ static int __trace_eprobe_create(int argc, const char *argv[]) trace_probe_log_clear(); return ret; +mem_error: + ret = -ENOMEM; + goto error; parse_error: ret = -EINVAL; error:
On Fri, 18 Jul 2025 13:55:49 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 18 Jul 2025 20:34:40 +0900 > "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > Allocate temporary string buffers for parsing eprobe-events > > from heap instead of stack. > > > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > --- > > kernel/trace/trace_eprobe.c | 24 ++++++++++++++++++++---- > > 1 file changed, 20 insertions(+), 4 deletions(-) > > > > diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c > > index 1e18a8619b40..75d8208cd859 100644 > > --- a/kernel/trace/trace_eprobe.c > > +++ b/kernel/trace/trace_eprobe.c > > @@ -9,6 +9,7 @@ > > * Copyright (C) 2021, VMware Inc, Tzvetomir Stoyanov tz.stoyanov@gmail.com> > > * > > */ > > +#include <linux/cleanup.h> > > #include <linux/module.h> > > #include <linux/mutex.h> > > #include <linux/ftrace.h> > > @@ -871,10 +872,10 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > > const char *event = NULL, *group = EPROBE_EVENT_SYSTEM; > > const char *sys_event = NULL, *sys_name = NULL; > > struct trace_event_call *event_call; > > + char *buf1 __free(kfree) = NULL; > > + char *buf2 __free(kfree) = NULL; > > + char *gbuf __free(kfree) = NULL; > > struct trace_eprobe *ep = NULL; > > - char buf1[MAX_EVENT_NAME_LEN]; > > - char buf2[MAX_EVENT_NAME_LEN]; > > - char gbuf[MAX_EVENT_NAME_LEN]; > > int ret = 0, filter_idx = 0; > > int i, filter_cnt; > > > > @@ -885,6 +886,11 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > > > > event = strchr(&argv[0][1], ':'); > > if (event) { > > + gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL); > > + if (!gbuf) { > > + ret = -ENOMEM; > > + goto parse_error; > > + } > > event++; > > ret = traceprobe_parse_event_name(&event, &group, gbuf, > > event - argv[0]); > > @@ -894,6 +900,12 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > > > > trace_probe_log_set_index(1); > > sys_event = argv[1]; > > + > > + buf2 = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL); > > + if (!buf2) { > > + ret = -ENOMEM; > > + goto parse_error; > > + } > > ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, 0); > > if (ret || !sys_event || !sys_name) { > > trace_probe_log_err(0, NO_EVENT_INFO); > > @@ -901,7 +913,11 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > > } > > > > if (!event) { > > - strscpy(buf1, sys_event, MAX_EVENT_NAME_LEN); > > + buf1 = kstrdup(sys_event, GFP_KERNEL); > > + if (!buf1) { > > + ret = -ENOMEM; > > + goto error; > > + } > > I kinda hate all these updating of "ret" before jumping to error. Agreed. > > > event = buf1; > > } > > > > What about this: Looks good to me. Note that eventually I will use cleanup.h to remove gotos from this function as same as other probes too. Anyway, in this series I will use the below code. Thank you! > > -- Steve > > diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c > index 916555f0de81..48cc1079a1dd 100644 > --- a/kernel/trace/trace_eprobe.c > +++ b/kernel/trace/trace_eprobe.c > @@ -9,6 +9,7 @@ > * Copyright (C) 2021, VMware Inc, Tzvetomir Stoyanov tz.stoyanov@gmail.com> > * > */ > +#include <linux/cleanup.h> > #include <linux/module.h> > #include <linux/mutex.h> > #include <linux/ftrace.h> > @@ -869,10 +870,10 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > const char *event = NULL, *group = EPROBE_EVENT_SYSTEM; > const char *sys_event = NULL, *sys_name = NULL; > struct trace_event_call *event_call; > + char *buf1 __free(kfree) = NULL; > + char *buf2 __free(kfree) = NULL; > + char *gbuf __free(kfree) = NULL; > struct trace_eprobe *ep = NULL; > - char buf1[MAX_EVENT_NAME_LEN]; > - char buf2[MAX_EVENT_NAME_LEN]; > - char gbuf[MAX_EVENT_NAME_LEN]; > int ret = 0, filter_idx = 0; > int i, filter_cnt; > > @@ -883,6 +884,9 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > > event = strchr(&argv[0][1], ':'); > if (event) { > + gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL); > + if (!gbuf) > + goto mem_error; > event++; > ret = traceprobe_parse_event_name(&event, &group, gbuf, > event - argv[0]); > @@ -892,6 +896,10 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > > trace_probe_log_set_index(1); > sys_event = argv[1]; > + > + buf2 = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL); > + if (!buf2) > + goto mem_error; > ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, 0); > if (ret || !sys_event || !sys_name) { > trace_probe_log_err(0, NO_EVENT_INFO); > @@ -899,7 +907,9 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > } > > if (!event) { > - strscpy(buf1, sys_event, MAX_EVENT_NAME_LEN); > + buf1 = kstrdup(sys_event, GFP_KERNEL); > + if (!buf1) > + goto mem_error; > event = buf1; > } > > @@ -972,6 +982,9 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > trace_probe_log_clear(); > return ret; > > +mem_error: > + ret = -ENOMEM; > + goto error; > parse_error: > ret = -EINVAL; > error: -- Masami Hiramatsu (Google) <mhiramat@kernel.org>
© 2016 - 2025 Red Hat, Inc.