Messages in this thread | | | From | Florent Revest <> | Date | Wed, 9 Aug 2023 12:28:38 +0200 | Subject | Re: [RFC PATCH v2 1/6] fprobe: Use fprobe_regs in fprobe entry handler |
| |
On Mon, Aug 7, 2023 at 8:48 AM Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > This allows fprobes to be available with CONFIG_DYNAMIC_FTRACE_WITH_ARGS > instead of CONFIG_DYNAMIC_FTRACE_WITH_REGS, then we can enable fprobe > on arm64.
This patch lets fprobe code build on configs WITH_ARGS and !WITH_REGS but fprobe wouldn't run on these builds because fprobe still registers to ftrace with FTRACE_OPS_FL_SAVE_REGS, which would fail on !WITH_REGS. Shouldn't we also let the fprobe_init callers decide whether they want REGS or not ?
> static int > kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip, > - unsigned long ret_ip, struct pt_regs *regs, > + unsigned long ret_ip, struct ftrace_regs *fregs, > void *data) > { > struct bpf_kprobe_multi_link *link; > + struct pt_regs *regs = ftrace_get_regs(fregs); > + > + if (!regs) > + return 0;
(with the above comment addressed) this means that BPF multi_kprobe would successfully attach on builds WITH_ARGS but the programs would never actually run because here regs would be 0. This is a confusing failure mode for the user. I think that if multi_kprobe won't work (because we don't have a pt_regs conversion path yet), the user should be notified at attachment time that they won't be getting any events. That's why I think kprobe_multi should inform fprobe_init that it wants FTRACE_OPS_FL_SAVE_REGS and fail if that's not possible (no trampoline for it for example)
| |