| Message ID | 164821817332.2373735.12048266953420821089.stgit@devnote2 (mailing list archive) |
|---|---|
| Headers | show |
| Series | kprobes: rethook: x86: Replace kretprobe trampoline with rethook | expand |
On Fri, Mar 25, 2022 at 11:22:53PM +0900, Masami Hiramatsu wrote: > Masami Hiramatsu (3): > kprobes: Use rethook for kretprobe if possible > rethook: kprobes: x86: Replace kretprobe with rethook on x86 > x86,kprobes: Fix optprobe trampoline to generate complete pt_regs > > Peter Zijlstra (1): > Subject: x86,rethook: Fix arch_rethook_trampoline() to generate a complete pt_regs You fat-fingered the subject there ^ Other than that: Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Hopefully the ftrace return trampoline can also be switched over..
On Fri, Mar 25, 2022 at 7:43 AM Peter Zijlstra <peterz@infradead.org> wrote: > > On Fri, Mar 25, 2022 at 11:22:53PM +0900, Masami Hiramatsu wrote: > > > Masami Hiramatsu (3): > > kprobes: Use rethook for kretprobe if possible > > rethook: kprobes: x86: Replace kretprobe with rethook on x86 > > x86,kprobes: Fix optprobe trampoline to generate complete pt_regs > > > > Peter Zijlstra (1): > > Subject: x86,rethook: Fix arch_rethook_trampoline() to generate a complete pt_regs > > You fat-fingered the subject there ^ > > Other than that: > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > Hopefully the ftrace return trampoline can also be switched over.. Thanks Peter. What's an ETA on landing endbr set? Did I miss a pull req? I see an odd error in linux-next with bpf selftests which may or may not be related. Planning to debug it when everything settles in Linus's tree. Masami, could you do another respin? Also do you mind squashing patches 2,3,4 ? It's odd to have the same lines of code patched up 3 times. Just do it right once.
On Fri, Mar 25, 2022 at 03:43:15PM +0100, Peter Zijlstra wrote: > On Fri, Mar 25, 2022 at 11:22:53PM +0900, Masami Hiramatsu wrote: > > > Masami Hiramatsu (3): > > kprobes: Use rethook for kretprobe if possible > > rethook: kprobes: x86: Replace kretprobe with rethook on x86 > > x86,kprobes: Fix optprobe trampoline to generate complete pt_regs > > > > Peter Zijlstra (1): > > Subject: x86,rethook: Fix arch_rethook_trampoline() to generate a complete pt_regs > > You fat-fingered the subject there ^ > > Other than that: > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > Hopefully the ftrace return trampoline can also be switched over.. Urgh, allnoconfig doesn't build because.. diff --git a/kernel/Makefile b/kernel/Makefile index 56f4ee97f328..471d71935e90 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -108,6 +108,7 @@ obj-$(CONFIG_TRACING) += trace/ obj-$(CONFIG_TRACE_CLOCK) += trace/ obj-$(CONFIG_RING_BUFFER) += trace/ obj-$(CONFIG_TRACEPOINTS) += trace/ +obj-$(CONFIG_RETHOOK) += trace/ obj-$(CONFIG_IRQ_WORK) += irq_work.o obj-$(CONFIG_CPU_PM) += cpu_pm.o obj-$(CONFIG_BPF) += bpf/
On Fri, 25 Mar 2022 15:43:14 +0100 Peter Zijlstra <peterz@infradead.org> wrote: > On Fri, Mar 25, 2022 at 11:22:53PM +0900, Masami Hiramatsu wrote: > > > Masami Hiramatsu (3): > > kprobes: Use rethook for kretprobe if possible > > rethook: kprobes: x86: Replace kretprobe with rethook on x86 > > x86,kprobes: Fix optprobe trampoline to generate complete pt_regs > > > > Peter Zijlstra (1): > > Subject: x86,rethook: Fix arch_rethook_trampoline() to generate a complete pt_regs > > You fat-fingered the subject there ^ > Oops, I missed to import the patch... > Other than that: > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Thanks! > > Hopefully the ftrace return trampoline can also be switched over.. The rethook clarifies the interfaces for the return trampoline, so I think this can step the integration forward.
On Fri, 25 Mar 2022 17:51:19 +0100 Peter Zijlstra <peterz@infradead.org> wrote: > On Fri, Mar 25, 2022 at 03:43:15PM +0100, Peter Zijlstra wrote: > > On Fri, Mar 25, 2022 at 11:22:53PM +0900, Masami Hiramatsu wrote: > > > > > Masami Hiramatsu (3): > > > kprobes: Use rethook for kretprobe if possible > > > rethook: kprobes: x86: Replace kretprobe with rethook on x86 > > > x86,kprobes: Fix optprobe trampoline to generate complete pt_regs > > > > > > Peter Zijlstra (1): > > > Subject: x86,rethook: Fix arch_rethook_trampoline() to generate a complete pt_regs > > > > You fat-fingered the subject there ^ > > > > Other than that: > > > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > > > Hopefully the ftrace return trampoline can also be switched over.. > > Urgh, allnoconfig doesn't build because.. > > diff --git a/kernel/Makefile b/kernel/Makefile > index 56f4ee97f328..471d71935e90 100644 > --- a/kernel/Makefile > +++ b/kernel/Makefile > @@ -108,6 +108,7 @@ obj-$(CONFIG_TRACING) += trace/ > obj-$(CONFIG_TRACE_CLOCK) += trace/ > obj-$(CONFIG_RING_BUFFER) += trace/ > obj-$(CONFIG_TRACEPOINTS) += trace/ > +obj-$(CONFIG_RETHOOK) += trace/ > obj-$(CONFIG_IRQ_WORK) += irq_work.o > obj-$(CONFIG_CPU_PM) += cpu_pm.o > obj-$(CONFIG_BPF) += bpf/ Oops, thanks for pointing out!
On Fri, 25 Mar 2022 09:49:47 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > On Fri, Mar 25, 2022 at 7:43 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > > On Fri, Mar 25, 2022 at 11:22:53PM +0900, Masami Hiramatsu wrote: > > > > > Masami Hiramatsu (3): > > > kprobes: Use rethook for kretprobe if possible > > > rethook: kprobes: x86: Replace kretprobe with rethook on x86 > > > x86,kprobes: Fix optprobe trampoline to generate complete pt_regs > > > > > > Peter Zijlstra (1): > > > Subject: x86,rethook: Fix arch_rethook_trampoline() to generate a complete pt_regs > > > > You fat-fingered the subject there ^ > > > > Other than that: > > > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > > > Hopefully the ftrace return trampoline can also be switched over.. > > Thanks Peter. What's an ETA on landing endbr set? > Did I miss a pull req? > I see an odd error in linux-next with bpf selftests > which may or may not be related. Planning to debug it > when everything settles in Linus's tree. That is what I pointed in cover mail. > BTW, this patch can be applied to next-20220324, not the bpf-next tree > directly, because this depends on ANNOTATE_NOENDBR macro. However, since > the fprobe is merged in the bpf-next, I marked this for bpf-next. > So until merging the both of fprobes and ENDBR series, to compile this > you need below 2 lines in arch/x86/kernel/rethook.c. > > #ifndef ANNOTATE_NOENDBR > #define ANNOTATE_NOENDBR > > Masami, could you do another respin? OK, I will add above temporary mitigation. > > Also do you mind squashing patches 2,3,4 ? > It's odd to have the same lines of code patched up 3 times. > Just do it right once. Hmm, I think those are different commit for different features. I would like to keep those 3 patches separated (for the case if we find any issue to introduce regs->ss later) Thank you,
Hi, Here are the 2nd version for generic kretprobe and kretprobe on x86 for replacing the kretprobe trampoline with rethook. The previous version is here[1] [1] https://lore.kernel.org/all/164818251899.2252200.7306353689206167903.stgit@devnote2/T/#u In this version I added completing pt_regs by saving regs->ss register for rethook (from Peter) and optprobe. Background: This rethook came from Jiri's request of multiple kprobe for bpf[1]. He tried to solve an issue that starting bpf with multiple kprobe will take a long time because bpf-kprobe will wait for RCU grace period for sync rcu events. Jiri wanted to attach a single bpf handler to multiple kprobes and he tried to introduce multiple-probe interface to kprobe. So I asked him to use ftrace and kretprobe-like hook if it is only for the function entry and exit, instead of adding ad-hoc interface to kprobes. For this purpose, I introduced the fprobe (kprobe like interface for ftrace) with the rethook (this is a generic return hook feature for fprobe exit handler)[2]. [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u [2] https://lore.kernel.org/all/164191321766.806991.7930388561276940676.stgit@devnote2/T/#u The rethook is basically same as the kretprobe trampoline. I just made it decoupled from kprobes. Eventually, the all arch dependent kretprobe trampolines will be replaced with the rethook trampoline instead of cloning and set HAVE_RETHOOK=y. When I port the rethook for all arch which supports kretprobe, the legacy kretprobe specific code (which is for CONFIG_KRETPROBE_ON_RETHOOK=n) will be removed eventually. Worktree notice: BTW, this patch can be applied to next-20220324, not the bpf-next tree directly, because this depends on ANNOTATE_NOENDBR macro. However, since the fprobe is merged in the bpf-next, I marked this for bpf-next. So until merging the both of fprobes and ENDBR series, to compile this you need below 2 lines in arch/x86/kernel/rethook.c. #ifndef ANNOTATE_NOENDBR #define ANNOTATE_NOENDBR But after those are merged, these lines will be unneeded. How should I handle this issue? (Just remove ANNOTATE_NOENDBR line in bpf-next?) Thank you, --- Masami Hiramatsu (3): kprobes: Use rethook for kretprobe if possible rethook: kprobes: x86: Replace kretprobe with rethook on x86 x86,kprobes: Fix optprobe trampoline to generate complete pt_regs Peter Zijlstra (1): Subject: x86,rethook: Fix arch_rethook_trampoline() to generate a complete pt_regs arch/Kconfig | 7 ++ arch/x86/Kconfig | 1 arch/x86/include/asm/unwind.h | 23 +++---- arch/x86/kernel/Makefile | 1 arch/x86/kernel/kprobes/common.h | 1 arch/x86/kernel/kprobes/core.c | 107 --------------------------------- arch/x86/kernel/kprobes/opt.c | 25 +++++--- arch/x86/kernel/rethook.c | 123 ++++++++++++++++++++++++++++++++++++++ arch/x86/kernel/unwind_orc.c | 10 ++- include/linux/kprobes.h | 51 +++++++++++++++- kernel/kprobes.c | 124 ++++++++++++++++++++++++++++++++------ kernel/trace/trace_kprobe.c | 4 + 12 files changed, 319 insertions(+), 158 deletions(-) create mode 100644 arch/x86/kernel/rethook.c -- Masami Hiramatsu (Linaro) <mhiramat@kernel.org>