Re: [PATCH bpf-next v2 2/3] selftests/bpf: Test fetching AND/OR/XOR atomics in arena

[bot+bpf-ci@kernel.org]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/arena_atomics.c b/tools/testing/selftests/bpf/prog_tests/arena_atomics.c
> index 1ad5d03d07ad..42061e70e1a3 100644
> --- a/tools/testing/selftests/bpf/prog_tests/arena_atomics.c
> +++ b/tools/testing/selftests/bpf/prog_tests/arena_atomics.c

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/arena_atomics.c b/tools/testing/selftests/bpf/progs/arena_atomics.c
> index 73bc2b835f3f..9a3eee526cc9 100644
> --- a/tools/testing/selftests/bpf/progs/arena_atomics.c
> +++ b/tools/testing/selftests/bpf/progs/arena_atomics.c

[ ... ]

> @@ -213,6 +198,64 @@ int xchg(const void *ctx)
>  	return 0;
>  }
>
> +__u64 __arena_global fetch_src_r0_value = 0x110;
> +__u64 __arena_global fetch_src_r0_result = 0;
> +__u64 __arena_global fetch_dst_r0_value = 0x110;
> +__u64 __arena_global fetch_dst_r0_result = 0;
> +__u64 __arena_global fetch_dst_r0_readback = 0;
> +
> +/*
> + * A fetching OR with the operand in r0, and one with the arena pointer in r0.
> + * The x86 JIT needs RAX for its CMPXCHG loop and substitutes BPF_REG_AX for
> + * whichever of the two is r0, so both have to keep working. Hand-written
> + * because clang picks its own registers and will not reliably emit either.
> + */
> +SEC("raw_tp/sys_enter")
> +int fetch_r0(const void *ctx)
> +{
> +	if (pid != (bpf_get_current_pid_tgid() >> 32))
> +		return 0;
> +#if defined(ENABLE_ATOMICS_TESTS) && defined(__BPF_FEATURE_ADDR_SPACE_CAST)
> +	asm volatile (
> +	"r1 = %[fetch_src_r0_value] ll;"
> +	"r1 = addr_space_cast(r1, 0x0, 0x1);"

[ ... ]

> @@ -221,15 +264,20 @@ int uaf(const void *ctx)
>  {
>  	if (pid != (bpf_get_current_pid_tgid() >> 32))
>  		return 0;
> -#if defined(ENABLE_ATOMICS_TESTS) && !defined(__TARGET_ARCH_arm64) && \
> -    !defined(__TARGET_ARCH_x86)
> +/*
> + * arm64 stays excluded as it has been since this test was added: its JIT only
> + * accepts arena RMW atomics when LSE is present, which is a run-time property.
> + * The rest of the file shares that dependency without a guard, so on a non-LSE
> + * arm64 the whole skeleton already fails to load.
> + */
> +#if defined(ENABLE_ATOMICS_TESTS) && !defined(__TARGET_ARCH_arm64)
>  	__u32 __arena *page32;
>  	__u64 __arena *page64;
>  	void __arena *page;
>
>  	page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
>  	bpf_arena_free_pages(&arena, page, 1);
> -	uaf_recovery_fails = 24;
> +	uaf_recovery_fails = 26;

[ ... ]

> +
> +	/*
> +	 * The x86 JIT needs RAX for the CMPXCHG loop it lowers a fetching
> +	 * AND/OR/XOR into, and substitutes BPF_REG_AX for r0 when r0 is either
> +	 * operand. Only then does the fault fixup have to name that register,
> +	 * so spell both forms out: clang picks its own and never produces them.
> +	 */
> +	asm volatile (
> +	"r0 = %[page];"
> +	"r0 = addr_space_cast(r0, 0x0, 0x1);"
> +	"r1 = 1;"
> +	".8byte %[fetch_dst_r0];"
> +	: : [page]"r"(page64),
> +	    __imm_insn(fetch_dst_r0,
> +		       BPF_ATOMIC_OP(BPF_DW, BPF_OR | BPF_FETCH, BPF_REG_0, BPF_REG_1, 0))
> +	: "r0", "r1", "memory");
> +	uaf_recovery_fails -= 1;
> +
> +	asm volatile (
> +	"r1 = %[page];"
> +	"r1 = addr_space_cast(r1, 0x0, 0x1);"
> +	"r0 = 1;"
> +	".8byte %[fetch_src_r0];"
> +	: : [page]"r"(page64),
> +	    __imm_insn(fetch_src_r0,
> +		       BPF_ATOMIC_OP(BPF_DW, BPF_OR | BPF_FETCH, BPF_REG_1, BPF_REG_0, 0))
> +	: "r0", "r1", "memory");
> +	uaf_recovery_fails -= 1;
>  #endif

Would this build with clang 18 or earlier?  Both new asm blocks in uaf()
use the textual addr_space_cast syntax, but uaf() is only guarded by
ENABLE_ATOMICS_TESTS and !__TARGET_ARCH_arm64.  It does not check for
__BPF_FEATURE_ADDR_SPACE_CAST.

Every other use of textual addr_space_cast in this file is under that
guard: fetch_r0() above at the new code, and load_acquire/store_release
elsewhere.  The addr_space_cast asm syntax and __BPF_FEATURE_ADDR_SPACE_CAST
both arrived in LLVM 19, so with clang 18 the integrated assembler would
reject the unknown mnemonic and arena_atomics.bpf.o would fail to build.

Before this patch uaf() was plain C and was compiled out on x86, so the
asm never reached older clang there.


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