Re: [PATCH 3/5] docs/bpf: Document pinned arena file mappings

[bot+bpf-ci@kernel.org]

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

> docs/bpf: Document pinned arena file mappings

TL;DR:
- Doc says a canonical arena mapping may start at address zero, but the
  verifier may then reject programs using it
- Doc claim about shorter mappings of non-exported arenas looks wrong when
  map_extra is nonzero
- Doc mentions an unreachable read-only restriction

> diff --git a/Documentation/bpf/map_arena.rst b/Documentation/bpf/map_arena.rst
> new file mode 100644
> index 000000000000..1b9eadd5fb3a
> --- /dev/null
> +++ b/Documentation/bpf/map_arena.rst
> @@ -0,0 +1,131 @@

[ ... ]

> +* Alternatively, leave ``map_extra`` zero and mmap the map FD first. This
> +  mapping establishes the canonical address and must cover the full map
> +  capacity. Shorter canonical mappings fail with ``EINVAL``.
> +
> +A canonical map FD mapping may start at address zero if the system's
> +low-address mapping policy permits it.

Can a BPF program use an arena in that state?  The BPF_MAP_TYPE_ARENA
handling in kernel/bpf/verifier.c still treats user_vm_start == 0 as unset:

    if (!bpf_arena_get_user_vm_start(env->prog->aux->arena)) {
        verbose(env, "arena's user address must be set via map_extra or mmap()\n");
        return -EINVAL;
    }

Consider an arena created with BPF_F_MMAPABLE, BPF_F_ARENA_NO_FREE and
BPF_F_ARENA_EXPORT, with map_extra == 0, followed by:

    mmap(0, capacity, ..., MAP_SHARED | MAP_FIXED_NOREPLACE, map_fd, 0)

With mmap_min_addr == 0 this succeeds, since arena_get_unmapped_area() and
arena_map_mmap() in kernel/bpf/arena.c now test user_vm_end for an
established range.  That leaves user_vm_start = 0 and user_vm_end = capacity,
and loading any BPF program that references the arena then fails with
-EINVAL.

Should the verifier check use the same "established" test, or should this
paragraph say that such an arena cannot be used by BPF programs?

[ ... ]

> +Once the canonical range is established, it covers the entire capacity
> +reported by ``fstat()``. Consumers can map the full file or smaller slices.
> +Establishing the full canonical mapping reserves virtual address space;
> +it does not populate every arena page. Arenas without
> +``BPF_F_ARENA_EXPORT`` can still establish shorter canonical mappings.

Is that last sentence true for every arena without BPF_F_ARENA_EXPORT?

For a non-exported arena created with a nonzero map_extra,
arena_map_alloc() sets user_vm_end = map_extra + max_entries * PAGE_SIZE at
creation, so the canonical range is already the full capacity.

arena_get_unmapped_area() then takes the "if (arena->user_vm_end)" branch and
rejects any len != user_vm_end - user_vm_start with -EINVAL (-E2BIG if
larger), and arena_map_mmap() returns -EBUSY when user_vm_end !=
vma->vm_end.  A shorter map FD mapping of such an arena fails, which also
conflicts with the earlier bullet saying a nonzero map_extra makes the range
cover the full map capacity.

Should the sentence be qualified with "when map_extra is zero"?

[ ... ]

> +Opening the pin uses ordinary filesystem permissions and file LSM checks,
> +as well as ``security_bpf_map()`` for the requested access mode. Mapping it
> +also passes the ordinary mmap LSM checks. Writable mappings remain subject
> +to the BPF map's frozen state and program-read-only restrictions.

Can the "program-read-only restrictions" apply to an arena?

bpf_map_mmap() does reject writable mappings of maps with BPF_F_RDONLY_PROG
(-EACCES), but arena_map_alloc() only accepts BPF_F_SEGV_ON_FAULT,
BPF_F_MMAPABLE, BPF_F_NO_USER_CONV, BPF_F_ARENA_NO_FREE and
BPF_F_ARENA_EXPORT.  Creating an arena with BPF_F_RDONLY_PROG fails with
-EINVAL, and nothing sets the flag afterwards.

Only the frozen state (BPF_MAP_FREEZE, -EPERM on a writable mapping) is a
live restriction for exported arena mappings.  Should the phrase be dropped,
or reworded to say that only the frozen state applies?


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