Re: [PATCH bpf-next v8 8/8] selftests/bpf: add tests to validate KASAN on JIT programs

[bot+bpf-ci]

> diff --git a/tools/testing/selftests/bpf/progs/kasan.c b/tools/testing/selftests/bpf/progs/kasan.c
> new file mode 100644
> index 0000000000000..fe8e0dd228a09
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/kasan.c

[ ... ]

> +SEC("tcx/ingress")
> +int simple_atomic_fetch_on_stack(struct __sk_buff *skb)
> +{
> +	struct kasan_test_val val;
> +
> +	bpf_kfunc_kasan_poison(&val, sizeof(struct kasan_test_val));
> +	__sync_fetch_and_or(&val.data_8, 8);
> +	bpf_kfunc_kasan_unpoison(&val, sizeof(struct kasan_test_val));
> +	return 0;
> +}
> +
> +SEC("tcx/ingress")
> +int simple_atomic_fetch_not_on_stack(struct __sk_buff *skb)
> +{
> +	struct kasan_test_val *val;
> +	__u32 key = 0;
> +
> +	val = bpf_map_lookup_elem(&test_map, &key);
> +	if (!val)
> +		return 0;
> +
> +	bpf_kfunc_kasan_poison(val, sizeof(struct kasan_test_val));
> +	__sync_fetch_and_or(&val->data_8, 8);
> +	bpf_kfunc_kasan_unpoison(val, sizeof(struct kasan_test_val));
> +	return 0;
> +}

Can these two programs build with the no_alu32 flavor? Looking at
Documentation/bpf/clang-notes.rst, __sync_fetch_and_or() needs -mcpu=v3
for the BPF_OR atomic instruction, but the no_alu32 build uses plain
-mcpu=v2 (tools/testing/selftests/bpf/Makefile:497-501). Clang at v2 can
only generate BPF_ADD atomics without BPF_FETCH, so BPF_OR should be an
instruction-selection error.

The sibling programs simple_atomic_on_stack and simple_atomic_not_on_stack
use __sync_fetch_and_add(), which lowers to BPF_ADD without BPF_FETCH when
the return value is unused, so those build fine. Every other prog in
selftests/bpf/progs/ that uses a non-ADD or fetching atomic guards it with
#ifdef ENABLE_ATOMICS_TESTS (progs/atomics.c, progs/arena_atomics.c,
progs/atomic_bounds.c, progs/tcp_ca_write_sk_pacing.c), and
-DENABLE_ATOMICS_TESTS is only passed to the default and cpuv4 runners, not
to no_alu32.

Would it make sense to guard both simple_atomic_fetch functions with
#ifdef ENABLE_ATOMICS_TESTS, or to verify whether test_progs-no_alu32
actually builds?

Also, since the return value of __sync_fetch_and_or() is discarded here,
does clang emit BPF_ATOMIC | BPF_OR without the BPF_FETCH flag? That would
make these identical to simple_atomic_*, just with OR instead of ADD, and
the JIT path for is_atomic_fetch (the CMPXCHG loop in
arch/x86/net/bpf_jit_comp.c added by bc9fb0ddd5f3b, where the KASAN check
is emitted before branch_target assignment) would not be exercised by
kasan/simple_atomic_fetch_on_stack or kasan/simple_atomic_fetch_not_on_stack.

The commit message shows #175/29 and #175/30 as :OK, but
progs/atomics.c:or64 uses or64_result = __sync_fetch_and_or(...) to force
the fetch variant (checked in prog_tests/atomics.c:98), and keeps a
separate or_noreturn_value program for the discarded-result form. Would
consuming the result here test the intended path?


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