On Mon, Jul 10, 2023 at 12:24 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Thu, Jul 06, 2023 at 03:29:08PM -0700, Andrii Nakryiko wrote:
> > On Fri, Jun 30, 2023 at 1:34 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> > >
> > > Adding support for bpf_get_func_ip helper being called from
> > > ebpf program attached by uprobe_multi link.
> > >
> > > It returns the ip of the uprobe.
> > >
> > > Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > > ---
> > > kernel/trace/bpf_trace.c | 33 ++++++++++++++++++++++++++++++---
> > > 1 file changed, 30 insertions(+), 3 deletions(-)
> > >
> >
> > A slight aside related to bpf_get_func_ip() support in
> > uprobe/uretprobe. We just had a conversation with Alastair and Jordan
> > (cc'ed) about bpftrace and using bpf_get_func_ip() there with
> > uretprobes, and it seems like it doesn't work.
> >
> > Is that intentional or we just missed that bpf_get_func_ip() doesn't
> > work with uprobes/uretprobes? Do you think it would be hard to add
> > support for them for bpf_get_func_ip()? It's a very useful helper,
> > would be nice to have it working in all cases where it has meaningful
> > behavior (and I think it does for uprobe and uretprobe).
>
> I'm not sure we discussed at the time we added this helper,
> but I see same problem as we had with kprobes where we need
> to know if the probe is on the start of the symbol
>
> we use KPROBE_FLAG_ON_FUNC_ENTRY flag for that in kprobe's
> get_func_ip helper version:
>
> BPF_CALL_1(bpf_get_func_ip_kprobe, struct pt_regs *, regs)
> {
> struct kprobe *kp = kprobe_running();
>
> if (!kp || !(kp->flags & KPROBE_FLAG_ON_FUNC_ENTRY))
> return 0;
>
> return get_entry_ip((uintptr_t)kp->addr);
> }
>
> I don't think we can have same flag for uprobe, we don't have
> the binary symbol data as we have for kernel
what if we just return whatever was the IP where uprobe/uretprobe
interrupt instruction was installed at? I haven't tried what does
regs->ip contain for u*ret*probe, though, if it already has the IP
where we installed uprobe itself, it might be ok as is. But still,
feels like it would be nice to have bpf_get_func_ip() working for
uprobe/uretprobe (even if semantics might differ for uprobes not at
the entry of the function).
>
> I guess the app needs to store regs->ip and match it against symbol
> addresses in user space
>
> jirka
>
>
> >
> > Thanks!
> >
> > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > > index 4ef51fd0497f..f5a41c1604b8 100644
> > > --- a/kernel/trace/bpf_trace.c
> > > +++ b/kernel/trace/bpf_trace.c
> > > @@ -88,6 +88,7 @@ static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx);
> > > static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx);
> > >
> >
> > [...]
