Re: [PATCH bpf-next v1 5/7] selftests/bpf: don't report arena tests as passing with an empty body

[Vineet Gupta <vineet.gupta@linux.dev>]

On 10/27/26 5:46 PM, bot+bpf-ci@kernel.org wrote:
>> selftests/bpf: don't report arena tests as passing with an empty body
> TL;DR: Commit message only: it cites arena_types to justify running arena
> kfunc tests without a cast, but the address args are __ign (ARG_IGNORE) and
> never checked against it. Conclusion looks right; the explanation may need
> rewording.
>
>> selftests/bpf: don't report arena tests as passing with an empty body
>>
>> Some arena tests put the __BPF_FEATURE_ADDR_SPACE_CAST guard inside the
>> function rather than around it, so without arena support the program
>> still exists, its body is empty, and __success __retval(0) is met by
>> doing nothing.  A reader sees
>>
>>    #584/1	 verifier_arena/basic_alloc1_nosleep:OK
>>
>> and concludes arena works.  That is worse than the tests which vanish
>> entirely, since nothing hints anything is missing.
>>
>> The guard was never needed to compile those bodies: without the
>> feature __arena is still btf_type_tag("arena"), which every compiler
>> accepts.  What it avoided was the load failure that follows from no
>> addr_space_cast being emitted — but only for a body that dereferences
>> an arena pointer.  The arena kfuncs take one as a plain scalar:
>>
>>    static const struct bpf_reg_types arena_types = {
>> 	  .types = { PTR_TO_ARENA, SCALAR_VALUE }
>>
>> so a test that only passes pointers to bpf_arena_{alloc,free,reserve}_pages()
>> needs no cast and runs fine.
>>
>> test_loader checks skip_reason before it opens anything, so the body no
>> longer has to be empty to stay loadable: drop all 19 in-body guards.
>> They do not all need a tag in their place, which is why 19 guards go
>> and 7 tags arrive:
>>
>>    6   dereference, so they get __skip_without_arena and report SKIP:
>>        basic_alloc1{,_nosleep}, basic_alloc2{,_nosleep}, big_alloc1,
>>        access_reserved
>>    11  never dereference, so they get no tag and run for real instead
>>        of reporting a vacuous OK
>>    2   big_alloc3 and big_alloc4, already inside the file's outer
>>        whole-program guard, so the in-body one was redundant
>>
>> The 7th tag goes on check_arena_arg_ret(), which has no guard at all
>> and is the one verifier_arena subtest that really fails under a stock
>> GCC.
>>
>> Measured on bpf-next with a stock bpf-gcc, test_progs before and after:
>>
>>    verifier_arena/basic_alloc1            OK   -> SKIP
>>    verifier_arena/basic_alloc1_nosleep    OK   -> SKIP
>>    verifier_arena/basic_alloc2            OK   -> SKIP
>>    verifier_arena/basic_alloc2_nosleep    OK   -> SKIP
>>    verifier_arena_large/big_alloc1        OK   -> SKIP
>>    verifier_arena_large/access_reserved   OK   -> SKIP
>>    verifier_arena/check_arena_arg_ret     FAIL -> SKIP
>>
>> the six that dereference stop claiming to pass, and check_arena_arg_ret
>> stops failing.  The eleven that do not dereference stay OK, having gone
>> from an empty body to actually running.
>>
>> Removing the guards exposes the arena_base() calls they were hiding.
>> The previous patch made that macro opaque to GCC's -Warray-bounds, so
>> nothing is needed here beyond the guards going away.
>>
>> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
> The paragraph that says "The arena kfuncs take one as a plain scalar" and
> quotes arena_types: is that the check that actually lets these tests run
> without an addr_space_cast?
>
> bpf_arena_alloc_pages(), bpf_arena_free_pages() and
> bpf_arena_reserve_pages() declare their address argument with the __ign
> suffix in kernel/bpf/arena.c:
>
>    __bpf_kfunc void *bpf_arena_alloc_pages(void *p__map, void *addr__ign, ...)
>    __bpf_kfunc void bpf_arena_free_pages(void *p__map, void *ptr__ign, ...)
>    __bpf_kfunc int bpf_arena_reserve_pages(void *p__map, void *ptr__ign, ...)
>
> get_kfunc_arg_type() tests the __ign suffix first and returns ARG_IGNORE:
>
>    if (is_kfunc_arg_ignore(meta->btf, &args[arg]) || is_kfunc_arg_implicit(meta, arg))
>            return ARG_IGNORE;
>
> and check_func_arg() returns for that type before any register type check:
>
>    if (arg_type == ARG_IGNORE)
>            return 0;
>
> arena_types is only reached through compatible_reg_types[ARG_PTR_TO_ARENA],
> which applies to kfunc parameters named with __arena or __arena__nullable and
> to arena-tagged global subprog arguments.  resolve_btfids adds the arena type
> tag for KF_ARENA_ARG2 but keeps the parameter name, so the __ign rule still
> applies.
>
> So the conclusion that no cast is needed looks right, but the address
> argument is not checked against { PTR_TO_ARENA, SCALAR_VALUE } at all.  The
> same reasoning applies to the tests that no longer have a guard or a tag:
> basic_reserve1, basic_reserve2, reserve_twice, reserve_invalid_region and
> arena_kfuncs_under_bpf_lock in verifier_arena.c, and
> request_partially_reserved and free_reserved in verifier_arena_large.c.
>
> Could the paragraph cite the addr__ign/ptr__ign parameters
> (is_kfunc_arg_ignore() -> ARG_IGNORE) instead of arena_types?

Yes this will be fixed in v2.

Thx,
-Vineet

> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40608355234