Re: [PATCH v2 bpf-next 5/8] selftests/bpf: Test BTF sanitization rejection for invalid vlen

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Incorrect assertion macro `ASSERT_OK` used for boolean return value `kernel_supports()`, leading to spurious test failures on kernels supporting the feature.
- [Medium] Malformed BTF construction: DATASEC byte size is too small for the added variables, resulting in out-of-bounds offsets.
--

commit 547d5170cb3d8f555e19115439d037e23963de75
Author: Alan Maguire <alan.maguire@oracle.com>

selftests/bpf: Test BTF sanitization rejection for invalid vlen

For a kernel that does not support extended vlen, BTF should be
rejected. This commit simulates non-support using the feature cache 
and ensures that BTF with an extended vlen DATASEC is rejected.

> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_sanitize.c b/tools/testing/selftests/bpf/prog_tests/btf_sanitize.c
> index 652b51efafc27..1349fc7b34e9c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_sanitize.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_sanitize.c

[ ... ]

> +static void test_btf_sanitize_extended(void)
> +{
> +	struct btf *btf = NULL, *sanitized = NULL;
> +	struct kern_feature_cache *cache = NULL;
> +	struct kfree_skb *skel = NULL;
> +	int i;
> +
> +	skel = kfree_skb__open();
> +	if (!ASSERT_OK_PTR(skel, "kfree_skb_skel"))
> +		return;
> +
> +	if (!ASSERT_OK(kernel_supports(skel->obj, FEAT_BTF_VLEN_KIND_EXTENDED),
> +		       "vlen_kind_extended_supported"))
> +		goto out;

Is ASSERT_OK the right macro to use for this check?

Because kernel_supports() returns a boolean, if the test is run on a kernel
that natively supports the feature, it will return true (1). Since ASSERT_OK
expects exactly 0 for success, this evaluates to ASSERT_OK(1) and triggers
a spurious test failure.

Would ASSERT_TRUE be more appropriate here to avoid aborting the test on
modern kernels?

> +
> +	cache = calloc(1, sizeof(*cache));

[ ... ]

> +	btf = btf__new_empty();
> +	if (!ASSERT_OK_PTR(btf, "empty_btf"))
> +		goto out;
> +	if (!ASSERT_GT(btf__add_int(btf, "int", 4, BTF_INT_SIGNED), 0, "add_int") ||
> +	    !ASSERT_GT(btf__add_var(btf, "var1", 0, 1), 0, "add_var") ||
> +	    !ASSERT_GT(btf__add_datasec(btf, "datasec1", 0x10000), 0, "add_datasec"))
> +		goto out;
> +
> +	for (i = 0; i < 0x10000; i++) {
> +		if (!ASSERT_OK(btf__add_datasec_var_info(btf, 2, i * 4, 4), "add_var_info"))
> +			goto out;
> +	}

Does this loop create a structurally malformed BTF object?

The DATASEC byte size is explicitly set to 0x10000 bytes above, but the loop
adds 0x10000 variables of size 4 at offsets up to 65535 * 4 (262140). This
places the variables far outside the declared bounds of the section.

Should the declared DATASEC size be 0x40000 instead to properly accommodate
the added variables and prevent unexpected failures if tighter bounds checks
are introduced?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260416143904.1220662-1-alan.maguire@oracle.com?part=5