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

[email protected]
Newsgroups org.kernel.vger.linux-kselftest,org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <97e15f09784afbb9da860fcfca52669fae313b681f6d15e729033cbbd30379c2@mail.kernel.org>
> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.