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

[Vineet Gupta]

On 10/27/26 5:46 PM, bot+bpf-ci@xxxxxxxxxx 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@xxxxxxxxx>
> 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