Re: [PATCH 2/4] fprobe: make fprobe_kprobe_handler recursion free

[Ze Gao]

Hi Steven,

On Tue, May 16, 2023 at 9:25 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Mon, 15 May 2023 11:26:39 +0800
> Ze Gao <zegao2021@xxxxxxxxx> 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@xxxxxxxxxxx>
> > ---
> >  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