Re: [PATCHv4 bpf-next 2/3] selftests/bpf: Add tracing_multi link info tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing UAPI and kernel implementation for 'tracing_multi' link info.
- [Medium] Flawed negative testing logic for 'tracing_multi' invalid user buffers.
--

commit 10d5275538071b94b5cfb747b2b37b5cb2e5d158
Author: Jiri Olsa <jolsa@kernel.org>

selftests/bpf: Add tracing_multi link info tests

Adding tracing_multi link info tests that follow the kprobe_multi
and uprobe_multi tests logic.

> diff --git a/tools/testing/selftests/bpf/prog_tests/fill_link_info.c b/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
> index f589eefbf9fbd..0918321c8e63d 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
[ ... ]
> @@ -396,6 +413,224 @@ static void test_kprobe_multi_fill_link_info(struct test_fill_link_info *skel,
[ ... ]
> +static int verify_tracing_multi_link_info(int fd, const struct bpf_program *prog,
> +					  const struct tmulti_target *targets,
> +					  __u32 btf_obj_id, bool has_cookies)
> +{
> +	enum bpf_attach_type attach_type = bpf_program__expected_attach_type(prog);
> +	__u64 addrs[TRACING_MULTI_CNT], cookies[TRACING_MULTI_CNT];
> +	__u32 ids[TRACING_MULTI_CNT];
> +	struct bpf_link_info info;
> +	__u32 len = sizeof(info);
> +	int err, i;
> +
> +	memset(&info, 0, sizeof(info));
> +	err = bpf_link_get_info_by_fd(fd, &info, &len);
> +	if (!ASSERT_OK(err, "bpf_link_get_info_by_fd"))
> +		return -1;
> +
> +	if (!ASSERT_EQ(info.type, BPF_LINK_TYPE_TRACING_MULTI, "info.type"))
> +		return -1;
> +
> +	ASSERT_EQ(info.tracing_multi.attach_type, attach_type, "info.tracing_multi.attach_type");
> +	ASSERT_EQ(info.tracing_multi.count, TRACING_MULTI_CNT, "info.tracing_multi.count");

[Severity: Medium]
Does struct bpf_link_info currently have a tracing_multi member? It appears
this might cause a compilation failure because the UAPI header doesn't include
it. Also, does the kernel side implement the .fill_link_info callback in
bpf_tracing_multi_link_lops?

> +
> +	memset(ids, 0, sizeof(ids));
> +	memset(cookies, 0, sizeof(cookies));
> +	memset(addrs, 0, sizeof(addrs));
[ ... ]
> +static void verify_tracing_multi_invalid_user_buffer(int fd, const struct tmulti_target *targets)
> +{
> +	__u32 ids[TRACING_MULTI_CNT] = {};
> +	struct bpf_link_info info;
> +	__u32 len = sizeof(info);
> +	int err, i;
[ ... ]
> +	/* Invalid addrs pointer -> EFAULT */
> +	memset(&info, 0, sizeof(info));
> +	info.tracing_multi.addrs = 0x1;
> +	info.tracing_multi.count = TRACING_MULTI_CNT;
> +	err = bpf_link_get_info_by_fd(fd, &info, &len);
> +	ASSERT_EQ(err, -EFAULT, "tracing_multi.bad_addrs");

[Severity: Medium]
Will this test actually return -EFAULT?

By zeroing out the info struct before these checks, the mandatory ids
pointer is left as NULL while count is non-zero. If the kernel validation
logic eagerly rejects the missing ids array with -EINVAL before reaching
the -EFAULT paths, won't this assertion fail?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629212208.895962-3-jolsa@kernel.org?part=1