Re: [PATCH bpf-next v2 4/4] selftests/bpf: Add kfunc set test to resolve_btfids

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Failure of the `ASSERT_EQ` check on `set->cnt` does not halt execution, leading to an out-of-bounds array read in the subsequent loop in `check_kfunc_set()`.
--

commit 9cd3f00b88448034723a89667e181cfaf756806b
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

selftests/bpf: Add kfunc set test to resolve_btfids

This commit extends the resolve_btfids selftest to cover kfunc sets defined
with BTF_KFUNCS_START and BTF_KFUNCS_END. It verifies that resolve_btfids
correctly processes BTF_ID_FLAGS, resolves function IDs, and ensures the
kfunc set is sorted.

> diff --git a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
> index 98ce8fca2007e..ac51fd4548214 100644
> --- a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
> +++ b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
[ ... ]
> @@ -122,6 +159,31 @@ static int resolve_symbols(struct btf *btf)
>  	return 0;
>  }
>  
> +static void check_kfunc_set(struct btf_id_set8 *set)
> +{
> +	unsigned int i, j;
> +
> +	ASSERT_EQ(set->flags, BTF_SET8_KFUNCS, "kfunc_set_flags");
> +	ASSERT_EQ(set->cnt, ARRAY_SIZE(kfunc_symbols), "kfunc_set_cnt");
> +

[Severity: Medium]
In the modern BPF selftests framework, ASSERT_EQ() evaluates to a boolean
and logs a failure but does not automatically return from the function.

If the resolve_btfids tool has a bug and sets an unexpectedly large value
for set->cnt, the assertion will fail, but execution unconditionally proceeds
to the loop.

Could this cause the test to iterate past the actual bounds of the array,
reading out of bounds from set->pairs[i] and crashing with a segmentation
fault? Would it be safer to wrap this check in an early return, such as
if (!ASSERT_EQ(...)) return; ?

> +	for (i = 0; i < set->cnt; i++) {
> +		for (j = 0; j < ARRAY_SIZE(kfunc_symbols); j++) {
> +			if (kfunc_symbols[j].id == (s32)set->pairs[i].id) {
> +				ASSERT_EQ(set->pairs[i].flags,
> +					  kfunc_symbols[j].flags, "kfunc_flags_check");
> +				break;
> +			}
> +		}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624005546.1818483-1-ihor.solodrai@linux.dev?part=4