Re: [PATCH bpf-next v2 3/6] selftests/bpf: Verify arena type tags in resolve_btfids test

On 8/5/26 4:17 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Missing NULL pointer check on the result of `btf__type_by_id()` causes a segmentation fault when a kfunc symbol is missing.
> --
> 
> --- Patch [3]: [PATCH bpf-next v2 3/6] selftests/bpf: Verify arena type tags in resolve_btfids test ---
> commit 833ac8bbb8ac1055e25790da1396864987e31541
> Author: Ihor Solodrai <[email protected]>
> 
> selftests/bpf: Verify arena type tags in resolve_btfids test
> 
> This commit extends test_resolve_btfids() to assert that resolve_btfids
> emits the address_space(1) type attribute on the return type and arguments
> of kfuncs marked with KF_ARENA_RET, KF_ARENA_ARG1 or KF_ARENA_ARG2.
> 
>> diff --git a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
>> index ac51fd4548214..8482f00046d4c 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
> [ ... ]
>> @@ -227,6 +260,39 @@ void test_resolve_btfids(void)
>>  	check_kfunc_set(&test_kfunc_set);
>>  	check_kfunc_set(&test_kfunc_set_rev);
>>  
>> +	/*
>> +	 * Check resolve_btfids wrapped exactly the arena-flagged return/args
>> +	 * with the address_space(1) type attribute, and left other
>> +	 * pointers/returns untouched.
>> +	 */
>> +	for (i = 0; i < ARRAY_SIZE(kfunc_symbols); i++) {
>> +		const struct btf_type *fn, *proto;
>> +		const struct btf_param *params;
>> +		const char *name = kfunc_symbols[i].name;
>> +		u32 fl = kfunc_symbols[i].flags;
>> +		__u32 nr;
>> +
>> +		fn = btf__type_by_id(btf, kfunc_symbols[i].id);
>> +		if (!ASSERT_TRUE(btf_is_func(fn), name))
>> +			continue;
> 
> [Severity: Medium]
> Could this cause a segmentation fault if a kfunc symbol is missing?
> 
> If resolve_symbols() fails to find a kfunc in the parsed BTF file, the
> symbol's id remains -1. When -1 is passed to btf__type_by_id(), it
> returns NULL.
> 
> Since btf_is_func() unconditionally accesses the info member, passing a
> NULL pointer to it would crash the test program before the ASSERT_TRUE
> evaluates.
> 
> Should we check if fn is NULL before passing it to btf_is_func()?

We are fine with the test program crashing here. Wontfix.

> 
>> +		proto = btf__type_by_id(btf, fn->type);
>> +		if (!ASSERT_TRUE(btf_is_func_proto(proto), name))
>> +			continue;
> [ ... ]
>