linux-kernel - Re: [PATCH v2 0/7] tracing/probes: Support function parameter access from return probe

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: <20240229175216.96bb2e16b510f81e3802ef23@kernel.org>
Date: Thu, 29 Feb 2024 17:52:16 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Masami Hiramatsu (Google) <mhiramat@...nel.org>
Cc: Jiri Olsa <olsajiri@...il.com>, Steven Rostedt <rostedt@...dmis.org>,
 linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v2 0/7] tracing/probes: Support function parameter
 access from return probe

On Thu, 29 Feb 2024 16:13:20 +0900
Masami Hiramatsu (Google) <mhiramat@...nel.org> wrote:

> On Thu, 29 Feb 2024 15:38:55 +0900
> Masami Hiramatsu (Google) <mhiramat@...nel.org> wrote:
> 
> > Hmm, this seems arch_rethook_trampoline caused the issue.
> > 
> > And curiously, it depends on the number of stored data.
> > 
> > OK:
> > /sys/kernel/tracing # echo 'f vfs_read%return $arg1 $arg2 $arg3' >> dynamic_events 
> > /sys/kernel/tracing # echo 1 > events/fprobes/enable 
> > 
> > NG:
> > /sys/kernel/tracing # echo 'f vfs_read%return $arg1 $arg2 $arg3 $arg4' >> dynamic_events 
> > /sys/kernel/tracing # echo 1 > events/fprobes/enable 
> > 
> > I also confirmed that on 'vfs_write' caused the same result. 3 arguments(24 bytes) is OK,
> > but 4 arguments (32bytes) is NG.
> 
> And this may be the fprobe bug. kretprobe events doesn't show this issue.
> 
> OK:
> /sys/kernel/tracing # echo 'r vfs_read $arg*' >> kprobe_events 
> /sys/kernel/tracing # echo 1 > events/kprobes/enable 
> 
> But this is strange because both uses same rethook...

Lol, I haven't allocate the entry data size when initialize rethook.
That's a bug.
Please try below.

diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 6cd2a4e3afb8..9ff018245840 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -189,9 +189,6 @@ static int fprobe_init_rethook(struct fprobe *fp, int num)
 {
 	int size;
 
-	if (num <= 0)
-		return -EINVAL;
-
 	if (!fp->exit_handler) {
 		fp->rethook = NULL;
 		return 0;
@@ -199,15 +196,16 @@ static int fprobe_init_rethook(struct fprobe *fp, int num)
 
 	/* Initialize rethook if needed */
 	if (fp->nr_maxactive)
-		size = fp->nr_maxactive;
+		num = fp->nr_maxactive;
 	else
-		size = num * num_possible_cpus() * 2;
-	if (size <= 0)
+		num *= num_possible_cpus() * 2;
+	if (num <= 0)
 		return -EINVAL;
 
+	size = sizeof(struct fprobe_rethook_node) + fp->entry_data_size;
+
 	/* Initialize rethook */
-	fp->rethook = rethook_alloc((void *)fp, fprobe_exit_handler,
-				sizeof(struct fprobe_rethook_node), size);
+	fp->rethook = rethook_alloc((void *)fp, fprobe_exit_handler, size, num);
 	if (IS_ERR(fp->rethook))
 		return PTR_ERR(fp->rethook);
 

-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>