On Sun, 29 Sep 2024 23:17:14 -0400
Mikel Rychliski [off-list ref] wrote:

On Sunday, September 29, 2024 7:40:18 P.M. EDT Masami Hiramatsu wrote:

quoted

Good catch! But this silently drop the arguments after MAX_TRACE_ARGS.
I rather like to reject such input with an error (-E2BIG) as below.
(Hmm, and I also need a new ftracetest test case for this.)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 39877c80d6cb..3f6654127d8c 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c

@@ -2194,6 +2194,9 @@ int trace_probe_create(const char *raw_command, int

(*createfn)(int, const char if (!argv)
                return -ENOMEM;

+       if (argc > MAX_TRACE_ARGS + 2)
+               return -E2BIG;
+
        if (argc)
                ret = createfn(argc, (const char **)argv);

I think the logic still needs to be cleaned up in the individual probe 
implementations (either to count consistently or remove the limit enforcement 
there), otherwise you can get an oops with something like:

	echo "f:testprobe copy_process" arg{1..127}=\$stack "\$arg*" > out
	cat out > /sys/kernel/debug/tracing/dynamic_events

Ah, good catch. Yes, the "$arg*" problem is still there.

BTF argument expansion results in >128 arguments, but we still attempt to 
process the excess unparsed ones.

OK, can you update your version to return an error from each probe?

Thank you,

-- 
Masami Hiramatsu (Google) [off-list ref]