| Message ID | 20230703092336.268371-1-zegao@tencent.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Masami Hiramatsu |
| Headers | show |
| Series | [v2] fprobe: add unlock to match a succeeded ftrace_test_recursion_trylock | expand |
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 >
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 >
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); > } >
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); > > } > > >
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); }
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(-)