Re: [PATCH bpf-next 6/6] selftests/bpf: Add arena fault tests for atomics with fetch
Eduard Zingerman <[email protected]>
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 2026-08-10 at 15:43 +0200, Daniel Borkmann wrote: > Add stream_arena_xchg_fault and stream_arena_cmpxchg_fault next to the > existing read, write and load-acquire fault tests, covering the two > places a read-modify-write can deposit the old value: src_reg for a > BPF_XCHG and r0 for a BPF_CMPXCHG. Both cover both halves of the JIT > bug that left the fetch destination alone when a RMW on an arena pointer > faulted: > > - the fault has to be reported as a WRITE, and at the address held by > the destination register, which __stderr() and test_address() check > - the register receiving the fetched value has to be cleared by the > fault handler, which the programs check by poisoning it before the > atomic and returning it, so __retval(0) fails if it is left untouched > > Note, the atomics are open coded since linux/filter.h cannot be included > alongside vmlinux.h. > > # LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t stream > [...] > #464/1 stream_arena_fault_address/read_fault:OK > #464/2 stream_arena_fault_address/write_fault:OK > #464/3 stream_arena_fault_address/load_acquire_fault:OK > #464/4 stream_arena_fault_address/xchg_fault:OK > #464/5 stream_arena_fault_address/cmpxchg_fault:OK > #464 stream_arena_fault_address:OK > [...] > #466/5 stream_success/stream_arena_write_fault:OK > #466/6 stream_success/stream_arena_read_fault:OK > #466/7 stream_success/stream_arena_load_acquire_fault:OK > #466/8 stream_success/stream_arena_xchg_fault:OK > #466/9 stream_success/stream_arena_cmpxchg_fault:OK > [...] > Summary: 4/22 PASSED, 0 SKIPPED, 0 FAILED > > Signed-off-by: Daniel Borkmann <[email protected]> > --- Acked-by: Eduard Zingerman <[email protected]> ... > diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c > index 15dd3ae2a84b..e4e9374309e2 100644 > --- a/tools/testing/selftests/bpf/prog_tests/stream.c > +++ b/tools/testing/selftests/bpf/prog_tests/stream.c > @@ -105,6 +105,10 @@ void test_stream_arena_fault_address(void) > test_address(skel->progs.stream_arena_write_fault, &skel->bss->fault_addr); > if (test__start_subtest("load_acquire_fault")) > test_address(skel->progs.stream_arena_load_acquire_fault, &skel->bss->fault_addr); > + if (test__start_subtest("xchg_fault")) > + test_address(skel->progs.stream_arena_xchg_fault, &skel->bss->fault_addr); > + if (test__start_subtest("cmpxchg_fault")) > + test_address(skel->progs.stream_arena_cmpxchg_fault, &skel->bss->fault_addr); Tbh, this seem to be an overkill. The point of these stream tests is to check the stream output format details etc. Here the __stderr annotation in the test already captures that the fault had happened and also checks retval. ...