On 10/8/26 00:20, [email protected] wrote:
>> 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?
>
Will update the cookies with different values.
>> 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.
>
Okay, will drop these two wrappers.
Thanks,
Leon
> [ ... ]
>
>
> ---
> 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
