On Fri, Sep 09, 2022 at 01:49:16PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 09, 2022 at 12:12:43PM +0200, Jiri Olsa wrote:
>
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 68e5cdd24cef..bcada91b0b3b 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2419,6 +2419,10 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long entry_ip,
> > {
> > struct bpf_kprobe_multi_link *link;
> >
> > +#ifdef CONFIG_X86_KERNEL_IBT
> > + if (is_endbr(*((u32 *) entry_ip - 1)))
> > + entry_ip -= ENDBR_INSN_SIZE;
> > +#endif
> > link = container_of(fp, struct bpf_kprobe_multi_link, fp);
> > kprobe_multi_link_prog_run(link, entry_ip, regs);
> > }
>
> Strictly speaking this can explode if this function is one without ENDBR
> and it's on a page-edge and -1 is a guard page or something silly like
> that (could conceivably happen for a module or so).
ok, as per discussion on irc, we could use get_kernel_nofault() to
read it safely as you suggested
>
> I'm also thinking this function might be a bit clearer if the argument
> were called fentry_ip -- that way it would be clearer this is an ftrace
> __fentry__ ip.
ok
>
> The canonical way to get at +0 would be something like:
>
> kallsyms_lookup_size_offset(fentry_ip, &size, &offset);
> entry_ip = fentry_ip - offset;
>
> But I appreciate that might be too expensive here; is this a hot path?
>
> Could you store this information in struct bpf_kprobe_multi_link?
we could use the same way we use when looking for cookies in
bpf_kprobe_multi_cookie ... having extra array of real entry
ips.. or some similar approach
but let's try to read it safely with get_kernel_nofault first
jirka
