Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests for arena fault reporting

[Puranjay Mohan <puranjay@kernel.org>]

Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:

> On Thu, 7 Aug 2025 at 15:25, <puranjay@kernel.org> wrote:
>>
>> Yonghong Song <yonghong.song@linux.dev> 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@kernel.org>
>> >> ---
>> >>   .../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