Received-SPF: pass (google.com: domain of linux-kernel+bounces-86347-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 29 Feb 2024 17:52:16 +0900
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Cc: Jiri Olsa <olsajiri@gmail.com>, Steven Rostedt <rostedt@goodmis.org>,
 linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
 linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v2 0/7] tracing/probes: Support function parameter
 access from return probe
Message-Id: <20240229175216.96bb2e16b510f81e3802ef23@kernel.org>
In-Reply-To: <20240229161320.978190f42dcc1a521c192e7d@kernel.org>
References: <170891987362.609861.6767830614537418260.stgit@devnote2>
	<Zd9eBn2FTQzYyg7L@krava>
	<20240229145139.a215085c44add62302463312@kernel.org>
	<20240229153855.6fe3fb454cf56eebc6ea9953@kernel.org>
	<20240229161320.978190f42dcc1a521c192e7d@kernel.org>
Precedence: bulk
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

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

> On Thu, 29 Feb 2024 15:38:55 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.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@kernel.org>