Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> writes:
> On Thu, 7 Aug 2025 at 15:25, <puranjay@xxxxxxxxxx> wrote:
>>
>> Yonghong Song <yonghong.song@xxxxxxxxx> writes:
>>
>> > On 8/6/25 1:58 AM, Puranjay Mohan wrote:
>> >> Add selftests for testing the reporting of arena page faults through BPF
>> >> streams. Two new bpf programs are added that read and write to an
>> >> unmapped arena address and the fault reporting is verified in the
>> >> userspace through streams.
>> >>
>> >> Signed-off-by: Puranjay Mohan <puranjay@xxxxxxxxxx>
>> >> ---
>> >> .../testing/selftests/bpf/prog_tests/stream.c | 24 ++++++++++++
>> >> tools/testing/selftests/bpf/progs/stream.c | 37 +++++++++++++++++++
>> >> 2 files changed, 61 insertions(+)
>> >>
>> >> diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
>> >> index d9f0185dca61b..4bdde56de35b1 100644
>> >> --- a/tools/testing/selftests/bpf/prog_tests/stream.c
>> >> +++ b/tools/testing/selftests/bpf/prog_tests/stream.c
>> >> @@ -41,6 +41,22 @@ struct {
>> >> "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
>> >> "|[ \t]+[^\n]+\n)*",
>> >> },
>> >> + {
>> >> + offsetof(struct stream, progs.stream_arena_read_fault),
>> >> + "ERROR: Arena READ access at unmapped address 0x.*\n"
>> >> + "CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
>> >> + "Call trace:\n"
>> >> + "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
>> >> + "|[ \t]+[^\n]+\n)*",
>> >> + },
>> >> + {
>> >> + offsetof(struct stream, progs.stream_arena_write_fault),
>> >> + "ERROR: Arena WRITE access at unmapped address 0x.*\n"
>> >> + "CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
>> >> + "Call trace:\n"
>> >> + "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
>> >> + "|[ \t]+[^\n]+\n)*",
>> >> + },
>> >> };
>> >>
>> >> static int match_regex(const char *pattern, const char *string)
>> >> @@ -85,6 +101,14 @@ void test_stream_errors(void)
>> >> continue;
>> >> }
>> >> #endif
>> >> +#if !defined(__x86_64__) && !defined(__aarch64__)
>> >> + ASSERT_TRUE(1, "Arena fault reporting unsupported, skip.");
>> >> + if (i == 2 || i == 3) {
>> >> + ret = bpf_prog_stream_read(prog_fd, 2, buf, sizeof(buf), &ropts);
>> >> + ASSERT_EQ(ret, 0, "stream read");
>> >> + continue;
>> >> + }
>> >> +#endif
>> >>
>> >> ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDERR, buf, sizeof(buf), &ropts);
>> >> ASSERT_GT(ret, 0, "stream read");
>> >> diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
>> >> index 35790897dc879..58ebff60cd96a 100644
>> >> --- a/tools/testing/selftests/bpf/progs/stream.c
>> >> +++ b/tools/testing/selftests/bpf/progs/stream.c
>> >> @@ -1,10 +1,15 @@
>> >> // SPDX-License-Identifier: GPL-2.0
>> >> /* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
>> >> +#define BPF_NO_KFUNC_PROTOTYPES
>> >
>> > Do we have to defineBPF_NO_KFUNC_PROTOTYPES in the above? Without the above, we do not need
>> > below extern bpf_res_spin_lock and bpf_res_spin_unlock.
>> >
>>
>> If we don't define BPF_NO_KFUNC_PROTOTYPES then there are build failures
>> for bpf_arena_alloc/free_pages() because the prototypes in vmlinux.h
>> lack __arena attribute.
>
> I would address this by dropping the alloc/free.
> Instead to work around "addr_space_cast insn in program without arena error",
> insert a dummy store "ptr = &arena" in the program, where ptr is a
> global void *.
>
I want to use alloc/free and not use a dummy address because because
arena pointers are special as they are returned by alloc() with
arena->user_vm_start added to them, and the
bpf_prog_report_arena_violation() also adds back arena->user_vm_start to
the 32 bit address received by the fault handler. If I use a random
address in the bpf program, bpf_prog_report_arena_violation() will print
a bogus address.
So, I think we should keep using alloc/free for this test because we
want to test this arena->user_vm_start addition as well.
Thanks,
Puranjay
