Messages in this thread | | | From | Ze Gao <> | Date | Tue, 16 May 2023 11:02:27 +0800 | Subject | Re: [PATCH 2/4] fprobe: make fprobe_kprobe_handler recursion free |
| |
Hi Steven,
On Tue, May 16, 2023 at 9:25 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Mon, 15 May 2023 11:26:39 +0800 > Ze Gao <zegao2021@gmail.com> wrote: > > > Current implementation calls kprobe related functions before doing > > ftrace recursion check in fprobe_kprobe_handler, which opens door > > to kernel crash due to stack recursion if preempt_count_{add, sub} > > is traceable. > > > > Refactor the common part out of fprobe_kprobe_handler and fprobe_ > > handler and call ftrace recursion detection at the very beginning, > > and also mark these functions notrace so that the whole fprobe_k- > > probe_handler is free from recusion. And > > > > Signed-off-by: Ze Gao <zegao@tencent.com> > > --- > > kernel/trace/fprobe.c | 61 +++++++++++++++++++++++++++++++------------ > > 1 file changed, 45 insertions(+), 16 deletions(-) > > > > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c > > index 9abb3905bc8e..ad9a36c87ad9 100644 > > --- a/kernel/trace/fprobe.c > > +++ b/kernel/trace/fprobe.c > > @@ -20,30 +20,22 @@ struct fprobe_rethook_node { > > char data[]; > > }; > > > > -static void fprobe_handler(unsigned long ip, unsigned long parent_ip, > > - struct ftrace_ops *ops, struct ftrace_regs *fregs) > > +static inline notrace void __fprobe_handler(unsigned long ip, unsigned long > > > FYI, if you look in kernel/trace/Makefile you'll see: > > ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE) > > Which removes the flags to add tracing. So there's no reason to add > "notrace" here, as all functions in this directory are by default "notrace".
Thanks for your valuable info, which I missed before. I'll send v2 to remove those unnecessary notrace annotations, and use the same trick for rethook too.
BTW, I think we can mark rethook routines decls notrace in include/linux/rethook.h, which helps to remind developers of other arch(s) this important info. What do you think of it?
Regards, Ze
| |