mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH bpf-next v3] selftests/bpf: Add arena direct-value one-past-end reject test
@ 2026-06-10  7:13 Woojin Ji
  2026-06-11 23:57 ` Emil Tsalapatis
  0 siblings, 1 reply; 2+ messages in thread
From: Woojin Ji @ 2026-06-10  7:13 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Song Liu, Yonghong Song, Jiri Olsa, Shuah Khan
  Cc: linux-kernel, bpf, linux-kselftest, Emil Tsalapatis,
	Junyoung Jang, Woojin Ji

BPF_MAP_TYPE_ARENA supports direct-value pseudo loads, but unlike array
maps its map value_size is zero and the valid direct-value range is the
arena mmap size, max_entries * PAGE_SIZE.

Commit 3ac1a467e376 ("bpf: Fix off-by-one boundary validation in arena
direct-value access") fixed arena_map_direct_value_addr() to reject an
offset exactly at the end of the arena mapping. Add a regression test
that loads a BPF_PSEUDO_MAP_VALUE with off == arena_size and verifies
that the verifier rejects it with the expected offset in the log.

This is intentionally kept as a userspace raw-instruction test. I tried
expressing the same BPF_PSEUDO_MAP_VALUE + off == arena_size case in
verifier_arena.c with inline assembly. The only form that produces the
desired instruction bytes uses __imm_addr(arena), but that emits
R_BPF_64_NODYLD32, which the libbpf/bpftool link step rejects. Other
register, immediate, and memory constraints either fail in the BPF
backend or lower to a normal R_BPF_64_64 load followed by an ALU add,
which does not exercise arena_map_direct_value_addr() with the boundary
offset in the second ldimm64 slot.

A legacy test_verifier fixture can express the raw instruction directly,
but it needs arena map creation, mmap, and fixup plumbing in the legacy
runner. That is more intrusive than the small prog_tests raw-instruction
test.

Use the userspace raw-instruction test, following the existing selftests
pattern used for direct map-value pseudo loads, so insns[1].imm can be
set to arena_size precisely.

Assisted-by: ChatGPT:gpt-5.5
Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
Cc: Emil Tsalapatis <emil@etsalapatis.com>
Cc: Junyoung Jang <graypanda.inzag@gmail.com>
---
Changes in v3:
- Document why verifier_arena.c inline assembly cannot express this case:
  __imm_addr emits R_BPF_64_NODYLD32, while other constraints either fail
  in the BPF backend or miss the pseudo-load boundary path.
- Mention that natural C forms lower to a normal load plus ALU add, so they
  do not exercise arena_map_direct_value_addr() with off == arena_size.
- Keep the userspace raw-instruction test as the least intrusive option;
  test_verifier was checked but would require legacy runner arena plumbing.
- Link to v2: https://patch.msgid.link/20260605-arena-direct-value-v1-v2-1-a92cb281e376@gmail.com

Changes in v2:
- Explain why this uses a userspace raw-instruction test rather than a
  verifier_arena.c failure program: the test must set the second
  BPF_PSEUDO_MAP_VALUE ldimm64 immediate to arena_size exactly. For
  arena globals, libbpf derives that immediate from the arena-relative
  symbol offset, so the boundary value cannot be represented directly as
  a verifier_arena.c C-level failure program.
- Link to v1: https://patch.msgid.link/20260603-arena-direct-value-v1-v1-1-7b31cc0a8cac@gmail.com

Tested on x86_64 with tools/testing/selftests/bpf/vmtest.sh:
  $ ./test_progs -t arena_direct_value
  #2/1     arena_direct_value/one_past_end:OK
  #2       arena_direct_value:OK
  Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
---
 .../selftests/bpf/prog_tests/arena_direct_value.c  | 73 ++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/arena_direct_value.c b/tools/testing/selftests/bpf/prog_tests/arena_direct_value.c
new file mode 100644
index 000000000000..b7760b021670
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/arena_direct_value.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include <bpf/bpf.h>
+#include <errno.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+#define ARENA_PAGES 32
+
+static void test_arena_direct_value_one_past_end(void)
+{
+	char log_buf[16384] = {}, expected[128];
+	__u32 arena_sz = ARENA_PAGES * getpagesize();
+	struct bpf_insn insns[] = {
+		BPF_LD_IMM64_RAW(BPF_REG_1, BPF_PSEUDO_MAP_VALUE, 0),
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	};
+	LIBBPF_OPTS(bpf_map_create_opts, map_opts);
+	LIBBPF_OPTS(bpf_prog_load_opts, prog_opts);
+	void *arena = MAP_FAILED;
+	int map_fd, prog_fd;
+
+	map_opts.map_flags = BPF_F_MMAPABLE;
+	prog_opts.log_buf = log_buf;
+	prog_opts.log_size = sizeof(log_buf);
+	prog_opts.log_level = 1;
+
+	map_fd = bpf_map_create(BPF_MAP_TYPE_ARENA, "arena_direct_value",
+				0, 0, ARENA_PAGES, &map_opts);
+	if (map_fd < 0) {
+		if (errno == EOPNOTSUPP || errno == EINVAL) {
+			test__skip();
+			return;
+		}
+		ASSERT_GE(map_fd, 0, "bpf_map_create");
+		return;
+	}
+
+	arena = mmap(NULL, arena_sz, PROT_READ | PROT_WRITE, MAP_SHARED, map_fd, 0);
+	if (!ASSERT_NEQ(arena, MAP_FAILED, "arena_mmap"))
+		goto cleanup;
+
+	insns[0].imm = map_fd;
+	insns[1].imm = arena_sz;
+
+	prog_fd = bpf_prog_load(BPF_PROG_TYPE_RAW_TRACEPOINT,
+				"arena_direct_value", "GPL", insns,
+				ARRAY_SIZE(insns), &prog_opts);
+	if (!ASSERT_LT(prog_fd, 0, "prog_load")) {
+		if (prog_fd >= 0)
+			close(prog_fd);
+		prog_fd = -1;
+		goto cleanup;
+	}
+
+	snprintf(expected, sizeof(expected),
+		 "invalid access to map value pointer, value_size=0 off=%u",
+		 arena_sz);
+	ASSERT_HAS_SUBSTR(log_buf, expected, "verifier_log");
+
+cleanup:
+	if (arena != MAP_FAILED)
+		munmap(arena, arena_sz);
+	close(map_fd);
+}
+
+void test_arena_direct_value(void)
+{
+	if (test__start_subtest("one_past_end"))
+		test_arena_direct_value_one_past_end();
+}

---
base-commit: ba3e43a9e601636f5edb54e259a74f96ca3b8fd8
change-id: 20260603-arena-direct-value-v1-ef4df857b98b

Best regards,
--  
Woojin Ji <random6.xyz@gmail.com>


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH bpf-next v3] selftests/bpf: Add arena direct-value one-past-end reject test
  2026-06-10  7:13 [PATCH bpf-next v3] selftests/bpf: Add arena direct-value one-past-end reject test Woojin Ji
@ 2026-06-11 23:57 ` Emil Tsalapatis
  0 siblings, 0 replies; 2+ messages in thread
From: Emil Tsalapatis @ 2026-06-11 23:57 UTC (permalink / raw)
  To: Woojin Ji, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Song Liu, Yonghong Song, Jiri Olsa, Shuah Khan
  Cc: linux-kernel, bpf, linux-kselftest, Emil Tsalapatis, Junyoung Jang

On Wed Jun 10, 2026 at 3:13 AM EDT, Woojin Ji wrote:
> BPF_MAP_TYPE_ARENA supports direct-value pseudo loads, but unlike array
> maps its map value_size is zero and the valid direct-value range is the
> arena mmap size, max_entries * PAGE_SIZE.
>
> Commit 3ac1a467e376 ("bpf: Fix off-by-one boundary validation in arena
> direct-value access") fixed arena_map_direct_value_addr() to reject an
> offset exactly at the end of the arena mapping. Add a regression test
> that loads a BPF_PSEUDO_MAP_VALUE with off == arena_size and verifies
> that the verifier rejects it with the expected offset in the log.
>
> This is intentionally kept as a userspace raw-instruction test. I tried
> expressing the same BPF_PSEUDO_MAP_VALUE + off == arena_size case in
> verifier_arena.c with inline assembly. The only form that produces the
> desired instruction bytes uses __imm_addr(arena), but that emits
> R_BPF_64_NODYLD32, which the libbpf/bpftool link step rejects. Other
> register, immediate, and memory constraints either fail in the BPF
> backend or lower to a normal R_BPF_64_64 load followed by an ALU add,
> which does not exercise arena_map_direct_value_addr() with the boundary
> offset in the second ldimm64 slot.
>
> A legacy test_verifier fixture can express the raw instruction directly,
> but it needs arena map creation, mmap, and fixup plumbing in the legacy
> runner. That is more intrusive than the small prog_tests raw-instruction
> test.
>
> Use the userspace raw-instruction test, following the existing selftests
> pattern used for direct map-value pseudo loads, so insns[1].imm can be
> set to arena_size precisely.
>
> Assisted-by: ChatGPT:gpt-5.5
> Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
> Cc: Emil Tsalapatis <emil@etsalapatis.com>
> Cc: Junyoung Jang <graypanda.inzag@gmail.com>

Nits below, after addressing them feel free to add:

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

> ---
> Changes in v3:
> - Document why verifier_arena.c inline assembly cannot express this case:
>   __imm_addr emits R_BPF_64_NODYLD32, while other constraints either fail
>   in the BPF backend or miss the pseudo-load boundary path.
> - Mention that natural C forms lower to a normal load plus ALU add, so they
>   do not exercise arena_map_direct_value_addr() with off == arena_size.
> - Keep the userspace raw-instruction test as the least intrusive option;
>   test_verifier was checked but would require legacy runner arena plumbing.
> - Link to v2: https://patch.msgid.link/20260605-arena-direct-value-v1-v2-1-a92cb281e376@gmail.com
>
> Changes in v2:
> - Explain why this uses a userspace raw-instruction test rather than a
>   verifier_arena.c failure program: the test must set the second
>   BPF_PSEUDO_MAP_VALUE ldimm64 immediate to arena_size exactly. For
>   arena globals, libbpf derives that immediate from the arena-relative
>   symbol offset, so the boundary value cannot be represented directly as
>   a verifier_arena.c C-level failure program.
> - Link to v1: https://patch.msgid.link/20260603-arena-direct-value-v1-v1-1-7b31cc0a8cac@gmail.com
>
> Tested on x86_64 with tools/testing/selftests/bpf/vmtest.sh:
>   $ ./test_progs -t arena_direct_value
>   #2/1     arena_direct_value/one_past_end:OK
>   #2       arena_direct_value:OK
>   Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
> ---
>  .../selftests/bpf/prog_tests/arena_direct_value.c  | 73 ++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/arena_direct_value.c b/tools/testing/selftests/bpf/prog_tests/arena_direct_value.c
> new file mode 100644
> index 000000000000..b7760b021670
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/arena_direct_value.c
> @@ -0,0 +1,73 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <test_progs.h>
> +#include <bpf/bpf.h>
> +#include <errno.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +#define ARENA_PAGES 32
> +
> +static void test_arena_direct_value_one_past_end(void)
> +{
> +	char log_buf[16384] = {}, expected[128];

Since the log_buf is huge, and the file has a simple test,
just move it to the BSS.

> +	__u32 arena_sz = ARENA_PAGES * getpagesize();
> +	struct bpf_insn insns[] = {
> +		BPF_LD_IMM64_RAW(BPF_REG_1, BPF_PSEUDO_MAP_VALUE, 0),
> +		BPF_MOV64_IMM(BPF_REG_0, 0),
> +		BPF_EXIT_INSN(),
> +	};
> +	LIBBPF_OPTS(bpf_map_create_opts, map_opts);
> +	LIBBPF_OPTS(bpf_prog_load_opts, prog_opts);
> +	void *arena = MAP_FAILED;

No need to initialize this, we have not code paths where we read the
variable before it gets assigned by mmap().

> +	int map_fd, prog_fd;
> +
> +	map_opts.map_flags = BPF_F_MMAPABLE;
> +	prog_opts.log_buf = log_buf;
> +	prog_opts.log_size = sizeof(log_buf);
> +	prog_opts.log_level = 1;
> +
> +	map_fd = bpf_map_create(BPF_MAP_TYPE_ARENA, "arena_direct_value",
> +				0, 0, ARENA_PAGES, &map_opts);
> +	if (map_fd < 0) {
> +		if (errno == EOPNOTSUPP || errno == EINVAL) {

Question: Why is an EINVAL on map creation a skippable event? What failure mode
does it correspond to?

> +			test__skip();
> +			return;
> +		}
> +		ASSERT_GE(map_fd, 0, "bpf_map_create");
> +		return;
> +	}
> +
> +	arena = mmap(NULL, arena_sz, PROT_READ | PROT_WRITE, MAP_SHARED, map_fd, 0);
> +	if (!ASSERT_NEQ(arena, MAP_FAILED, "arena_mmap"))
> +		goto cleanup;
> +
> +	insns[0].imm = map_fd;
> +	insns[1].imm = arena_sz;
> +
> +	prog_fd = bpf_prog_load(BPF_PROG_TYPE_RAW_TRACEPOINT,
> +				"arena_direct_value", "GPL", insns,
> +				ARRAY_SIZE(insns), &prog_opts);
> +	if (!ASSERT_LT(prog_fd, 0, "prog_load")) {
> +		if (prog_fd >= 0)

If we only take this path if prog_fd >= 0, why the extra check
internally?

> +			close(prog_fd);
> +		prog_fd = -1;

No need to set this, we never use it after this.

> +		goto cleanup;
> +	}
> +
> +	snprintf(expected, sizeof(expected),
> +		 "invalid access to map value pointer, value_size=0 off=%u",
> +		 arena_sz);
> +	ASSERT_HAS_SUBSTR(log_buf, expected, "verifier_log");
> +
> +cleanup:
> +	if (arena != MAP_FAILED)
> +		munmap(arena, arena_sz);
> +	close(map_fd);
> +}
> +
> +void test_arena_direct_value(void)
> +{
> +	if (test__start_subtest("one_past_end"))
> +		test_arena_direct_value_one_past_end();
> +}
>
> ---
> base-commit: ba3e43a9e601636f5edb54e259a74f96ca3b8fd8
> change-id: 20260603-arena-direct-value-v1-ef4df857b98b
>
> Best regards,
> --  
> Woojin Ji <random6.xyz@gmail.com>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-11 23:57 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-10  7:13 [PATCH bpf-next v3] selftests/bpf: Add arena direct-value one-past-end reject test Woojin Ji
2026-06-11 23:57 ` Emil Tsalapatis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®