Re: [PATCH bpf-next v2 11/21] bpf: Add tracing_multi link support for bpf progs

[bot+bpf-ci@kernel.org]

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

> bpf: Add tracing_multi link support for bpf progs

TL;DR: The new tracing_multi fds field is not tail-checked, so older 7.2/7.3
kernels may silently treat program BTF ids as vmlinux ids, attaching to
unrelated kernel functions. Suggests a link_create flag to gate it.

> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 6f70db7515dd..94302a3b680c 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1927,6 +1927,8 @@ union bpf_attr {
>  				__aligned_u64	ids;
>  				__aligned_u64	cookies;
>  				__u32		cnt;
> +				__u32		:32;
> +				__aligned_u64	fds;
>  			} tracing_multi;
>  		};
>  	} link_create;

Can a kernel that already has tracing_multi but not fds reject a request
that sets fds?

The generic tail-zero check only covers bytes after
BPF_LINK_CREATE_LAST_FIELD:

    #define BPF_LINK_CREATE_LAST_FIELD link_create.uprobe_multi.path_fd

uprobe_multi.path_fd ends at byte 64 of union bpf_attr, while the new
tracing_multi.fds sits at bytes 40-47 (overlapping uprobe_multi.cookies).
So CHECK_ATTR(BPF_LINK_CREATE) in link_create() never looks at it.

The tracing_multi link shipped in 7.2 (c1d32dea5d46 "bpf: Add support for
tracing multi link"), and there bpf_tracing_multi_attach() reads only ids,
cookies and cnt:

    uids = u64_to_user_ptr(attr->link_create.tracing_multi.ids);
    cnt = attr->link_create.tracing_multi.cnt;
    ...
    if (attr->link_create.flags || attr->link_create.target_fd)
        return -EINVAL;
    ...
    err = bpf_trampoline_multi_attach(prog, ids, link);

bpf_trampoline_multi_attach() then resolves each id with
bpf_check_attach_btf_id_multi(prog->aux->attach_btf, prog, ids[i], ...),
which looks the id up in the vmlinux (or module) BTF that the fentry.multi
program was loaded against.  If the type is a FUNC with an ftrace location,
the program is attached there.

The libbpf change later in this series (85b62f93fd2d "libbpf: Add
tracing_multi link support for bpf progs") fills ids[] from
libbpf_find_prog_btf_id(funcs[i], fds[i], ...) and sets
lopts.tracing_multi.fds = fds, with no feature probe.  A new libbpf running
on a 7.2 or 7.3 kernel would therefore send target-program BTF ids, and the
older kernel would treat them as vmlinux BTF ids.  Depending on what those
ids name in vmlinux BTF, that gives either:

- a misleading -EINVAL or -ENOENT that cannot be told apart from bad input,
  or
- a successful link whose program runs on unrelated kernel functions
  instead of the requested BPF program functions.

No later commit in the series changes this.  ef1ca3de15ec only adds a flag
to the bpf_link_info output struct, and older kernels already reject a
non-zero attr->link_create.flags in bpf_tracing_multi_attach().

Could the program-target mode be keyed off a new link_create.flags bit (for
example a BPF_F_TRACING_MULTI_PROGS style flag), so that kernels without
fds support fail with -EINVAL instead of reinterpreting ids?

Related to that, the new padding is not checked for zero:

> +				__u32		:32;

so it cannot later become an input field either.


---
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/38273555988