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

[Alexis Lothoré]

On Fri Aug 28, 2026 at 12:30 PM CEST, bot+bpf-ci wrote:
>> 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?

Even for no_alu32, the __sync_fetch_and_or is properly generated:

  $ llvm-objdump --disassemble-symbols=simple_atomic_fetch_on_stack tools/testing/selftests/bpf/no_alu32/kasan.bpf.o
  
  tools/testing/selftests/bpf/no_alu32/kasan.bpf.o:       file format elf64-bpf
  
  Disassembly of section tcx/ingress:
  
  00000000000006d0 <simple_atomic_fetch_on_stack>:
       218:       bf a6 00 00 00 00 00 00 r6 = r10
       219:       07 06 00 00 f0 ff ff ff r6 += -0x10
       220:       bf 61 00 00 00 00 00 00 r1 = r6
       221:       b7 02 00 00 10 00 00 00 r2 = 0x10
       222:       85 10 00 00 ff ff ff ff call -0x1
       223:       b7 01 00 00 08 00 00 00 r1 = 0x8
       224:       db 1a f8 ff 41 00 00 00 r1 = atomic_fetch_or((u64 *)(r10 - 0x8), r1)
       225:       bf 61 00 00 00 00 00 00 r1 = r6
       226:       b7 02 00 00 10 00 00 00 r2 = 0x10
       227:       85 10 00 00 ff ff ff ff call -0x1
       228:       b7 00 00 00 00 00 00 00 r0 = 0x0
       229:       95 00 00 00 00 00 00 00 exit


My understanding is that _32 bits_ atomics aren't available with
-mcpu=v2, but 64 bits atomic fetch operations are, so that's why I stuck
with a DW operation for the simple_atomic_fetch(_not)_on_stack tests.

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




-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com