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

Steven Rostedt
2025-07-18T17:39:10Z

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

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Allocate temporary string buffers for fprobe-event from heap
> instead of stack. This fixes the stack frame exceed limit error.
>
> 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_fprobe.c | 39 ++++++++++++++++++++++++++-------------
> 1 file changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
> index 264cf7fc9a1d..fd1036e27309 100644
> --- a/kernel/trace/trace_fprobe.c
> +++ b/kernel/trace/trace_fprobe.c
> @@ -1234,18 +1234,18 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
> * FETCHARG:TYPE : use TYPE instead of unsigned long.
> */
> struct trace_fprobe *tf __free(free_trace_fprobe) = NULL;
> - struct module *mod __free(module_put) = NULL;
> - int i, new_argc = 0, ret = 0;
> - bool is_return = false;
> - char *symbol __free(kfree) = NULL;
> const char *event = NULL, *group = FPROBE_EVENT_SYSTEM;
> + struct module *mod __free(module_put) = NULL;
> const char **new_argv __free(kfree) = NULL;
> - char buf[MAX_EVENT_NAME_LEN];
> - char gbuf[MAX_EVENT_NAME_LEN];
> - char sbuf[KSYM_NAME_LEN];
> - char abuf[MAX_BTF_ARGS_LEN];
> + char *symbol __free(kfree) = NULL;
> + char *ebuf __free(kfree) = NULL;
> + char *gbuf __free(kfree) = NULL;
> + char *sbuf __free(kfree) = NULL;
> + char *abuf __free(kfree) = NULL;
> char *dbuf __free(kfree) = NULL;
> + int i, new_argc = 0, ret = 0;
> bool is_tracepoint = false;
> + bool is_return = false;
>
> if ((argv[0][0] != 'f' && argv[0][0] != 't') || argc < 2)
> return -ECANCELED;
> @@ -1273,6 +1273,9 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
>
> trace_probe_log_set_index(0);
> if (event) {
> + gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
> + if (!gbuf)
> + return -ENOMEM;
> ret = traceprobe_parse_event_name(&event, &group, gbuf,
> event - argv[0]);
> if (ret)
> @@ -1280,15 +1283,18 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
> }
>
> if (!event) {
> + ebuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);

ebuf and gbuf are used with the same length. Why not just keep them
using the same buffer? It worked before this patch, it should work
after too.

buf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
if (!buf)
return -ENOMEM;

if (event) {
[..]
}

if (!event) {
[..]
}

And not require two different variables that will add two exit codes
when one would do.

-- Steve

> + if (!ebuf)
> + return -ENOMEM;
> /* Make a new event name */
> if (is_tracepoint)
> - snprintf(buf, MAX_EVENT_NAME_LEN, "%s%s",
> + snprintf(ebuf, MAX_EVENT_NAME_LEN, "%s%s",
> isdigit(*symbol) ? "_" : "", symbol);
> else
> - snprintf(buf, MAX_EVENT_NAME_LEN, "%s__%s", symbol,
> + snprintf(ebuf, MAX_EVENT_NAME_LEN, "%s__%s", symbol,
> is_return ? "exit" : "entry");
> - sanitize_event_name(buf);
> - event = buf;
> + sanitize_event_name(ebuf);
> + event = ebuf;
> }
>
> if (is_return)
> @@ -1304,13 +1310,20 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
> ctx->flags |= TPARG_FL_TPOINT;
> mod = NULL;
> tpoint = find_tracepoint(symbol, &mod);
> - if (tpoint)
> + if (tpoint) {
> + sbuf = kmalloc(KSYM_NAME_LEN, GFP_KERNEL);
> + if (!sbuf)
> + return -ENOMEM;
> ctx->funcname = kallsyms_lookup((unsigned long)tpoint->probestub,
> NULL, NULL, NULL, sbuf);
> + }
> }
> if (!ctx->funcname)
> ctx->funcname = symbol;
>
> + abuf = kmalloc(MAX_BTF_ARGS_LEN, GFP_KERNEL);
> + if (!abuf)
> + return -ENOMEM;
> argc -= 2; argv += 2;
> new_argv = traceprobe_expand_meta_args(argc, argv, &new_argc,
> abuf, MAX_BTF_ARGS_LEN, ctx);