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

[Andrii Nakryiko]

On Mon, Sep 9, 2024 at 12:46 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> 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

> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>

[...]

> @@ -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)?

> +       return ret;
>  }
>

[...]