Re: [PATCH 5/5] tracing: uprobe-event: Allocate string buffers from heap

Steven Rostedt
2025-07-18T17:58:55Z

On Fri, 18 Jul 2025 20:34:51 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Allocate temporary string buffers for parsing uprobe-events
> from heap instead of stack.
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> kernel/trace/trace_uprobe.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 1fd479718d03..17124769e254 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -7,6 +7,7 @@
> */
> #define pr_fmt(fmt) "trace_uprobe: " fmt
>
> +#include <linux/cleanup.h>
> #include <linux/bpf-cgroup.h>
> #include <linux/security.h>
> #include <linux/ctype.h>
> @@ -19,6 +20,7 @@
> #include <linux/filter.h>
> #include <linux/percpu.h>
>
> +#include "trace.h"
> #include "trace_dynevent.h"
> #include "trace_probe.h"
> #include "trace_probe_tmpl.h"
> @@ -538,15 +540,15 @@ 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;
> - char buf[MAX_EVENT_NAME_LEN];
> - char gbuf[MAX_EVENT_NAME_LEN];
> - enum probe_print_type ptype;
> - struct path path;
> unsigned long offset, ref_ctr_offset;
> + char *gbuf __free(kfree) = NULL;
> + char *buf __free(kfree) = NULL;
> + enum probe_print_type ptype;
> + struct trace_uprobe *tu;
> bool is_return = false;
> + struct path path;
> int i, ret;
>
> ref_ctr_offset = 0;
> @@ -654,6 +656,11 @@ static int __trace_uprobe_create(int argc, const char **argv)
> /* setup a probe */
> trace_probe_log_set_index(0);
> if (event) {
> + gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
> + if (!gbuf) {
> + ret = -ENOMEM;
> + goto fail_address_parse;
> + }
> ret = traceprobe_parse_event_name(&event, &group, gbuf,
> event - argv[0]);
> if (ret)
> @@ -674,6 +681,11 @@ static int __trace_uprobe_create(int argc, const char **argv)
> if (ptr)
> *ptr = '\0';
>
> + buf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto fail_address_parse;
> + }
> snprintf(buf, MAX_EVENT_NAME_LEN, "%c_%s_0x%lx", 'p', tail, offset);
> event = buf;
> kfree(tail);

You could easily do the same thing as I mentioned in my reply to patch 4:

if (!buf)
goto fail_mem;

error:
free_trace_uprobe(tu);
out:
trace_probe_log_clear();
return ret;

fail_mem:
ret = -ENOMEM;
fail_address_parse:
trace_probe_log_clear();
path_put(&path);
kfree(filename);

return ret;
}

-- Steve