Re: [PATCH bpf-next 10/13] selftests/bpf: Add tracing_multi bpf prog cookie test

> diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_multi.c b/tools/testing/selftests/bpf/prog_tests/tracing_multi.c
> index 5cf11707eaf89..070266a864d4f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/tracing_multi.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tracing_multi.c
> @@ -304,9 +304,12 @@ static int run_bpf_target(struct bpf_program *prog, __u32 retval, const char *na
>  	return 0;
>  }
>
> -static void test_link_api_bpf_prog(void)
> +static void __test_link_api_bpf_prog(bool test_cookies)
>  {
>  	const char *funcs[] = { "target_1", "target_2" };
> +	__u64 fentry_cookies[] = { 0xfeed01, 0xfeed01 };
> +	__u64 fexit_cookies[] = { 0xfeed02, 0xfeed02 };
> +	__u64 fsession_cookies[] = { 0xfeed03, 0xfeed03 };

A subsystem pattern flags this as potentially concerning: every per-target
cookie array holds the same value twice. The test on the BPF side compares
against a single scalar global set from element [0]:

  skel->bss->fentry_cookie = fentry_cookies[0];

and in progs/tracing_multi_bpf.c:

  if (!test_cookies || bpf_get_attach_cookie(ctx) == fentry_cookie)
          test_result_fentry++;

Since cookie[0] == cookie[1] for all three arrays, the test cannot
distinguish a correct cookie-to-target mapping from a permuted one. The
assertions (test_result_fentry == 2, etc.) succeed as long as some
correct-looking cookie value is delivered on both invocations, but swapping
the cookies of target_1 and target_2 in the kernel is invisible.

That matters because the kernel path this commit exercises is exactly a
permutation: bpf_tracing_multi_attach() calls check_dup_keys() which calls
keys_swap_r() in kernel/trace/bpf_trace.c to reorder the cookies, ids, and
progs arrays to follow the sorted trampoline keys:

  id_a    = ids + (key_a - keys);
  if (data->progs) { ... swap(*prog_a, *prog_b); }
  if (cookies) { cookie_a = cookies + (id_a - ids); ... swap(*cookie_a, *cookie_b); }

An indexing mistake in that swap could desynchronise cookies from progs/ids,
yet the test would still pass. Since fds[]/ids[] come from BPF prog FDs, the
sort order is not the array order the caller supplied.

The neighbouring test for kernel-function targets in this same file already
does this correctly: bpf_fentry_test_cookies[] uses ten distinct values
deliberately out of order (8, 9, 7, 5, 4, 2, 3, 1, 10, 6) and
progs/tracing_multi_check.c checks the cookie per-IP with something like:

  err |= test_cookies ? cookie != 8 : 0;

under `ip == &bpf_fentry_test1`.

Can the test use distinct cookies and a bpf_get_func_ip()-based per-target
expectation so it is sensitive to cookie mis-association?

>  	LIBBPF_OPTS(bpf_tracing_multi_opts, opts);
>  	struct tracing_multi_bpf *skel = NULL;
>  	int fds[ARRAY_SIZE(funcs)];

[ ... ]

> @@ -356,6 +367,16 @@ static void test_link_api_bpf_prog(void)
>  	tracing_multi_bpf__destroy(skel);
>  }
>
> +static void test_link_api_bpf_prog(void)
> +{
> +	__test_link_api_bpf_prog(false);
> +}
> +
> +static void test_cookies_bpf_prog(void)
> +{
> +	__test_link_api_bpf_prog(true);
> +}

This isn't a bug, but would calling __test_link_api_bpf_prog(false) and
__test_link_api_bpf_prog(true) straight from the two subtest arms read more
consistently? The neighbouring test_link_api_ids() is dispatched directly as
test_link_api_ids(false) and test_link_api_ids(true) just a dozen lines
down, without wrapper functions.

[ ... ]


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