Re: [PATCH bpf-next 12/13] selftests/bpf: Add tracing_multi bpf prog link info test

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing array sorting in user-space targets leads to flaky test
- [Medium] Uninitialized stack variable `fids` passed to kernel
--

--- Patch [12]: [PATCH bpf-next 12/13] selftests/bpf: Add tracing_multi bpf prog link info test ---
commit 5ee3239754522a052ad0eb2cf3e2485319bff581
Author: Leon Hwang <[email protected]>

selftests/bpf: Add tracing_multi bpf prog link info test
    
Verify the link info works well for bpf prog targets of tracing_multi link.

> 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 0918321c8e63d..a5f8bfcb1e07a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
[ ... ]
> @@ -467,13 +489,48 @@ static int setup_tmulti_targets(const struct bpf_program *prog,
>  	return -1;
>  }
>  
> +static int setup_tmulti_bpf_prog_targets(const struct bpf_program *prog, int token_fd,
> +					 const __u64 *cookies,
> +					 const struct bpf_program * const *target_progs,
> +					 struct tmulti_target *targets,
> +					 __u32 *btf_obj_id)
> +{
[ ... ]
> +		targets[i].cookie = cookies[i];
> +		targets[i].id = prog_info.id;
> +		targets[i].func_btf_id = func_btf_id;
> +	}
> +
> +	return setup_tmulti_btf_obj_id(prog, btf_obj_id);
> +}

[Severity: Medium]
Will this lead to flaky tests? The kernel sorts the tracing_multi targets by a
computed key during attachment.

When returning link info, the arrays are returned in this sorted order, and
verify_tracing_multi_link_info() asserts that they perfectly match the targets
array at the exact same indexes.

However, unlike setup_tmulti_targets(), this function does not call qsort() on
the targets array. If ID allocation wraps around or another process allocates
IDs concurrently, this might fail non-deterministically.

> +
>  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)
> +					  __u32 btf_obj_id, __u32 count,
> +					  bool has_cookies, bool tgt_progs)
>  {
> +#define TMULTI_CNT (TRACING_MULTI_CNT > TRACING_MULTI_BPF_CNT ? \
> +		    TRACING_MULTI_CNT : TRACING_MULTI_BPF_CNT)
>  	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];
> +	__u64 addrs[TMULTI_CNT], cookies[TMULTI_CNT];
> +	__u32 ids[TMULTI_CNT], fids[TMULTI_CNT];
>  	struct bpf_link_info info;
>  	__u32 len = sizeof(info);
>  	int err, i;
[ ... ]
>  	memset(ids, 0, sizeof(ids));
>  	memset(cookies, 0, sizeof(cookies));
> @@ -496,7 +555,8 @@ static int verify_tracing_multi_link_info(int fd, const struct bpf_program *prog
>  	info.tracing_multi.ids = ptr_to_u64(ids);
>  	info.tracing_multi.addrs = ptr_to_u64(addrs);
>  	info.tracing_multi.cookies = has_cookies ? ptr_to_u64(cookies) : 0;
> -	info.tracing_multi.count = TRACING_MULTI_CNT;
> +	info.tracing_multi.func_btf_ids = tgt_progs ? ptr_to_u64(fids) : 0;
> +	info.tracing_multi.count = count;
>  
>  	err = bpf_link_get_info_by_fd(fd, &info, &len);

[Severity: Medium]
Does this code leave the fids array uninitialized?

The other stack arrays (ids, cookies, addrs) are explicitly cleared with
memset() before being passed to the bpf_link_get_info_by_fd() syscall, but
fids is not. This could potentially cause issues with uninitialized memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12