Re: [RFC PATCH bpf-next v2 1/3] libbpf: Auto-upgrade uprobes to multi-uprobes when supported

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3530 bytes --]

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 9ea41f40dc82..056bc50a0bbb 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[ ... ]

> @@ -12783,6 +12802,32 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
>  		func_offset += sym_off;
>  	}
>
> +	/* This provides backwards compatibility to programs using uprobe, but
> +	 * have been auto-upgraded to multi uprobe.
> +	 */
> +	if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI) {
> +		LIBBPF_OPTS(bpf_uprobe_multi_opts, multi_opts);
> +		unsigned long offsets[1] = {func_offset};
> +		__u64 bpf_cookie;
> +
> +		multi_opts.retprobe = OPTS_GET(opts, retprobe, false);
> +		if (offsets[0] || func_name) {
> +			multi_opts.offsets = offsets;
> +			multi_opts.cnt = 1;
> +		}

When func_offset is 0 and func_name is NULL, this condition is false
and multi_opts.offsets and multi_opts.cnt are never set. The subsequent
call to bpf_program__attach_uprobe_multi() then fails with -EINVAL
because both func_pattern and cnt are 0:

bpf_program__attach_uprobe_multi() {
    ...
    if (!func_pattern && cnt == 0)
        return libbpf_err_ptr(-EINVAL);
    ...
}

This can happen when callers use bpf_program__attach_uprobe() on an
auto-upgraded program with offset 0:

bpf_program__attach_uprobe() {
    DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts, .retprobe = retprobe);
    return bpf_program__attach_uprobe_opts(..., func_offset, &opts);
}

Here func_name is never set in opts. The old perf_event path handled
offset 0 without issue. Should multi_opts.offsets and multi_opts.cnt
always be set unconditionally, since offset 0 is a valid uprobe offset?

> +		if (ref_ctr_off) {
> +			multi_opts.ref_ctr_offsets = &ref_ctr_off;
> +			multi_opts.cnt = 1;
> +		}
> +		bpf_cookie = OPTS_GET(opts, bpf_cookie, 0);
> +		if (bpf_cookie) {
> +			multi_opts.cookies = &bpf_cookie;
> +			multi_opts.cnt = 1;
> +		}
> +
> +		return bpf_program__attach_uprobe_multi(prog, pid, binary_path,
> +							NULL, &multi_opts);
> +	}

[ ... ]

> @@ -12902,13 +12948,14 @@ static int attach_uprobe(const struct bpf_program *prog, long cookie, struct bpf
>  			else
>  				offset = 0;
>  		}
> -		opts.retprobe = strcmp(probe_type, "uretprobe") == 0 ||
> -				strcmp(probe_type, "uretprobe.s") == 0;
> -		if (opts.retprobe && offset != 0) {
> +		is_retprobe = strcmp(probe_type, "uretprobe") == 0 ||
> +			      strcmp(probe_type, "uretprobe.s") == 0;

Shouldn't this also match "uretprobe.single"? When auto-attaching
with SEC("uretprobe.single/binary:func"), sscanf sets probe_type to
"uretprobe.single", which doesn't match either "uretprobe" or
"uretprobe.s". As a result is_retprobe is false and opts.retprobe
stays false, so the program gets attached as a regular uprobe instead
of a return probe.

Something like strncmp(probe_type, "uretprobe", 9) == 0 would cover
all uretprobe variants ("uretprobe", "uretprobe.s", "uretprobe.single").

> +		if (is_retprobe && offset != 0) {
>  			pr_warn("prog '%s': uretprobes do not support offset specification\n",
>  				prog->name);
>  			break;
>  		}
> +		opts.retprobe = is_retprobe;
>  		opts.func_name = func_name;
>  		*link = bpf_program__attach_uprobe_opts(prog, -1, binary_path, offset, &opts);
>  		ret = libbpf_get_error(*link);


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/23741893093