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

Mykyta Yatsenko
2026-04-15T16:03:22Z

On 4/14/26 11:26 PM, sashiko-bot@kernel.org wrote:
> Sashiko AI review found 3 potential issue(s):
> - [Low] Leaking the BTF object on unexpected success of sanitization.
> - [Low] Lack of error checks for BTF construction calls.
> - [Low] Using `BTF_MAX_VLEN` as the `byte_sz` argument in `btf__add_datasec()`.
> --
>
> commit 9c5af444155b34aa840793bf185bc0c0aaa04f3f
> Author: Alan Maguire <alan.maguire@oracle.com>
>
> selftests/bpf: Test BTF sanitization rejection for invalid vlen
>
> This patch adds a selftest to verify that BTF objects with extended vlen
> are properly rejected by kernels that do not support the extended feature.
> It simulates the lack of support using the feature cache and checks if
> sanitization fails when the vlen limit is surpassed.
>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_sanitize.c b/tools/testing/selftests/bpf/prog_tests/btf_sanitize.c
>> index 652b51efafc27..f41cea5a769a2 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/btf_sanitize.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/btf_sanitize.c
> [ ... ]
>> @@ -95,3 +95,65 @@ void test_btf_sanitize_layout(void)
>> +static void test_btf_sanitize_extended(void)
>> +{
> [ ... ]
>> + btf = btf__new_empty();
>> + if (!ASSERT_OK_PTR(btf, "empty_btf"))
>> + goto out;
>> + btf__add_int(btf, "int", 4, BTF_INT_SIGNED);
>> + btf__add_var(btf, "var1", 0, 1);
>
> Are we missing error checks on these btf__add_*() calls?
>
> If any of these allocations silently fail, the test might proceed with an
> incomplete BTF object that hasn't actually surpassed the vlen limit. This
> would cause the subsequent bpf_object__sanitize_btf() call to unexpectedly
> succeed, failing the test for the wrong reason.
>
>> + btf__add_datasec(btf, "datasec1", BTF_MAX_VLEN);
>
> Is it intentional to use BTF_MAX_VLEN for the byte_sz argument here?
>
> BTF_MAX_VLEN is a bitmask representing the maximum number of items, not a byte
> size. Using a proper calculated byte size like 0x10000 * 4 might be clearer.
>
>> + for (i = 0; i <= 0xffff; i++)
>> + btf__add_datasec_var_info(btf, 2, i * 4, 4);
>> +
>> + /* Sanitization should fail here as vlen surpasses unextended limit. */
>> + sanitized = bpf_object__sanitize_btf(skel->obj, btf);
>> + if (!ASSERT_ERR_PTR(sanitized, "bpf_object__sanitize_btf_should_fail")) {
>> + sanitized = NULL;
>
> If the first bpf_object__sanitize_btf() call unexpectedly succeeds, sanitized
> will hold a valid pointer.
>
> Doesn't setting it to NULL before the goto out path leak the memory, since
> btf__free(sanitized) will be called with a NULL pointer at the end of the
> function?
>

This is no leak, but setting to NULL is not needed, because btf__free()
checks IS_ERR_OR_NULL(btf).

>> + goto out;
>> + }
>> + /* Now switch extended feature on and ensure success. */
>