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

On 9/8/26 23:29, [email protected] wrote:
> 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.
> 

Will add qsort() here.

>> +
>>  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.
> 

Will add 'memset(fids, 0, sizeof(fids))'.

Thanks,
Leon