Re: [PATCH 4/5] tracing: eprobe-event: Allocate string buffers from heap
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Date: 2025-07-19 01:12:00
Also in:
lkml
On Fri, 18 Jul 2025 13:55:49 -0400 Steven Rostedt [off-list ref] wrote:
On Fri, 18 Jul 2025 20:34:40 +0900 "Masami Hiramatsu (Google)" [off-list ref] wrote:quoted
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.
quoted
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!
quoted hunk ↗ jump to hunk
-- Stevediff --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) [off-list ref]