Hi Fernando,

I found the patch from Mikel solves the same issue and his v2 patch fixes
wider situation ("$arg*" BTF argument case). So I decided to pick his patch.

https://lore.kernel.org/all/20240930202656.292869-1-mikel@mikelr.com/ (local)

Let me know if there is any case the above fix does not cover.

Thank you,


On Tue, 22 Oct 2024 14:40:45 +0900
Masami Hiramatsu (Google) [off-list ref] wrote:

Hi,

On Thu, 5 Sep 2024 00:19:16 +0200
"Fernando F. Mancera" [off-list ref] wrote:

quoted

Hi,

On 26/08/2024 01:56, Masami Hiramatsu (Google) wrote:

quoted

On Sun, 25 Aug 2024 19:06:22 +0200
"Fernando F. Mancera" [off-list ref] wrote:

quoted

Hi,

On 25/08/2024 09:41, Masami Hiramatsu (Google) wrote:

quoted

Hi,

On Tue, 13 Aug 2024 13:25:40 -0400
Fernando Fernandez Mancera [off-list ref] wrote:

quoted

When initializing trace_probes::nr_args, make sure the maximum number of
probe arguments is honored. Oherwise, we can hit a NULL pointer
dereferences in multiple situations like on traceprobe_set_print_fmt().

Link: https://bugzilla.redhat.com/2303876

Sorry for replying later. I'm not sure why but I did not found this in my mbox...

No worries!

quoted

Anyway, trace_probe_init() should return -E2BIG in this case because
it is actuall wrong value.

Can you update your patch?

I agree this is the right solution but it would mean a change in
behavior. Is that fine? Before merging 035ba76014c0, it was writing up
to the maximum number of arguments and ignoring the rest. If you confirm
it is fine to change the behavior, I will update the patch.

Yes, I think this is the better behavior for users too, rather than
silently droping some arguments.
We also need to handle this error code in callsite and log an error,
and add a testcase in the syntax error testcase. But that should be done
in another series. We should fix this first.

Just in case this fell through the cracks, I've sent a V2 patch. In 
addition I also have a series ready for the logging and syntax error 
testcase in kselftest but I didn't send it because it requires this 
patch to be applied.

I think that v2 was lost in my mailbox, but I found that in patchwork.
Let me pick it.

Thanks, 

quoted

Thank you,
Fernando.

quoted

Thank you,

quoted

Thank you!

quoted

Thank you,

quoted

Fixes: 035ba76014c0 ("tracing/probes: cleanup: Set trace_probe::nr_args at trace_probe_init")
Signed-off-by: Fernando Fernandez Mancera <redacted>
---
   kernel/trace/trace_probe.c | 8 ++++++--
   1 file changed, 6 insertions(+), 2 deletions(-)

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

@@ -2043,10 +2043,14 @@ int trace_probe_init(struct trace_probe *tp, const char *event,
   		goto error;
   	}
   
-	tp->nr_args = nargs;
+	if (nargs > MAX_TRACE_ARGS)
+		tp->nr_args = MAX_TRACE_ARGS;
+	else
+		tp->nr_args = nargs;
+
   	/* Make sure pointers in args[] are NULL */
   	if (nargs)
-		memset(tp->args, 0, sizeof(tp->args[0]) * nargs);
+		memset(tp->args, 0, sizeof(tp->args[0]) * tp->nr_args);
   
   	return 0;
   

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

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