* [PATCH v2] fprobe: add unlock to match a succeeded ftrace_test_recursion_trylock
@ 2023-07-03 9:23 Ze Gao
2023-07-03 10:01 ` Masami Hiramatsu
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Ze Gao @ 2023-07-03 9:23 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Ze Gao, linux-kernel, linux-trace-kernel, Ze Gao,
Yafang
Unlock ftrace recursion lock when fprobe_kprobe_handler() is failed
because of some running kprobe.
Fixes: 3cc4e2c5fbae ("fprobe: make fprobe_kprobe_handler recursion free")
Reported-by: Yafang <laoar.shao@gmail.com>
Closes: https://lore.kernel.org/linux-trace-kernel/CALOAHbC6UpfFOOibdDiC7xFc5YFUgZnk3MZ=3Ny6we=AcrNbew@mail.gmail.com/
Signed-off-by: Ze Gao <zegao@tencent.com>
---
kernel/trace/fprobe.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 18d36842faf5..93b3e361bb97 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -102,12 +102,14 @@ static void fprobe_kprobe_handler(unsigned long ip, unsigned long parent_ip,
if (unlikely(kprobe_running())) {
fp->nmissed++;
- return;
+ goto recursion_unlock;
}
kprobe_busy_begin();
__fprobe_handler(ip, parent_ip, ops, fregs);
kprobe_busy_end();
+
+recursion_unlock:
ftrace_test_recursion_unlock(bit);
}
--
2.40.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] fprobe: add unlock to match a succeeded ftrace_test_recursion_trylock
2023-07-03 9:23 [PATCH v2] fprobe: add unlock to match a succeeded ftrace_test_recursion_trylock Ze Gao
@ 2023-07-03 10:01 ` Masami Hiramatsu
2023-07-03 13:44 ` Yafang Shao
2023-07-06 16:09 ` Steven Rostedt
2 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2023-07-03 10:01 UTC (permalink / raw)
To: Ze Gao; +Cc: Steven Rostedt, linux-kernel, linux-trace-kernel, Ze Gao, Yafang
On Mon, 3 Jul 2023 17:23:36 +0800
Ze Gao <zegao2021@gmail.com> wrote:
> Unlock ftrace recursion lock when fprobe_kprobe_handler() is failed
> because of some running kprobe.
>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks!
> Fixes: 3cc4e2c5fbae ("fprobe: make fprobe_kprobe_handler recursion free")
> Reported-by: Yafang <laoar.shao@gmail.com>
> Closes: https://lore.kernel.org/linux-trace-kernel/CALOAHbC6UpfFOOibdDiC7xFc5YFUgZnk3MZ=3Ny6we=AcrNbew@mail.gmail.com/
> Signed-off-by: Ze Gao <zegao@tencent.com>
> ---
> kernel/trace/fprobe.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 18d36842faf5..93b3e361bb97 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -102,12 +102,14 @@ static void fprobe_kprobe_handler(unsigned long ip, unsigned long parent_ip,
>
> if (unlikely(kprobe_running())) {
> fp->nmissed++;
> - return;
> + goto recursion_unlock;
> }
>
> kprobe_busy_begin();
> __fprobe_handler(ip, parent_ip, ops, fregs);
> kprobe_busy_end();
> +
> +recursion_unlock:
> ftrace_test_recursion_unlock(bit);
> }
>
> --
> 2.40.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] fprobe: add unlock to match a succeeded ftrace_test_recursion_trylock
2023-07-03 9:23 [PATCH v2] fprobe: add unlock to match a succeeded ftrace_test_recursion_trylock Ze Gao
2023-07-03 10:01 ` Masami Hiramatsu
@ 2023-07-03 13:44 ` Yafang Shao
2023-07-06 16:09 ` Steven Rostedt
2 siblings, 0 replies; 5+ messages in thread
From: Yafang Shao @ 2023-07-03 13:44 UTC (permalink / raw)
To: Ze Gao
Cc: Masami Hiramatsu, Steven Rostedt, linux-kernel,
linux-trace-kernel, Ze Gao
On Mon, Jul 3, 2023 at 5:23 PM Ze Gao <zegao2021@gmail.com> wrote:
>
> Unlock ftrace recursion lock when fprobe_kprobe_handler() is failed
> because of some running kprobe.
>
> Fixes: 3cc4e2c5fbae ("fprobe: make fprobe_kprobe_handler recursion free")
> Reported-by: Yafang <laoar.shao@gmail.com>
> Closes: https://lore.kernel.org/linux-trace-kernel/CALOAHbC6UpfFOOibdDiC7xFc5YFUgZnk3MZ=3Ny6we=AcrNbew@mail.gmail.com/
> Signed-off-by: Ze Gao <zegao@tencent.com>
Acked-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> kernel/trace/fprobe.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 18d36842faf5..93b3e361bb97 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -102,12 +102,14 @@ static void fprobe_kprobe_handler(unsigned long ip, unsigned long parent_ip,
>
> if (unlikely(kprobe_running())) {
> fp->nmissed++;
> - return;
> + goto recursion_unlock;
> }
>
> kprobe_busy_begin();
> __fprobe_handler(ip, parent_ip, ops, fregs);
> kprobe_busy_end();
> +
> +recursion_unlock:
> ftrace_test_recursion_unlock(bit);
> }
>
> --
> 2.40.1
>
--
Regards
Yafang
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] fprobe: add unlock to match a succeeded ftrace_test_recursion_trylock
2023-07-03 9:23 [PATCH v2] fprobe: add unlock to match a succeeded ftrace_test_recursion_trylock Ze Gao
2023-07-03 10:01 ` Masami Hiramatsu
2023-07-03 13:44 ` Yafang Shao
@ 2023-07-06 16:09 ` Steven Rostedt
2023-07-07 0:15 ` Masami Hiramatsu
2 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2023-07-06 16:09 UTC (permalink / raw)
To: Ze Gao; +Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel, Ze Gao,
Yafang
On Mon, 3 Jul 2023 17:23:36 +0800
Ze Gao <zegao2021@gmail.com> wrote:
> Unlock ftrace recursion lock when fprobe_kprobe_handler() is failed
> because of some running kprobe.
>
> Fixes: 3cc4e2c5fbae ("fprobe: make fprobe_kprobe_handler recursion free")
> Reported-by: Yafang <laoar.shao@gmail.com>
> Closes: https://lore.kernel.org/linux-trace-kernel/CALOAHbC6UpfFOOibdDiC7xFc5YFUgZnk3MZ=3Ny6we=AcrNbew@mail.gmail.com/
> Signed-off-by: Ze Gao <zegao@tencent.com>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> kernel/trace/fprobe.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 18d36842faf5..93b3e361bb97 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -102,12 +102,14 @@ static void fprobe_kprobe_handler(unsigned long ip, unsigned long parent_ip,
>
> if (unlikely(kprobe_running())) {
Off topic for this patch, but Masami, what's the purpose of not calling the
fprobe when a kprobe is running? Does that mean it has probed another kprobe?
Probably could add a comment here to explain the issue.
-- Steve
> fp->nmissed++;
> - return;
> + goto recursion_unlock;
> }
>
> kprobe_busy_begin();
> __fprobe_handler(ip, parent_ip, ops, fregs);
> kprobe_busy_end();
> +
> +recursion_unlock:
> ftrace_test_recursion_unlock(bit);
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] fprobe: add unlock to match a succeeded ftrace_test_recursion_trylock
2023-07-06 16:09 ` Steven Rostedt
@ 2023-07-07 0:15 ` Masami Hiramatsu
0 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2023-07-07 0:15 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ze Gao, Masami Hiramatsu, linux-kernel, linux-trace-kernel,
Ze Gao, Yafang
On Thu, 6 Jul 2023 12:09:16 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 3 Jul 2023 17:23:36 +0800
> Ze Gao <zegao2021@gmail.com> wrote:
>
> > Unlock ftrace recursion lock when fprobe_kprobe_handler() is failed
> > because of some running kprobe.
> >
> > Fixes: 3cc4e2c5fbae ("fprobe: make fprobe_kprobe_handler recursion free")
> > Reported-by: Yafang <laoar.shao@gmail.com>
> > Closes: https://lore.kernel.org/linux-trace-kernel/CALOAHbC6UpfFOOibdDiC7xFc5YFUgZnk3MZ=3Ny6we=AcrNbew@mail.gmail.com/
> > Signed-off-by: Ze Gao <zegao@tencent.com>
>
> Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>
> > ---
> > kernel/trace/fprobe.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> > index 18d36842faf5..93b3e361bb97 100644
> > --- a/kernel/trace/fprobe.c
> > +++ b/kernel/trace/fprobe.c
> > @@ -102,12 +102,14 @@ static void fprobe_kprobe_handler(unsigned long ip, unsigned long parent_ip,
> >
> > if (unlikely(kprobe_running())) {
>
> Off topic for this patch, but Masami, what's the purpose of not calling the
> fprobe when a kprobe is running? Does that mean it has probed another kprobe?
This is for the user who is sharing their handler with kprobes (like eBPF),
which may expect that the handler is not called recursively. (e.g. an interrupt
happens while kprobe handler is running and that interrupt calls a function
which is fprobed)
>
> Probably could add a comment here to explain the issue.
OK, it is also documented in Documentation/trace/fprobe.rst, but it is better
to comment in the code too.
Thanks,
>
> -- Steve
>
>
> > fp->nmissed++;
> > - return;
> > + goto recursion_unlock;
> > }
> >
> > kprobe_busy_begin();
> > __fprobe_handler(ip, parent_ip, ops, fregs);
> > kprobe_busy_end();
> > +
> > +recursion_unlock:
> > ftrace_test_recursion_unlock(bit);
> > }
> >
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-07 0:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-03 9:23 [PATCH v2] fprobe: add unlock to match a succeeded ftrace_test_recursion_trylock Ze Gao
2023-07-03 10:01 ` Masami Hiramatsu
2023-07-03 13:44 ` Yafang Shao
2023-07-06 16:09 ` Steven Rostedt
2023-07-07 0:15 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).