* [PATCH v3 0/5] libbpf: move arena variables out of the zero page
@ 2025-12-15 16:13 Emil Tsalapatis
2025-12-15 16:13 ` [PATCH v3 1/5] selftests/bpf: explicitly account for globals in verifier_arena_large Emil Tsalapatis
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Emil Tsalapatis @ 2025-12-15 16:13 UTC (permalink / raw)
To: bpf
Cc: andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song, Emil Tsalapatis
Modify libbpf to place arena globals at the end of the arena mapping
instead of the very beginning. This allows programs to leave the
"zero page" of the arena unmapped, so that NULL arena pointer
dereferences trigger a page fault and associated backtrace in BPF streams.
In contrast, the current policy of placing global data in the zero pages
means that NULL dereferences silently corrupt global data, e.g, arena
qspinlock state. This makes arena bugs more difficult to debug.
The patchset adds code to libbpf to move global arena data to the end of
the arena. At load time, libbpf adjusts each symbol's location within
the arena to point to the right location in the arena. The patchset
also adjusts the arena skeleton pointer to point to the arena globals,
now that they are not in the beginning of the arena region.
CHANGESET
=========
v2->v3: (https://lore.kernel.org/bpf/20251203162625.13152-1-emil@etsalapatis.com/)
- Remove unnecessary kernel bounds check in resolve_pseudo_ldimm64
(Andrii)
- Added patch to turn sym_off unsigned to prevent overflow (AI)
- Remove obsolete references to offsets from test patch description
(Andrii)
- Use size_t for arena_data_off (Andrii)
- Remove extra mutable variable from offset calculations (Andrii)
v1->v2: (https://lore.kernel.org/bpf/20251118030058.162967-1-emil@etsalapatis.com)
- Moved globals to the end of the mapping: (Andrii)
- Removed extra parameter for offset and parameter picking logic
- Removed padding in the skeleton
- Removed additional libbpf call
- Added Reviewed-by from Eduard on patch 1
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
Emil Tsalapatis (5):
selftests/bpf: explicitly account for globals in verifier_arena_large
bpf/verifier: do not limit maximum direct offset into arena map
libbpf: turn relo_core->sym_off unsigned
libbpf: move arena globals to the end of the arena
selftests/bpf: add tests for the arena offset of globals
kernel/bpf/verifier.c | 5 --
tools/lib/bpf/libbpf.c | 21 ++++--
.../selftests/bpf/prog_tests/verifier.c | 4 +
.../bpf/progs/verifier_arena_globals1.c | 75 +++++++++++++++++++
.../bpf/progs/verifier_arena_globals2.c | 49 ++++++++++++
.../bpf/progs/verifier_arena_large.c | 21 +++++-
6 files changed, 160 insertions(+), 15 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
create mode 100644 tools/testing/selftests/bpf/progs/verifier_arena_globals2.c
--
2.49.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 1/5] selftests/bpf: explicitly account for globals in verifier_arena_large
2025-12-15 16:13 [PATCH v3 0/5] libbpf: move arena variables out of the zero page Emil Tsalapatis
@ 2025-12-15 16:13 ` Emil Tsalapatis
2025-12-15 16:13 ` [PATCH v3 2/5] bpf/verifier: do not limit maximum direct offset into arena map Emil Tsalapatis
` (3 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Emil Tsalapatis @ 2025-12-15 16:13 UTC (permalink / raw)
To: bpf
Cc: andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song, Emil Tsalapatis
The big_alloc1 test in verifier_arena_large assumes that the arena base
and the first page allocated by bpf_arena_alloc_pages are identical.
This is not the case, because the first page in the arena is populated
by global arena data. The test still passes because the code makes the
tacit assumption that the first page is on offset PAGE_SIZE instead of
0.
Make this distinction explicit in the code, and adjust the page offsets
requested during the test to count from the beginning of the arena
instead of using the address of the first allocated page.
Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
.../selftests/bpf/progs/verifier_arena_large.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_large.c b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
index f19e15400b3e..bd430a34c3ab 100644
--- a/tools/testing/selftests/bpf/progs/verifier_arena_large.c
+++ b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
@@ -23,18 +23,25 @@ int big_alloc1(void *ctx)
{
#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
volatile char __arena *page1, *page2, *no_page, *page3;
- void __arena *base;
+ u64 base;
- page1 = base = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
+ base = (u64)arena_base(&arena);
+
+ page1 = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
if (!page1)
return 1;
+
+ /* Account for global arena data. */
+ if ((u64)page1 != base + PAGE_SIZE)
+ return 15;
+
*page1 = 1;
- page2 = bpf_arena_alloc_pages(&arena, base + ARENA_SIZE - PAGE_SIZE * 2,
+ page2 = bpf_arena_alloc_pages(&arena, (void __arena *)(ARENA_SIZE - PAGE_SIZE),
1, NUMA_NO_NODE, 0);
if (!page2)
return 2;
*page2 = 2;
- no_page = bpf_arena_alloc_pages(&arena, base + ARENA_SIZE - PAGE_SIZE,
+ no_page = bpf_arena_alloc_pages(&arena, (void __arena *)ARENA_SIZE,
1, NUMA_NO_NODE, 0);
if (no_page)
return 3;
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 2/5] bpf/verifier: do not limit maximum direct offset into arena map
2025-12-15 16:13 [PATCH v3 0/5] libbpf: move arena variables out of the zero page Emil Tsalapatis
2025-12-15 16:13 ` [PATCH v3 1/5] selftests/bpf: explicitly account for globals in verifier_arena_large Emil Tsalapatis
@ 2025-12-15 16:13 ` Emil Tsalapatis
2025-12-15 20:19 ` Eduard Zingerman
2025-12-15 16:13 ` [PATCH v3 3/5] libbpf: turn relo_core->sym_off unsigned Emil Tsalapatis
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Emil Tsalapatis @ 2025-12-15 16:13 UTC (permalink / raw)
To: bpf
Cc: andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song, Emil Tsalapatis
The verifier currently limits direct offsets into a map to 512MiB
to avoid overflow during pointer arithmetic. However, this prevents
arena maps from using direct addressing instructions to access data
at the end of > 512MiB arena maps. This is necessary when moving
arena globals to the end of the arena instead of the front.
Refactor the verifier code to remove the offset calculation during
direct value access calculations. This is possible because the only
two map types that implement .map_direct_value_addr() are arrays and
arenas, and they both do their own internal checks to ensure the
offset is within bounds.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
kernel/bpf/verifier.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index bb7eca1025c3..dbb60d6bb73c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -21136,11 +21136,6 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
} else {
u32 off = insn[1].imm;
- if (off >= BPF_MAX_VAR_OFF) {
- verbose(env, "direct value offset of %u is not allowed\n", off);
- return -EINVAL;
- }
-
if (!map->ops->map_direct_value_addr) {
verbose(env, "no direct value access support for this map type\n");
return -EINVAL;
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 3/5] libbpf: turn relo_core->sym_off unsigned
2025-12-15 16:13 [PATCH v3 0/5] libbpf: move arena variables out of the zero page Emil Tsalapatis
2025-12-15 16:13 ` [PATCH v3 1/5] selftests/bpf: explicitly account for globals in verifier_arena_large Emil Tsalapatis
2025-12-15 16:13 ` [PATCH v3 2/5] bpf/verifier: do not limit maximum direct offset into arena map Emil Tsalapatis
@ 2025-12-15 16:13 ` Emil Tsalapatis
2025-12-15 16:37 ` bot+bpf-ci
2025-12-15 20:05 ` Eduard Zingerman
2025-12-15 16:13 ` [PATCH v3 4/5] libbpf: move arena globals to the end of the arena Emil Tsalapatis
2025-12-15 16:13 ` [PATCH v3 5/5] selftests/bpf: add tests for the arena offset of globals Emil Tsalapatis
4 siblings, 2 replies; 16+ messages in thread
From: Emil Tsalapatis @ 2025-12-15 16:13 UTC (permalink / raw)
To: bpf
Cc: andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song, Emil Tsalapatis
The symbols' relocation offsets in BPF are stored in an int field,
but cannot actually be negative. When in the next patch libbpf relocates
globals to the end of the arena, it is also possible to have valid
offsets > 2GiB that are used to calculate the final relo offsets.
Avoid accidentally interpreting large offsets as negative by turning
the sym_off field unsigned.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
tools/lib/bpf/libbpf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c7c79014d46c..5e66bbc2ab85 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -380,7 +380,7 @@ struct reloc_desc {
const struct bpf_core_relo *core_relo; /* used when type == RELO_CORE */
struct {
int map_idx;
- int sym_off;
+ unsigned int sym_off;
/*
* The following two fields can be unionized, as the
* ext_idx field is used for extern symbols, and the
@@ -763,7 +763,7 @@ struct bpf_object {
struct {
struct bpf_program *prog;
- int sym_off;
+ unsigned int sym_off;
int fd;
} *jumptable_maps;
size_t jumptable_map_cnt;
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 4/5] libbpf: move arena globals to the end of the arena
2025-12-15 16:13 [PATCH v3 0/5] libbpf: move arena variables out of the zero page Emil Tsalapatis
` (2 preceding siblings ...)
2025-12-15 16:13 ` [PATCH v3 3/5] libbpf: turn relo_core->sym_off unsigned Emil Tsalapatis
@ 2025-12-15 16:13 ` Emil Tsalapatis
2025-12-15 21:12 ` Eduard Zingerman
2025-12-15 16:13 ` [PATCH v3 5/5] selftests/bpf: add tests for the arena offset of globals Emil Tsalapatis
4 siblings, 1 reply; 16+ messages in thread
From: Emil Tsalapatis @ 2025-12-15 16:13 UTC (permalink / raw)
To: bpf
Cc: andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song, Emil Tsalapatis
Arena globals are currently placed at the beginning of the arena
by libbpf. This is convenient, but prevents users from reserving
guard pages in the beginning of the arena to identify NULL pointer
dereferences. Adjust the load logic to place the globals at the
end of the arena instead.
Also modify bpftool to set the arena pointer in the program's BPF
skeleton to point to the globals. Users now call bpf_map__initial_value()
to find the beginning of the arena mapping and use the arena pointer
in the skeleton to determine which part of the mapping holds the
arena globals and which part is free.
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
tools/lib/bpf/libbpf.c | 17 +++++++++++++----
.../selftests/bpf/progs/verifier_arena_large.c | 12 +++++++++---
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5e66bbc2ab85..135f5b36256e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -757,6 +757,7 @@ struct bpf_object {
int arena_map_idx;
void *arena_data;
size_t arena_data_sz;
+ size_t arena_data_off;
void *jumptables_data;
size_t jumptables_data_sz;
@@ -2991,10 +2992,11 @@ static int init_arena_map_data(struct bpf_object *obj, struct bpf_map *map,
void *data, size_t data_sz)
{
const long page_sz = sysconf(_SC_PAGE_SIZE);
+ const size_t data_alloc_sz = roundup(data_sz, page_sz);
size_t mmap_sz;
mmap_sz = bpf_map_mmap_sz(map);
- if (roundup(data_sz, page_sz) > mmap_sz) {
+ if (data_alloc_sz > mmap_sz) {
pr_warn("elf: sec '%s': declared ARENA map size (%zu) is too small to hold global __arena variables of size %zu\n",
sec_name, mmap_sz, data_sz);
return -E2BIG;
@@ -3006,6 +3008,9 @@ static int init_arena_map_data(struct bpf_object *obj, struct bpf_map *map,
memcpy(obj->arena_data, data, data_sz);
obj->arena_data_sz = data_sz;
+ /* place globals at the end of the arena */
+ obj->arena_data_off = mmap_sz - data_alloc_sz;
+
/* make bpf_map__init_value() work for ARENA maps */
map->mmaped = obj->arena_data;
@@ -4663,7 +4668,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog,
reloc_desc->type = RELO_DATA;
reloc_desc->insn_idx = insn_idx;
reloc_desc->map_idx = obj->arena_map_idx;
- reloc_desc->sym_off = sym->st_value;
+ reloc_desc->sym_off = sym->st_value + obj->arena_data_off;
map = &obj->maps[obj->arena_map_idx];
pr_debug("prog '%s': found arena map %d (%s, sec %d, off %zu) for insn %u\n",
@@ -5624,7 +5629,8 @@ bpf_object__create_maps(struct bpf_object *obj)
return err;
}
if (obj->arena_data) {
- memcpy(map->mmaped, obj->arena_data, obj->arena_data_sz);
+ memcpy(map->mmaped + obj->arena_data_off, obj->arena_data,
+ obj->arena_data_sz);
zfree(&obj->arena_data);
}
}
@@ -14429,7 +14435,10 @@ int bpf_object__load_skeleton(struct bpf_object_skeleton *s)
if (!map_skel->mmaped)
continue;
- *map_skel->mmaped = map->mmaped;
+ if (map->def.type == BPF_MAP_TYPE_ARENA)
+ *map_skel->mmaped = map->mmaped + map->obj->arena_data_off;
+ else
+ *map_skel->mmaped = map->mmaped;
}
return 0;
diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_large.c b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
index bd430a34c3ab..2b8cf2a4d880 100644
--- a/tools/testing/selftests/bpf/progs/verifier_arena_large.c
+++ b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
@@ -31,16 +31,22 @@ int big_alloc1(void *ctx)
if (!page1)
return 1;
- /* Account for global arena data. */
- if ((u64)page1 != base + PAGE_SIZE)
+ if ((u64)page1 != base)
return 15;
*page1 = 1;
- page2 = bpf_arena_alloc_pages(&arena, (void __arena *)(ARENA_SIZE - PAGE_SIZE),
+ page2 = bpf_arena_alloc_pages(&arena, (void __arena *)(ARENA_SIZE - 2 * PAGE_SIZE),
1, NUMA_NO_NODE, 0);
if (!page2)
return 2;
*page2 = 2;
+
+ /* Test for the guard region at the end of the arena. */
+ no_page = bpf_arena_alloc_pages(&arena, (void __arena *)ARENA_SIZE - PAGE_SIZE,
+ 1, NUMA_NO_NODE, 0);
+ if (no_page)
+ return 16;
+
no_page = bpf_arena_alloc_pages(&arena, (void __arena *)ARENA_SIZE,
1, NUMA_NO_NODE, 0);
if (no_page)
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 5/5] selftests/bpf: add tests for the arena offset of globals
2025-12-15 16:13 [PATCH v3 0/5] libbpf: move arena variables out of the zero page Emil Tsalapatis
` (3 preceding siblings ...)
2025-12-15 16:13 ` [PATCH v3 4/5] libbpf: move arena globals to the end of the arena Emil Tsalapatis
@ 2025-12-15 16:13 ` Emil Tsalapatis
2025-12-15 21:26 ` Eduard Zingerman
4 siblings, 1 reply; 16+ messages in thread
From: Emil Tsalapatis @ 2025-12-15 16:13 UTC (permalink / raw)
To: bpf
Cc: andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song, Emil Tsalapatis
Add tests for the new libbpf globals arena offset logic. The
tests cover the case of globals being as large as the arena
itself, and being smaller than the arena. In that case, the
data is placed at the end of the arena, and the beginning
of the arena is free.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
.../selftests/bpf/prog_tests/verifier.c | 4 +
.../bpf/progs/verifier_arena_globals1.c | 75 +++++++++++++++++++
.../bpf/progs/verifier_arena_globals2.c | 49 ++++++++++++
3 files changed, 128 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
create mode 100644 tools/testing/selftests/bpf/progs/verifier_arena_globals2.c
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 4b4b081b46cc..5829ffd70f8f 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -6,6 +6,8 @@
#include "verifier_and.skel.h"
#include "verifier_arena.skel.h"
#include "verifier_arena_large.skel.h"
+#include "verifier_arena_globals1.skel.h"
+#include "verifier_arena_globals2.skel.h"
#include "verifier_array_access.skel.h"
#include "verifier_async_cb_context.skel.h"
#include "verifier_basic_stack.skel.h"
@@ -147,6 +149,8 @@ static void run_tests_aux(const char *skel_name,
void test_verifier_and(void) { RUN(verifier_and); }
void test_verifier_arena(void) { RUN(verifier_arena); }
void test_verifier_arena_large(void) { RUN(verifier_arena_large); }
+void test_verifier_arena_globals1(void) { RUN(verifier_arena_globals1); }
+void test_verifier_arena_globals2(void) { RUN(verifier_arena_globals2); }
void test_verifier_basic_stack(void) { RUN(verifier_basic_stack); }
void test_verifier_bitfield_write(void) { RUN(verifier_bitfield_write); }
void test_verifier_bounds(void) { RUN(verifier_bounds); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_globals1.c b/tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
new file mode 100644
index 000000000000..d998a277e5e7
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#define BPF_NO_KFUNC_PROTOTYPES
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_experimental.h"
+#include "bpf_arena_common.h"
+#include "bpf_misc.h"
+
+#define ARENA_PAGES (1UL<< (32 - 12))
+#define GLOBAL_PAGES (16)
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARENA);
+ __uint(map_flags, BPF_F_MMAPABLE);
+ __uint(max_entries, ARENA_PAGES);
+#ifdef __TARGET_ARCH_arm64
+ __ulong(map_extra, (1ull << 32) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
+#else
+ __ulong(map_extra, (1ull << 44) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
+#endif
+} arena SEC(".maps");
+
+/*
+ * Global data, to be placed at the end of the arena.
+ */
+char __arena global_data[GLOBAL_PAGES][PAGE_SIZE];
+
+SEC("syscall")
+__success __retval(0)
+int check_reserve1(void *ctx)
+{
+#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
+ __u8 __arena *guard, *globals;
+ int ret;
+
+ guard = (void __arena *)arena_base(&arena);
+ globals = (void __arena *)(arena_base(&arena) + (ARENA_PAGES - GLOBAL_PAGES) * PAGE_SIZE);
+
+ /* Reserve the region we've offset the globals by. */
+ ret = bpf_arena_reserve_pages(&arena, guard, ARENA_PAGES - GLOBAL_PAGES);
+ if (ret)
+ return 1;
+
+ /* Make sure the globals are in the expected offset. */
+ ret = bpf_arena_reserve_pages(&arena, globals, 1);
+ if (!ret)
+ return 2;
+#endif
+ return 0;
+}
+
+/*
+ * Relocation check by reading directly into the global data w/o using symbols.
+ */
+SEC("syscall")
+__success __retval(0)
+int check_relocation(void *ctx)
+{
+#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
+ const u8 magic = 0xfa;
+ u8 __arena *ptr;
+
+ global_data[GLOBAL_PAGES - 1][PAGE_SIZE / 2] = magic;
+ ptr = (u8 __arena *)((u64)(ARENA_PAGES * PAGE_SIZE - PAGE_SIZE / 2));
+ if (*ptr != magic)
+ return 1;
+
+#endif
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_globals2.c b/tools/testing/selftests/bpf/progs/verifier_arena_globals2.c
new file mode 100644
index 000000000000..5a6f6bc3b00c
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_arena_globals2.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#define BPF_NO_KFUNC_PROTOTYPES
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+#include "bpf_arena_common.h"
+
+#define ARENA_PAGES (32)
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARENA);
+ __uint(map_flags, BPF_F_MMAPABLE);
+ __uint(max_entries, ARENA_PAGES);
+#ifdef __TARGET_ARCH_arm64
+ __ulong(map_extra, (1ull << 32) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
+#else
+ __ulong(map_extra, (1ull << 44) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
+#endif
+} arena SEC(".maps");
+
+/*
+ * Fill the entire arena with global data.
+ * The offset into the arena should be 0.
+ */
+char __arena global_data[PAGE_SIZE][ARENA_PAGES];
+
+SEC("syscall")
+__success __retval(0)
+int check_reserve2(void *ctx)
+{
+#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
+ void __arena *guard;
+ int ret;
+
+ guard = (void __arena *)arena_base(&arena);
+
+ /* Make sure the data at offset 0 case is properly handled. */
+ ret = bpf_arena_reserve_pages(&arena, guard, 1);
+ if (!ret)
+ return 1;
+#endif
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/5] libbpf: turn relo_core->sym_off unsigned
2025-12-15 16:13 ` [PATCH v3 3/5] libbpf: turn relo_core->sym_off unsigned Emil Tsalapatis
@ 2025-12-15 16:37 ` bot+bpf-ci
2025-12-15 17:08 ` Emil Tsalapatis
2025-12-15 20:05 ` Eduard Zingerman
1 sibling, 1 reply; 16+ messages in thread
From: bot+bpf-ci @ 2025-12-15 16:37 UTC (permalink / raw)
To: emil, bpf
Cc: andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song, emil, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index c7c79014d46c..5e66bbc2ab85 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -380,7 +380,7 @@ struct reloc_desc {
> const struct bpf_core_relo *core_relo; /* used when type == RELO_CORE */
> struct {
> int map_idx;
> - int sym_off;
> + unsigned int sym_off;
> /*
> * The following two fields can be unionized, as the
> * ext_idx field is used for extern symbols, and the
> @@ -763,7 +763,7 @@ struct bpf_object {
>
> struct {
> struct bpf_program *prog;
> - int sym_off;
> + unsigned int sym_off;
> int fd;
> } *jumptable_maps;
> size_t jumptable_map_cnt;
The field type change looks correct, but does create_jt_map() defeat
the purpose of this patch? That function declares a local variable
"int sym_off" that immediately converts the unsigned field back to
signed. When sym_off is >= 2GiB (the exact scenario this patch enables),
the local variable will interpret it as negative, causing incorrect
behavior in the modulo check, bounds check, and pointer arithmetic that
follow.
---
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/20239524817
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/5] libbpf: turn relo_core->sym_off unsigned
2025-12-15 16:37 ` bot+bpf-ci
@ 2025-12-15 17:08 ` Emil Tsalapatis
0 siblings, 0 replies; 16+ messages in thread
From: Emil Tsalapatis @ 2025-12-15 17:08 UTC (permalink / raw)
To: bot+bpf-ci, bpf
Cc: andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song, martin.lau, clm, ihor.solodrai
On Mon Dec 15, 2025 at 11:37 AM EST, bot+bpf-ci wrote:
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index c7c79014d46c..5e66bbc2ab85 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -380,7 +380,7 @@ struct reloc_desc {
>> const struct bpf_core_relo *core_relo; /* used when type == RELO_CORE */
>> struct {
>> int map_idx;
>> - int sym_off;
>> + unsigned int sym_off;
>> /*
>> * The following two fields can be unionized, as the
>> * ext_idx field is used for extern symbols, and the
>> @@ -763,7 +763,7 @@ struct bpf_object {
>>
>> struct {
>> struct bpf_program *prog;
>> - int sym_off;
>> + unsigned int sym_off;
>> int fd;
>> } *jumptable_maps;
>> size_t jumptable_map_cnt;
>
> The field type change looks correct, but does create_jt_map() defeat
> the purpose of this patch? That function declares a local variable
> "int sym_off" that immediately converts the unsigned field back to
> signed. When sym_off is >= 2GiB (the exact scenario this patch enables),
> the local variable will interpret it as negative, causing incorrect
> behavior in the modulo check, bounds check, and pointer arithmetic that
> follow.
>
>
The jumptable relocations handled by create_jt_map() don't have their
symbol offset artificially inflated they way we do for symbols in the
arena section, so keeping the sym_off signed inside create_jt_map()
should not cause a problem. We should still change it to handle
sym_off consistently across the codebase.
> ---
> 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/20239524817
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/5] libbpf: turn relo_core->sym_off unsigned
2025-12-15 16:13 ` [PATCH v3 3/5] libbpf: turn relo_core->sym_off unsigned Emil Tsalapatis
2025-12-15 16:37 ` bot+bpf-ci
@ 2025-12-15 20:05 ` Eduard Zingerman
1 sibling, 0 replies; 16+ messages in thread
From: Eduard Zingerman @ 2025-12-15 20:05 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, john.fastabend, memxor, yonghong.song
On Mon, 2025-12-15 at 11:13 -0500, Emil Tsalapatis wrote:
> The symbols' relocation offsets in BPF are stored in an int field,
> but cannot actually be negative. When in the next patch libbpf relocates
> globals to the end of the arena, it is also possible to have valid
> offsets > 2GiB that are used to calculate the final relo offsets.
> Avoid accidentally interpreting large offsets as negative by turning
> the sym_off field unsigned.
>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
(But agree with the nit from LLM, create_jt_map() should be updated
for consistency).
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/5] bpf/verifier: do not limit maximum direct offset into arena map
2025-12-15 16:13 ` [PATCH v3 2/5] bpf/verifier: do not limit maximum direct offset into arena map Emil Tsalapatis
@ 2025-12-15 20:19 ` Eduard Zingerman
2025-12-16 17:25 ` Emil Tsalapatis
0 siblings, 1 reply; 16+ messages in thread
From: Eduard Zingerman @ 2025-12-15 20:19 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, john.fastabend, memxor, yonghong.song
[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]
On Mon, 2025-12-15 at 11:13 -0500, Emil Tsalapatis wrote:
> The verifier currently limits direct offsets into a map to 512MiB
> to avoid overflow during pointer arithmetic. However, this prevents
> arena maps from using direct addressing instructions to access data
> at the end of > 512MiB arena maps. This is necessary when moving
> arena globals to the end of the arena instead of the front.
>
> Refactor the verifier code to remove the offset calculation during
> direct value access calculations. This is possible because the only
> two map types that implement .map_direct_value_addr() are arrays and
> arenas, and they both do their own internal checks to ensure the
> offset is within bounds.
Nit: instruction array map also implements it (bpf_insn_array.c).
>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
I double checked implementations for all 3 map types and confirm that
the above is correct. Also, I commented out the range checks in kernel
implementations (as in the attached patch), and no tests seem to fail.
Do we need to extend selftests?
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
[...]
[-- Attachment #2: disable-range-checks.patch --]
[-- Type: text/x-patch, Size: 1704 bytes --]
diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 1074ac4459f2..cf38388bb094 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -388,8 +388,10 @@ static int arena_map_direct_value_addr(const struct bpf_map *map, u64 *imm, u32
{
struct bpf_arena *arena = container_of(map, struct bpf_arena, map);
- if ((u64)off > arena->user_vm_end - arena->user_vm_start)
- return -ERANGE;
+ /*
+ * if ((u64)off > arena->user_vm_end - arena->user_vm_start)
+ * return -ERANGE;
+ */
*imm = (unsigned long)arena->user_vm_start;
return 0;
}
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 1eeb31c5b317..e13c58f2e3b8 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -193,8 +193,10 @@ static int array_map_direct_value_addr(const struct bpf_map *map, u64 *imm,
if (map->max_entries != 1)
return -ENOTSUPP;
- if (off >= map->value_size)
- return -EINVAL;
+ /*
+ * if (off >= map->value_size)
+ * return -EINVAL;
+ */
*imm = (unsigned long)array->value;
return 0;
diff --git a/kernel/bpf/bpf_insn_array.c b/kernel/bpf/bpf_insn_array.c
index c96630cb75bf..aa4b71365541 100644
--- a/kernel/bpf/bpf_insn_array.c
+++ b/kernel/bpf/bpf_insn_array.c
@@ -121,9 +121,11 @@ static int insn_array_map_direct_value_addr(const struct bpf_map *map, u64 *imm,
{
struct bpf_insn_array *insn_array = cast_insn_array(map);
- if ((off % sizeof(long)) != 0 ||
- (off / sizeof(long)) >= map->max_entries)
- return -EINVAL;
+ /*
+ * if ((off % sizeof(long)) != 0 ||
+ * (off / sizeof(long)) >= map->max_entries)
+ * return -EINVAL;
+ */
/* from BPF's point of view, this map is a jump table */
*imm = (unsigned long)insn_array->ips + off;
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/5] libbpf: move arena globals to the end of the arena
2025-12-15 16:13 ` [PATCH v3 4/5] libbpf: move arena globals to the end of the arena Emil Tsalapatis
@ 2025-12-15 21:12 ` Eduard Zingerman
0 siblings, 0 replies; 16+ messages in thread
From: Eduard Zingerman @ 2025-12-15 21:12 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, john.fastabend, memxor, yonghong.song
On Mon, 2025-12-15 at 11:13 -0500, Emil Tsalapatis wrote:
> Arena globals are currently placed at the beginning of the arena
> by libbpf. This is convenient, but prevents users from reserving
> guard pages in the beginning of the arena to identify NULL pointer
> dereferences. Adjust the load logic to place the globals at the
> end of the arena instead.
>
> Also modify bpftool to set the arena pointer in the program's BPF
> skeleton to point to the globals. Users now call bpf_map__initial_value()
> to find the beginning of the arena mapping and use the arena pointer
> in the skeleton to determine which part of the mapping holds the
> arena globals and which part is free.
>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
[...]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 5/5] selftests/bpf: add tests for the arena offset of globals
2025-12-15 16:13 ` [PATCH v3 5/5] selftests/bpf: add tests for the arena offset of globals Emil Tsalapatis
@ 2025-12-15 21:26 ` Eduard Zingerman
2025-12-16 2:28 ` Emil Tsalapatis
0 siblings, 1 reply; 16+ messages in thread
From: Eduard Zingerman @ 2025-12-15 21:26 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, john.fastabend, memxor, yonghong.song
On Mon, 2025-12-15 at 11:13 -0500, Emil Tsalapatis wrote:
> Add tests for the new libbpf globals arena offset logic. The
> tests cover the case of globals being as large as the arena
> itself, and being smaller than the arena. In that case, the
> data is placed at the end of the arena, and the beginning
> of the arena is free.
>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
> .../selftests/bpf/prog_tests/verifier.c | 4 +
> .../bpf/progs/verifier_arena_globals1.c | 75 +++++++++++++++++++
> .../bpf/progs/verifier_arena_globals2.c | 49 ++++++++++++
> 3 files changed, 128 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
> create mode 100644 tools/testing/selftests/bpf/progs/verifier_arena_globals2.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index 4b4b081b46cc..5829ffd70f8f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -6,6 +6,8 @@
> #include "verifier_and.skel.h"
> #include "verifier_arena.skel.h"
> #include "verifier_arena_large.skel.h"
> +#include "verifier_arena_globals1.skel.h"
> +#include "verifier_arena_globals2.skel.h"
> #include "verifier_array_access.skel.h"
> #include "verifier_async_cb_context.skel.h"
> #include "verifier_basic_stack.skel.h"
> @@ -147,6 +149,8 @@ static void run_tests_aux(const char *skel_name,
> void test_verifier_and(void) { RUN(verifier_and); }
> void test_verifier_arena(void) { RUN(verifier_arena); }
> void test_verifier_arena_large(void) { RUN(verifier_arena_large); }
> +void test_verifier_arena_globals1(void) { RUN(verifier_arena_globals1); }
> +void test_verifier_arena_globals2(void) { RUN(verifier_arena_globals2); }
> void test_verifier_basic_stack(void) { RUN(verifier_basic_stack); }
> void test_verifier_bitfield_write(void) { RUN(verifier_bitfield_write); }
> void test_verifier_bounds(void) { RUN(verifier_bounds); }
> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_globals1.c b/tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
> new file mode 100644
> index 000000000000..d998a277e5e7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
> @@ -0,0 +1,75 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
> +
> +#define BPF_NO_KFUNC_PROTOTYPES
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include "bpf_experimental.h"
> +#include "bpf_arena_common.h"
> +#include "bpf_misc.h"
> +
> +#define ARENA_PAGES (1UL<< (32 - 12))
> +#define GLOBAL_PAGES (16)
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_ARENA);
> + __uint(map_flags, BPF_F_MMAPABLE);
> + __uint(max_entries, ARENA_PAGES);
> +#ifdef __TARGET_ARCH_arm64
> + __ulong(map_extra, (1ull << 32) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
> +#else
> + __ulong(map_extra, (1ull << 44) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
> +#endif
> +} arena SEC(".maps");
> +
> +/*
> + * Global data, to be placed at the end of the arena.
> + */
> +char __arena global_data[GLOBAL_PAGES][PAGE_SIZE];
> +
> +SEC("syscall")
> +__success __retval(0)
> +int check_reserve1(void *ctx)
> +{
> +#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
> + __u8 __arena *guard, *globals;
> + int ret;
> +
> + guard = (void __arena *)arena_base(&arena);
> + globals = (void __arena *)(arena_base(&arena) + (ARENA_PAGES - GLOBAL_PAGES) * PAGE_SIZE);
> +
> + /* Reserve the region we've offset the globals by. */
> + ret = bpf_arena_reserve_pages(&arena, guard, ARENA_PAGES - GLOBAL_PAGES);
> + if (ret)
> + return 1;
> +
> + /* Make sure the globals are in the expected offset. */
> + ret = bpf_arena_reserve_pages(&arena, globals, 1);
In addition to checking that reserving pages succeeds,
do we need to test pages content here?
> + if (!ret)
> + return 2;
> +#endif
> + return 0;
> +}
> +
> +/*
> + * Relocation check by reading directly into the global data w/o using symbols.
> + */
> +SEC("syscall")
> +__success __retval(0)
> +int check_relocation(void *ctx)
> +{
> +#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
> + const u8 magic = 0xfa;
> + u8 __arena *ptr;
> +
> + global_data[GLOBAL_PAGES - 1][PAGE_SIZE / 2] = magic;
> + ptr = (u8 __arena *)((u64)(ARENA_PAGES * PAGE_SIZE - PAGE_SIZE / 2));
> + if (*ptr != magic)
> + return 1;
> +
> +#endif
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_globals2.c b/tools/testing/selftests/bpf/progs/verifier_arena_globals2.c
> new file mode 100644
> index 000000000000..5a6f6bc3b00c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_arena_globals2.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
> +
> +#define BPF_NO_KFUNC_PROTOTYPES
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include "bpf_misc.h"
> +#include "bpf_experimental.h"
> +#include "bpf_arena_common.h"
> +
> +#define ARENA_PAGES (32)
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_ARENA);
> + __uint(map_flags, BPF_F_MMAPABLE);
> + __uint(max_entries, ARENA_PAGES);
> +#ifdef __TARGET_ARCH_arm64
> + __ulong(map_extra, (1ull << 32) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
> +#else
> + __ulong(map_extra, (1ull << 44) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
> +#endif
> +} arena SEC(".maps");
> +
> +/*
> + * Fill the entire arena with global data.
> + * The offset into the arena should be 0.
> + */
> +char __arena global_data[PAGE_SIZE][ARENA_PAGES];
^^^^^^^^^
Nit: this is reversed compared to the declaration in the previous test:
> +/*
> + * Global data, to be placed at the end of the arena.
> + */
> +char __arena global_data[GLOBAL_PAGES][PAGE_SIZE];
^^^^^^^^^
> +
> +SEC("syscall")
> +__success __retval(0)
> +int check_reserve2(void *ctx)
> +{
> +#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
> + void __arena *guard;
> + int ret;
> +
> + guard = (void __arena *)arena_base(&arena);
> +
> + /* Make sure the data at offset 0 case is properly handled. */
> + ret = bpf_arena_reserve_pages(&arena, guard, 1);
> + if (!ret)
> + return 1;
> +#endif
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 5/5] selftests/bpf: add tests for the arena offset of globals
2025-12-15 21:26 ` Eduard Zingerman
@ 2025-12-16 2:28 ` Emil Tsalapatis
0 siblings, 0 replies; 16+ messages in thread
From: Emil Tsalapatis @ 2025-12-16 2:28 UTC (permalink / raw)
To: Eduard Zingerman, bpf
Cc: andrii, ast, daniel, john.fastabend, memxor, yonghong.song
On Mon Dec 15, 2025 at 4:26 PM EST, Eduard Zingerman wrote:
> On Mon, 2025-12-15 at 11:13 -0500, Emil Tsalapatis wrote:
>> Add tests for the new libbpf globals arena offset logic. The
>> tests cover the case of globals being as large as the arena
>> itself, and being smaller than the arena. In that case, the
>> data is placed at the end of the arena, and the beginning
>> of the arena is free.
>>
>> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> ---
>> .../selftests/bpf/prog_tests/verifier.c | 4 +
>> .../bpf/progs/verifier_arena_globals1.c | 75 +++++++++++++++++++
>> .../bpf/progs/verifier_arena_globals2.c | 49 ++++++++++++
>> 3 files changed, 128 insertions(+)
>> create mode 100644 tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
>> create mode 100644 tools/testing/selftests/bpf/progs/verifier_arena_globals2.c
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
>> index 4b4b081b46cc..5829ffd70f8f 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
>> @@ -6,6 +6,8 @@
>> #include "verifier_and.skel.h"
>> #include "verifier_arena.skel.h"
>> #include "verifier_arena_large.skel.h"
>> +#include "verifier_arena_globals1.skel.h"
>> +#include "verifier_arena_globals2.skel.h"
>> #include "verifier_array_access.skel.h"
>> #include "verifier_async_cb_context.skel.h"
>> #include "verifier_basic_stack.skel.h"
>> @@ -147,6 +149,8 @@ static void run_tests_aux(const char *skel_name,
>> void test_verifier_and(void) { RUN(verifier_and); }
>> void test_verifier_arena(void) { RUN(verifier_arena); }
>> void test_verifier_arena_large(void) { RUN(verifier_arena_large); }
>> +void test_verifier_arena_globals1(void) { RUN(verifier_arena_globals1); }
>> +void test_verifier_arena_globals2(void) { RUN(verifier_arena_globals2); }
>> void test_verifier_basic_stack(void) { RUN(verifier_basic_stack); }
>> void test_verifier_bitfield_write(void) { RUN(verifier_bitfield_write); }
>> void test_verifier_bounds(void) { RUN(verifier_bounds); }
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_globals1.c b/tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
>> new file mode 100644
>> index 000000000000..d998a277e5e7
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
>> @@ -0,0 +1,75 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
>> +
>> +#define BPF_NO_KFUNC_PROTOTYPES
>> +#include <vmlinux.h>
>> +#include <bpf/bpf_helpers.h>
>> +#include <bpf/bpf_tracing.h>
>> +#include "bpf_experimental.h"
>> +#include "bpf_arena_common.h"
>> +#include "bpf_misc.h"
>> +
>> +#define ARENA_PAGES (1UL<< (32 - 12))
>> +#define GLOBAL_PAGES (16)
>> +
>> +struct {
>> + __uint(type, BPF_MAP_TYPE_ARENA);
>> + __uint(map_flags, BPF_F_MMAPABLE);
>> + __uint(max_entries, ARENA_PAGES);
>> +#ifdef __TARGET_ARCH_arm64
>> + __ulong(map_extra, (1ull << 32) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
>> +#else
>> + __ulong(map_extra, (1ull << 44) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
>> +#endif
>> +} arena SEC(".maps");
>> +
>> +/*
>> + * Global data, to be placed at the end of the arena.
>> + */
>> +char __arena global_data[GLOBAL_PAGES][PAGE_SIZE];
>> +
>> +SEC("syscall")
>> +__success __retval(0)
>> +int check_reserve1(void *ctx)
>> +{
>> +#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
>> + __u8 __arena *guard, *globals;
>> + int ret;
>> +
>> + guard = (void __arena *)arena_base(&arena);
>> + globals = (void __arena *)(arena_base(&arena) + (ARENA_PAGES - GLOBAL_PAGES) * PAGE_SIZE);
>> +
>> + /* Reserve the region we've offset the globals by. */
>> + ret = bpf_arena_reserve_pages(&arena, guard, ARENA_PAGES - GLOBAL_PAGES);
>> + if (ret)
>> + return 1;
>> +
>> + /* Make sure the globals are in the expected offset. */
>> + ret = bpf_arena_reserve_pages(&arena, globals, 1);
>
> In addition to checking that reserving pages succeeds,
> do we need to test pages content here?
>
For sure, I will make sure the arena globals are writable.
>> + if (!ret)
>> + return 2;
>> +#endif
>> + return 0;
>> +}
>> +
>> +/*
>> + * Relocation check by reading directly into the global data w/o using symbols.
>> + */
>> +SEC("syscall")
>> +__success __retval(0)
>> +int check_relocation(void *ctx)
>> +{
>> +#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
>> + const u8 magic = 0xfa;
>> + u8 __arena *ptr;
>> +
>> + global_data[GLOBAL_PAGES - 1][PAGE_SIZE / 2] = magic;
>> + ptr = (u8 __arena *)((u64)(ARENA_PAGES * PAGE_SIZE - PAGE_SIZE / 2));
>> + if (*ptr != magic)
>> + return 1;
>> +
>> +#endif
>> + return 0;
>> +}
>> +
>> +char _license[] SEC("license") = "GPL";
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_globals2.c b/tools/testing/selftests/bpf/progs/verifier_arena_globals2.c
>> new file mode 100644
>> index 000000000000..5a6f6bc3b00c
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/verifier_arena_globals2.c
>> @@ -0,0 +1,49 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
>> +
>> +#define BPF_NO_KFUNC_PROTOTYPES
>> +#include <vmlinux.h>
>> +#include <bpf/bpf_helpers.h>
>> +#include <bpf/bpf_tracing.h>
>> +#include "bpf_misc.h"
>> +#include "bpf_experimental.h"
>> +#include "bpf_arena_common.h"
>> +
>> +#define ARENA_PAGES (32)
>> +
>> +struct {
>> + __uint(type, BPF_MAP_TYPE_ARENA);
>> + __uint(map_flags, BPF_F_MMAPABLE);
>> + __uint(max_entries, ARENA_PAGES);
>> +#ifdef __TARGET_ARCH_arm64
>> + __ulong(map_extra, (1ull << 32) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
>> +#else
>> + __ulong(map_extra, (1ull << 44) | (~0u - __PAGE_SIZE * ARENA_PAGES + 1));
>> +#endif
>> +} arena SEC(".maps");
>> +
>> +/*
>> + * Fill the entire arena with global data.
>> + * The offset into the arena should be 0.
>> + */
>> +char __arena global_data[PAGE_SIZE][ARENA_PAGES];
> ^^^^^^^^^
>
> Nit: this is reversed compared to the declaration in the previous test:
>
> > +/*
> > + * Global data, to be placed at the end of the arena.
> > + */
> > +char __arena global_data[GLOBAL_PAGES][PAGE_SIZE];
> ^^^^^^^^^
>
Ack, will fix.
>> +
>> +SEC("syscall")
>> +__success __retval(0)
>> +int check_reserve2(void *ctx)
>> +{
>> +#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
>> + void __arena *guard;
>> + int ret;
>> +
>> + guard = (void __arena *)arena_base(&arena);
>> +
>> + /* Make sure the data at offset 0 case is properly handled. */
>> + ret = bpf_arena_reserve_pages(&arena, guard, 1);
>> + if (!ret)
>> + return 1;
>> +#endif
>> + return 0;
>> +}
>> +
>> +char _license[] SEC("license") = "GPL";
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/5] bpf/verifier: do not limit maximum direct offset into arena map
2025-12-15 20:19 ` Eduard Zingerman
@ 2025-12-16 17:25 ` Emil Tsalapatis
2025-12-16 20:13 ` Eduard Zingerman
0 siblings, 1 reply; 16+ messages in thread
From: Emil Tsalapatis @ 2025-12-16 17:25 UTC (permalink / raw)
To: Eduard Zingerman, bpf
Cc: andrii, ast, daniel, john.fastabend, memxor, yonghong.song
On Mon Dec 15, 2025 at 3:19 PM EST, Eduard Zingerman wrote:
> On Mon, 2025-12-15 at 11:13 -0500, Emil Tsalapatis wrote:
>> The verifier currently limits direct offsets into a map to 512MiB
>> to avoid overflow during pointer arithmetic. However, this prevents
>> arena maps from using direct addressing instructions to access data
>> at the end of > 512MiB arena maps. This is necessary when moving
>> arena globals to the end of the arena instead of the front.
>>
>> Refactor the verifier code to remove the offset calculation during
>> direct value access calculations. This is possible because the only
>> two map types that implement .map_direct_value_addr() are arrays and
>> arenas, and they both do their own internal checks to ensure the
>> offset is within bounds.
>
> Nit: instruction array map also implements it (bpf_insn_array.c).
>
>>
>> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> ---
>
> I double checked implementations for all 3 map types and confirm that
> the above is correct. Also, I commented out the range checks in kernel
> implementations (as in the attached patch), and no tests seem to fail.
> Do we need to extend selftests?
I forgot to address a couple selftest errors from this patch in this version,
but after fixing them for v4 and applying the attached patch I am getting a
couple failures - direct map access tests #332, #334, #336, #337, #338, #345.
#332 (write test 7) is an unexpected load success, while the rest are about a
mismatch in the error message. Maybe the test wasn't being marked as an
unexpected success because I hadn't fixed it up?
>
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
I will omit the Acked-by tag in v4 because I changed the selftests - sorry for the
after-the-fact changes.
> [...]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/5] bpf/verifier: do not limit maximum direct offset into arena map
2025-12-16 17:25 ` Emil Tsalapatis
@ 2025-12-16 20:13 ` Eduard Zingerman
2025-12-16 21:48 ` Emil Tsalapatis
0 siblings, 1 reply; 16+ messages in thread
From: Eduard Zingerman @ 2025-12-16 20:13 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, john.fastabend, memxor, yonghong.song
On Tue, 2025-12-16 at 12:25 -0500, Emil Tsalapatis wrote:
> On Mon Dec 15, 2025 at 3:19 PM EST, Eduard Zingerman wrote:
> > On Mon, 2025-12-15 at 11:13 -0500, Emil Tsalapatis wrote:
> > > The verifier currently limits direct offsets into a map to 512MiB
> > > to avoid overflow during pointer arithmetic. However, this prevents
> > > arena maps from using direct addressing instructions to access data
> > > at the end of > 512MiB arena maps. This is necessary when moving
> > > arena globals to the end of the arena instead of the front.
> > >
> > > Refactor the verifier code to remove the offset calculation during
> > > direct value access calculations. This is possible because the only
> > > two map types that implement .map_direct_value_addr() are arrays and
> > > arenas, and they both do their own internal checks to ensure the
> > > offset is within bounds.
> >
> > Nit: instruction array map also implements it (bpf_insn_array.c).
> >
> > >
> > > Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> > > ---
> >
> > I double checked implementations for all 3 map types and confirm that
> > the above is correct. Also, I commented out the range checks in kernel
> > implementations (as in the attached patch), and no tests seem to fail.
> > Do we need to extend selftests?
>
> I forgot to address a couple selftest errors from this patch in this version,
> but after fixing them for v4 and applying the attached patch I am getting a
> couple failures - direct map access tests #332, #334, #336, #337, #338, #345.
Uh-oh, sorry, I forgot about test_verifier binary.
> #332 (write test 7) is an unexpected load success, while the rest are about a
> mismatch in the error message. Maybe the test wasn't being marked as an
> unexpected success because I hadn't fixed it up?
For me it shows:
#332/p direct map access, write test 7 FAIL
Unexpected verifier log!
EXP: direct value offset of 4294967295 is not allowed
RES:
FAIL
Unexpected error message!
EXP: direct value offset of 4294967295 is not allowed
RES: invalid access to map value pointer, value_size=48 off=4294967295
So, seem to be an expected behavior given your changes?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/5] bpf/verifier: do not limit maximum direct offset into arena map
2025-12-16 20:13 ` Eduard Zingerman
@ 2025-12-16 21:48 ` Emil Tsalapatis
0 siblings, 0 replies; 16+ messages in thread
From: Emil Tsalapatis @ 2025-12-16 21:48 UTC (permalink / raw)
To: Eduard Zingerman, bpf
Cc: andrii, ast, daniel, john.fastabend, memxor, yonghong.song
On Tue Dec 16, 2025 at 3:13 PM EST, Eduard Zingerman wrote:
> On Tue, 2025-12-16 at 12:25 -0500, Emil Tsalapatis wrote:
>> On Mon Dec 15, 2025 at 3:19 PM EST, Eduard Zingerman wrote:
>> > On Mon, 2025-12-15 at 11:13 -0500, Emil Tsalapatis wrote:
>> > > The verifier currently limits direct offsets into a map to 512MiB
>> > > to avoid overflow during pointer arithmetic. However, this prevents
>> > > arena maps from using direct addressing instructions to access data
>> > > at the end of > 512MiB arena maps. This is necessary when moving
>> > > arena globals to the end of the arena instead of the front.
>> > >
>> > > Refactor the verifier code to remove the offset calculation during
>> > > direct value access calculations. This is possible because the only
>> > > two map types that implement .map_direct_value_addr() are arrays and
>> > > arenas, and they both do their own internal checks to ensure the
>> > > offset is within bounds.
>> >
>> > Nit: instruction array map also implements it (bpf_insn_array.c).
>> >
>> > >
>> > > Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> > > ---
>> >
>> > I double checked implementations for all 3 map types and confirm that
>> > the above is correct. Also, I commented out the range checks in kernel
>> > implementations (as in the attached patch), and no tests seem to fail.
>> > Do we need to extend selftests?
>>
>> I forgot to address a couple selftest errors from this patch in this version,
>> but after fixing them for v4 and applying the attached patch I am getting a
>> couple failures - direct map access tests #332, #334, #336, #337, #338, #345.
>
> Uh-oh, sorry, I forgot about test_verifier binary.
>
>> #332 (write test 7) is an unexpected load success, while the rest are about a
>> mismatch in the error message. Maybe the test wasn't being marked as an
>> unexpected success because I hadn't fixed it up?
>
> For me it shows:
>
> #332/p direct map access, write test 7 FAIL
> Unexpected verifier log!
> EXP: direct value offset of 4294967295 is not allowed
> RES:
> FAIL
> Unexpected error message!
> EXP: direct value offset of 4294967295 is not allowed
> RES: invalid access to map value pointer, value_size=48 off=4294967295
>
> So, seem to be an expected behavior given your changes?
Yes, for v3 this was the expected behavior: The tests used to trigger
the old error msg (in EXP), but even after removing it they fail
because the map isn't actually large enough to support the value
offset offset of the tests ((-1) and (1UL << 29) for tests 7 and
12). The RES output is from the .map_direct_value_addr() method
doing bounds checking and rejecting the out-of-bounds offset.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-12-16 21:48 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-15 16:13 [PATCH v3 0/5] libbpf: move arena variables out of the zero page Emil Tsalapatis
2025-12-15 16:13 ` [PATCH v3 1/5] selftests/bpf: explicitly account for globals in verifier_arena_large Emil Tsalapatis
2025-12-15 16:13 ` [PATCH v3 2/5] bpf/verifier: do not limit maximum direct offset into arena map Emil Tsalapatis
2025-12-15 20:19 ` Eduard Zingerman
2025-12-16 17:25 ` Emil Tsalapatis
2025-12-16 20:13 ` Eduard Zingerman
2025-12-16 21:48 ` Emil Tsalapatis
2025-12-15 16:13 ` [PATCH v3 3/5] libbpf: turn relo_core->sym_off unsigned Emil Tsalapatis
2025-12-15 16:37 ` bot+bpf-ci
2025-12-15 17:08 ` Emil Tsalapatis
2025-12-15 20:05 ` Eduard Zingerman
2025-12-15 16:13 ` [PATCH v3 4/5] libbpf: move arena globals to the end of the arena Emil Tsalapatis
2025-12-15 21:12 ` Eduard Zingerman
2025-12-15 16:13 ` [PATCH v3 5/5] selftests/bpf: add tests for the arena offset of globals Emil Tsalapatis
2025-12-15 21:26 ` Eduard Zingerman
2025-12-16 2:28 ` Emil Tsalapatis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox