linux-kernel - Re: [PATCH] tracing/probes: Fix MAX_TRACE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9997571.eNJFYEL58v@basin>
Date: Sun, 29 Sep 2024 23:17:14 -0400
From: Mikel Rychliski <mikel@...elr.com>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH] tracing/probes: Fix MAX_TRACE_ARGS limit handling

On Sunday, September 29, 2024 7:40:18 P.M. EDT Masami Hiramatsu wrote:
> 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

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