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

Andrii Nakryiko
2024-09-09T23:44:42Z

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

[...]

> @@ -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;
> }
> …