Re: [PATCH bpf-next 14/16] selftests/bpf: Add test for arena percpu-only object

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] selftests/bpf: mismatch between __arena_percpu addr_space_cast and verifier
--

commit 978ace4d7c1590716d35b8badc4a0e110474bd79
Author: Xu Kuohai <xukuohai@huawei.com>

selftests/bpf: Add test for arena percpu-only object

This commit adds the __arena_percpu macro to bpf_arena_common.h along with a
selftest for an object containing only an arena per-cpu variable.

> diff --git a/tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h b/tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h
> index 82aafe879fae2..7cdee141764e7 100644
> --- a/tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h
> +++ b/tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h
> @@ -35,11 +35,13 @@
>  #if defined(__BPF_FEATURE_ADDR_SPACE_CAST) && !defined(BPF_ARENA_FORCE_ASM)
>  #define __arena __attribute__((address_space(1))) __attribute__((btf_type_tag("arena")))
>  #define __arena_global __attribute__((address_space(1)))
> +#define __arena_percpu __attribute__((address_space(1))) __attribute__((section(".arena.percpu")))
>  #define cast_kern(ptr) /* nop for bpf prog. emitted by LLVM */
>  #define cast_user(ptr) /* nop for bpf prog. emitted by LLVM */
>  #else
>  #define __arena __attribute__((btf_type_tag("arena")))
>  #define __arena_global SEC(".addr_space.1")
> +#define __arena_percpu SEC(".arena.percpu")
>  #define cast_kern(ptr) bpf_addr_space_cast(ptr, 0, 1)
>  #define cast_user(ptr) bpf_addr_space_cast(ptr, 1, 0)
>  #endif

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/arena_percpu_only.c b/tools/testing/selftests/bpf/progs/arena_percpu_only.c
> new file mode 100644
> index 0000000000000..35e4ff983e65d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/arena_percpu_only.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Huawei Technologies Inc. */
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_arena_common.h"
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_ARENA);
> +	__uint(map_flags, BPF_F_MMAPABLE);
> +} arena SEC(".maps");
> +
> +__u64 __arena_percpu counter = 0x123456;
> +
> +SEC("syscall")
> +int arena_percpu_only_write(void *ctx)
> +{
> +	counter = bpf_get_smp_processor_id();
> +	return 0;
> +}

[Severity: High]
How does __arena_percpu interact with the verifier and JITs across the two
branches in tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h?

First, when compiled without __BPF_FEATURE_ADDR_SPACE_CAST (such as under
test_progs-bpf_gcc or when BPF_ARENA_FORCE_ASM is defined), __arena_percpu is
defined without address_space(1), and arena_percpu_only_write() assigns to
counter unconditionally without #ifdef __BPF_FEATURE_ADDR_SPACE_CAST or
cast_kern().

In that configuration, the compiler emits a BPF_LD | BPF_IMM | BPF_DW
instruction (relocated by libbpf to BPF_PSEUDO_ARENA_PERCPU) followed directly
by BPF_STX | BPF_MEM | BPF_DW without an rX = addr_space_cast(rX, 0, 1)
instruction. Unlike BPF_PSEUDO_MAP_VALUE on an arena map (which check_ld_imm()
marks as SCALAR_VALUE so the verifier rejects uncast dereferences),
check_ld_imm() in kernel/bpf/verifier.c marks BPF_PSEUDO_ARENA_PERCPU directly
as PTR_TO_ARENA, and bpf_convert_ctx_accesses() rewrites the store to
BPF_STX | BPF_PROBE_MEM32 | BPF_DW.

However, check_and_resolve_insns() resolves the 64-bit immediate of
BPF_PSEUDO_ARENA_PERCPU to the full 64-bit arena->user_vm_start + off (with
insn[1].imm = addr >> 32), and the x86 and arm64 JITs load this full 64-bit
immediate and add smp_processor_id() * chunk_sz using 64-bit addition without
clearing the upper 32 bits. Without addr_space_cast(rX, 0, 1) (which
bpf_do_misc_fixups() rewrites to a 32-bit wX = wY move to zero the upper 32
bits), the upper 32 bits of the PTR_TO_ARENA register remain
user_vm_start >> 32. When test_arena_percpu_only() in
tools/testing/selftests/bpf/prog_tests/arena_percpu.c runs
arena_percpu_only_write(), BPF_PROBE_MEM32 adds the 64-bit kern_vm_start to
this 64-bit register, targeting kern_vm_start + user_vm_start + off +
cpu * chunk_sz outside the arena's 4 GiB kernel vmalloc area.

Second, when __BPF_FEATURE_ADDR_SPACE_CAST is defined, __arena_percpu has
__attribute__((address_space(1))), which causes Clang to emit
rX = addr_space_cast(rX, 0, 1) immediately after BPF_PSEUDO_ARENA_PERCPU. In
check_alu_op() in kernel/bpf/verifier.c, addr_space_cast calls
mark_reg_unknown() on dst_reg, which zeroes dst_reg->mem_size and resets the
chunk offset pos set by check_ld_imm():

kernel/bpf/verifier.c:check_alu_op() {
    ...
	if (bpf_insn_is_addr_space_cast(insn)) {
		...
		/* convert from ptr_to_arena to scalar or vice versa */
		mark_reg_unknown(env, regs, insn->dst_reg);
    ...
}

Doesn't this wipe out dst_reg->mem_size and bypass the per-cpu chunk bounds
check in check_mem_access()?

> +
> +char LICENSE[] SEC("license") = "GPL";

[ ... ]

>
>
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792503001.git.xukuohai@huawei.com?part=14