Re: [PATCHv3 2/7] bpf: Add support for uprobe multi session attach

On Tue, Sep 10, 2024 at 12:17 AM Jiri Olsa [off-list ref] wrote:

On Mon, Sep 09, 2024 at 04:44:29PM -0700, Andrii Nakryiko wrote:

quoted

On Mon, Sep 9, 2024 at 12:46 AM Jiri Olsa [off-list ref] wrote:

quoted

Adding support to attach bpf program for entry and return probe
of the same function. This is common use case which at the moment
requires to create two uprobe multi links.

Adding new BPF_TRACE_UPROBE_SESSION attach type that instructs
kernel to attach single link program to both entry and exit probe.

It's possible to control execution of the bpf program on return
probe simply by returning zero or non zero from the entry bpf
program execution to execute or not the bpf program on return
probe respectively.

pedantic nit: bpf -> BPF

ok

quoted

quoted

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/uapi/linux/bpf.h       |  1 +
 kernel/bpf/syscall.c           |  9 +++++++--
 kernel/trace/bpf_trace.c       | 32 ++++++++++++++++++++++++--------
 tools/include/uapi/linux/bpf.h |  1 +
 tools/lib/bpf/libbpf.c         |  1 +
 5 files changed, 34 insertions(+), 10 deletions(-)

LGTM

Acked-by: Andrii Nakryiko <andrii@kernel.org>

[...]

quoted

@@ -3336,9 +3347,13 @@ uprobe_multi_link_handler(struct uprobe_consumer *con, struct pt_regs *regs,
                          __u64 *data)
 {
        struct bpf_uprobe *uprobe;
+       int ret;

        uprobe = container_of(con, struct bpf_uprobe, consumer);
-       return uprobe_prog_run(uprobe, instruction_pointer(regs), regs);
+       ret = uprobe_prog_run(uprobe, instruction_pointer(regs), regs);
+       if (uprobe->consumer.session)
+               return ret ? UPROBE_HANDLER_IGNORE : 0;

Should we restrict the return range to [0, 1] for UPROBE_SESSION
programs on the verifier side (given it's a new program type and we
can do that)?

yes, I think we can do that.. we have BPF_TRACE_UPROBE_SESSION as
expected_attach_type so we can do that during the load

hum, is it too late to do that for kprobe session as well?

I'd say let's do it, unlikely we'll break anyone. I'd expect everyone
doing explicit return 0 or return 1 anyways.

thanks,
jirka

quoted

quoted

+       return ret;
 }

[...]