Re: [PATCH bpf-next 2/2] selftests/bpf: Cover mixed arena and map-value atomics

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3348 bytes --]

> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena.c b/tools/testing/selftests/bpf/progs/verifier_arena.c
> index b241bbcf54a8a..2e43da188194b 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_arena.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_arena.c
> @@ -25,6 +25,15 @@ struct {
>  	__ulong(map_extra, ARENA_VM_START); /* start of mmap() region */
>  } arena SEC(".maps");
>
> +struct {
> +	__uint(type, BPF_MAP_TYPE_ARRAY);
> +	__uint(max_entries, 1);
> +	__type(key, __u32);
> +	__type(value, __u64);
> +} atomic_map SEC(".maps");
> +
> +static __u64 arena_atomic_target SEC(".addr_space.1");
                  ^^^^^^^^^^^^^^^^^^^^

Does this arena global permanently consume one of the arena's two pages
and break the existing allocation tests in this file?

The arena is declared with max_entries = 2 (two pages). When libbpf loads
the object, it places .addr_space.1 globals at the end of the arena:

  obj->arena_data_off = bpf_map_mmap_sz(arena_map) -
                        roundup(arena_data_sz, PAGE_SIZE)
                      = 2*4096 - 4096 = 4096

Then it copies the globals with:

  memcpy(map->mmaped + obj->arena_data_off, obj->arena_data,
         obj->arena_data_sz)

That user-space write faults page 1. In arena_vm_fault(), the kernel calls:

  range_tree_clear(&arena->rt, vmf->pgoff, 1)

marking page 1 as allocated and unavailable to bpf_arena_alloc_pages().

This happens during bpf_object__load() for every subtest since test_loader
reopens and reloads the object each time. So every subtest now has 1 free
page instead of 2.

Looking at the existing tests that should fail:

  basic_alloc1_nosleep() / basic_alloc1(): second
  bpf_arena_alloc_pages(&arena, NULL, 1, ...) returns NULL -> return 2

  basic_alloc2_nosleep() / basic_alloc2():
  bpf_arena_alloc_pages(&arena, NULL, 2, ...) cannot find 2 contiguous
  free pages -> return 1

  basic_alloc3_nosleep() / basic_alloc3(): allocates
  ar->map.max_entries (== 2) pages -> NULL -> return 1

  basic_reserve1_nosleep() / basic_reserve1():
  bpf_arena_reserve_pages() on the second page hits is_range_tree_set()
  == -ESRCH -> arena_reserve_pages() returns -EBUSY -> return 2

  free_scalar_below_arena(): second alloc returns NULL -> return 2

All nine of these are annotated __success __retval(0) but would now
return non-zero.

verifier_arena_globals1.c is the existing precedent: it sizes the arena
large and reserves a dedicated 16-page region for globals, and its
check_reserve1() asserts that bpf_arena_reserve_pages() over the globals
region fails because libbpf already mapped them in.

The new test doesn't need an arena global at all. The idiomatic way to get
a PTR_TO_ARENA in a __naked test is to call bpf_arena_alloc_pages() and
use addr_space_cast(), as verifier_ldsx.c and compute_live_registers.c do.
That links the arena to the program without adding arena data.

Alternatively, could the arena's max_entries be grown and the page-count
expectations of the existing tests updated?

> +
>  SEC("socket")
>  __success __retval(0)
>  int basic_alloc1_nosleep(void *ctx)

[ ... ]


---
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/31699396501