* [PATCH v2 1/4] selftests/bpf: explicitly account for globals in verifier_arena_large
2025-12-03 16:26 [PATCH v2 0/4] libbpf: move arena variables out of the zero Emil Tsalapatis
@ 2025-12-03 16:26 ` Emil Tsalapatis
2025-12-03 16:26 ` [PATCH v2 2/4] bpf/verifier: do not limit maximum direct offset into arena map Emil Tsalapatis
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2025-12-03 16:26 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] 11+ messages in thread* [PATCH v2 2/4] bpf/verifier: do not limit maximum direct offset into arena map
2025-12-03 16:26 [PATCH v2 0/4] libbpf: move arena variables out of the zero Emil Tsalapatis
2025-12-03 16:26 ` [PATCH v2 1/4] selftests/bpf: explicitly account for globals in verifier_arena_large Emil Tsalapatis
@ 2025-12-03 16:26 ` Emil Tsalapatis
2025-12-06 0:11 ` Andrii Nakryiko
2025-12-03 16:26 ` [PATCH v2 3/4] libbpf: move arena globals to the end of the arena Emil Tsalapatis
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Emil Tsalapatis @ 2025-12-03 16:26 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.
Relax the limitation for direct offsets into arena maps to 4GiB,
the maximum arena size.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
kernel/bpf/verifier.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 098dd7f21c89..a64cc5caf4aa 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -21084,13 +21084,13 @@ 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);
+ if (!map->ops->map_direct_value_addr) {
+ verbose(env, "no direct value access support for this map type\n");
return -EINVAL;
}
- if (!map->ops->map_direct_value_addr) {
- verbose(env, "no direct value access support for this map type\n");
+ if (off >= BPF_MAX_VAR_OFF && map->map_type != BPF_MAP_TYPE_ARENA) {
+ verbose(env, "direct value offset of %u is not allowed\n", off);
return -EINVAL;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 2/4] bpf/verifier: do not limit maximum direct offset into arena map
2025-12-03 16:26 ` [PATCH v2 2/4] bpf/verifier: do not limit maximum direct offset into arena map Emil Tsalapatis
@ 2025-12-06 0:11 ` Andrii Nakryiko
0 siblings, 0 replies; 11+ messages in thread
From: Andrii Nakryiko @ 2025-12-06 0:11 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song
On Wed, Dec 3, 2025 at 8:26 AM Emil Tsalapatis <emil@etsalapatis.com> 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.
>
> Relax the limitation for direct offsets into arena maps to 4GiB,
> the maximum arena size.
>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
> kernel/bpf/verifier.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 098dd7f21c89..a64cc5caf4aa 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -21084,13 +21084,13 @@ 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);
> + if (!map->ops->map_direct_value_addr) {
> + verbose(env, "no direct value access support for this map type\n");
> return -EINVAL;
> }
>
> - if (!map->ops->map_direct_value_addr) {
> - verbose(env, "no direct value access support for this map type\n");
> + if (off >= BPF_MAX_VAR_OFF && map->map_type != BPF_MAP_TYPE_ARENA) {
> + verbose(env, "direct value offset of %u is not allowed\n", off);
> return -EINVAL;
> }
both arena and array maps validate off for correct range, not sure we
even need this check here?
>
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] libbpf: move arena globals to the end of the arena
2025-12-03 16:26 [PATCH v2 0/4] libbpf: move arena variables out of the zero Emil Tsalapatis
2025-12-03 16:26 ` [PATCH v2 1/4] selftests/bpf: explicitly account for globals in verifier_arena_large Emil Tsalapatis
2025-12-03 16:26 ` [PATCH v2 2/4] bpf/verifier: do not limit maximum direct offset into arena map Emil Tsalapatis
@ 2025-12-03 16:26 ` Emil Tsalapatis
2025-12-03 16:52 ` bot+bpf-ci
2025-12-06 0:11 ` Andrii Nakryiko
2025-12-03 16:26 ` [PATCH v2 4/4] selftests/bpf: add tests for the arena offset of globals Emil Tsalapatis
2025-12-06 0:11 ` [PATCH v2 0/4] libbpf: move arena variables out of the zero Andrii Nakryiko
4 siblings, 2 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2025-12-03 16:26 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 | 19 +++++++++++++++----
.../bpf/progs/verifier_arena_large.c | 12 +++++++++---
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 706e7481bdf6..9642d697b690 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;
+ __u32 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);
}
}
@@ -14387,6 +14393,7 @@ void bpf_object__destroy_subskeleton(struct bpf_object_subskeleton *s)
int bpf_object__load_skeleton(struct bpf_object_skeleton *s)
{
+ void *mmaped;
int i, err;
err = bpf_object__load(*s->obj);
@@ -14402,7 +14409,11 @@ int bpf_object__load_skeleton(struct bpf_object_skeleton *s)
if (!map_skel->mmaped)
continue;
- *map_skel->mmaped = map->mmaped;
+ mmaped = map->mmaped;
+ if (map->def.type == BPF_MAP_TYPE_ARENA)
+ mmaped += map->obj->arena_data_off;
+
+ *map_skel->mmaped = 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] 11+ messages in thread* Re: [PATCH v2 3/4] libbpf: move arena globals to the end of the arena
2025-12-03 16:26 ` [PATCH v2 3/4] libbpf: move arena globals to the end of the arena Emil Tsalapatis
@ 2025-12-03 16:52 ` bot+bpf-ci
2025-12-06 0:11 ` Andrii Nakryiko
2025-12-06 0:11 ` Andrii Nakryiko
1 sibling, 1 reply; 11+ messages in thread
From: bot+bpf-ci @ 2025-12-03 16:52 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: 2658 bytes --]
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 706e7481b..9642d697b 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
[ ... ]
> @@ -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;
> +
Here obj->arena_data_off is a __u32, but the calculation is done in
size_t (64-bit). For a 4GB arena (which the test verifier_arena_large.c
uses), mmap_sz is 4GB and the result can approach 4GB - PAGE_SIZE. This
fits in u32 correctly.
> @@ -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;
^^^^^^^^^^^^^^^^^^^^^^
Can this overflow when storing to reloc_desc->sym_off? The sym_off field
is 'int' (signed 32-bit), but arena_data_off can be larger than INT_MAX
for arenas approaching 4GB in size.
For example, with a 4GB arena and one page of globals:
- mmap_sz = 4GB = 0x100000000
- data_alloc_sz = 4096
- arena_data_off = 0xFFFFF000 = 4294963200
This value (4294963200) exceeds INT_MAX (2147483647), so when stored in
the signed int sym_off field, it becomes negative.
Later in bpf_object__relocate_data(), this is used as:
insn[1].imm = insn[0].imm + relo->sym_off;
With sym_off being negative due to overflow, this produces an incorrect
offset for accessing arena globals.
Should sym_off be changed to an unsigned type to handle large arena
offsets correctly?
[ ... ]
---
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/19901383089
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 3/4] libbpf: move arena globals to the end of the arena
2025-12-03 16:52 ` bot+bpf-ci
@ 2025-12-06 0:11 ` Andrii Nakryiko
2025-12-08 22:30 ` Emil Tsalapatis
0 siblings, 1 reply; 11+ messages in thread
From: Andrii Nakryiko @ 2025-12-06 0:11 UTC (permalink / raw)
To: bot+bpf-ci
Cc: emil, bpf, andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song, martin.lau, clm, ihor.solodrai
On Wed, Dec 3, 2025 at 8:52 AM <bot+bpf-ci@kernel.org> wrote:
>
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 706e7481b..9642d697b 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
>
> [ ... ]
>
> > @@ -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;
> > +
>
> Here obj->arena_data_off is a __u32, but the calculation is done in
> size_t (64-bit). For a 4GB arena (which the test verifier_arena_large.c
> uses), mmap_sz is 4GB and the result can approach 4GB - PAGE_SIZE. This
> fits in u32 correctly.
>
> > @@ -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;
> ^^^^^^^^^^^^^^^^^^^^^^
>
> Can this overflow when storing to reloc_desc->sym_off? The sym_off field
> is 'int' (signed 32-bit), but arena_data_off can be larger than INT_MAX
> for arenas approaching 4GB in size.
>
> For example, with a 4GB arena and one page of globals:
> - mmap_sz = 4GB = 0x100000000
> - data_alloc_sz = 4096
> - arena_data_off = 0xFFFFF000 = 4294963200
>
> This value (4294963200) exceeds INT_MAX (2147483647), so when stored in
> the signed int sym_off field, it becomes negative.
>
> Later in bpf_object__relocate_data(), this is used as:
> insn[1].imm = insn[0].imm + relo->sym_off;
>
> With sym_off being negative due to overflow, this produces an incorrect
> offset for accessing arena globals.
>
> Should sym_off be changed to an unsigned type to handle large arena
> offsets correctly?
AI has good a point, valid offset with 4GB arena will overflow int...
But then ldimm64 instructions that allow direct access to arena have
insn[1].imm as s32, so they cannot even address more than 2GB? Can you
please double check this, maybe we actually treat insn[1].imm as u32
on the verifier side for this use case?
>
> [ ... ]
>
>
> ---
> 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/19901383089
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 3/4] libbpf: move arena globals to the end of the arena
2025-12-06 0:11 ` Andrii Nakryiko
@ 2025-12-08 22:30 ` Emil Tsalapatis
0 siblings, 0 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2025-12-08 22:30 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bot+bpf-ci, bpf, andrii, eddyz87, ast, daniel, john.fastabend,
memxor, yonghong.song, martin.lau, clm, ihor.solodrai
On Fri, Dec 5, 2025 at 7:11 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Dec 3, 2025 at 8:52 AM <bot+bpf-ci@kernel.org> wrote:
> >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 706e7481b..9642d697b 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> >
> > [ ... ]
> >
> > > @@ -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;
> > > +
> >
> > Here obj->arena_data_off is a __u32, but the calculation is done in
> > size_t (64-bit). For a 4GB arena (which the test verifier_arena_large.c
> > uses), mmap_sz is 4GB and the result can approach 4GB - PAGE_SIZE. This
> > fits in u32 correctly.
> >
> > > @@ -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;
> > ^^^^^^^^^^^^^^^^^^^^^^
> >
> > Can this overflow when storing to reloc_desc->sym_off? The sym_off field
> > is 'int' (signed 32-bit), but arena_data_off can be larger than INT_MAX
> > for arenas approaching 4GB in size.
> >
> > For example, with a 4GB arena and one page of globals:
> > - mmap_sz = 4GB = 0x100000000
> > - data_alloc_sz = 4096
> > - arena_data_off = 0xFFFFF000 = 4294963200
> >
> > This value (4294963200) exceeds INT_MAX (2147483647), so when stored in
> > the signed int sym_off field, it becomes negative.
> >
> > Later in bpf_object__relocate_data(), this is used as:
> > insn[1].imm = insn[0].imm + relo->sym_off;
> >
> > With sym_off being negative due to overflow, this produces an incorrect
> > offset for accessing arena globals.
> >
> > Should sym_off be changed to an unsigned type to handle large arena
> > offsets correctly?
>
> AI has good a point, valid offset with 4GB arena will overflow int...
> But then ldimm64 instructions that allow direct access to arena have
> insn[1].imm as s32, so they cannot even address more than 2GB? Can you
> please double check this, maybe we actually treat insn[1].imm as u32
> on the verifier side for this use case?
>
I can confirm we treat insn[1].imm as a u32, but the following
relocation logic in
bpf_object__relocate_data looks suspect:
case RELO_DATA:
map = &obj->maps[relo->map_idx];
insn[1].imm = insn[0].imm + relo->sym_off;
if (obj->gen_loader) {
If sym_off is interpreted as a negative, then insn[1].imm should be
completely wrong and typecasting
back to u32 should do nothing. I added tests with both nonzero
insn[0].imm and st_value, and none
of them are failing which unless I"m missing something is honestly
kind of surprising.
I don't think sym_off should actually ever be negative. The st_value
from the ELF headers
should be always positive, and elf_find_func_offset_* also returns
only positive numbers.
Should we change sym_off in reloc_desc to be a u32? If anything that
is more accurate than having it
as an int.
For the rest of the feedback: Ack, will incorporate it into the next patch.
> >
> > [ ... ]
> >
> >
> > ---
> > 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/19901383089
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] libbpf: move arena globals to the end of the arena
2025-12-03 16:26 ` [PATCH v2 3/4] libbpf: move arena globals to the end of the arena Emil Tsalapatis
2025-12-03 16:52 ` bot+bpf-ci
@ 2025-12-06 0:11 ` Andrii Nakryiko
1 sibling, 0 replies; 11+ messages in thread
From: Andrii Nakryiko @ 2025-12-06 0:11 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song
On Wed, Dec 3, 2025 at 8:26 AM Emil Tsalapatis <emil@etsalapatis.com> 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>
> ---
> tools/lib/bpf/libbpf.c | 19 +++++++++++++++----
> .../bpf/progs/verifier_arena_large.c | 12 +++++++++---
> 2 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 706e7481bdf6..9642d697b690 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;
> + __u32 arena_data_off;
nit: use size_t, please
>
> 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);
> }
> }
> @@ -14387,6 +14393,7 @@ void bpf_object__destroy_subskeleton(struct bpf_object_subskeleton *s)
>
> int bpf_object__load_skeleton(struct bpf_object_skeleton *s)
> {
> + void *mmaped;
> int i, err;
>
> err = bpf_object__load(*s->obj);
> @@ -14402,7 +14409,11 @@ int bpf_object__load_skeleton(struct bpf_object_skeleton *s)
> if (!map_skel->mmaped)
> continue;
>
> - *map_skel->mmaped = map->mmaped;
> + mmaped = map->mmaped;
> + if (map->def.type == BPF_MAP_TYPE_ARENA)
> + mmaped += map->obj->arena_data_off;
> +
> + *map_skel->mmaped = mmaped;
this is minor, but I think doing
if (map->def.type == BPF_MAP_TYPE_ARENA)
/* arena data is placed at the end of arena memory region */
*map_skel->mmaped = map->mmaped + map->obj->arena_data_off;
else
*map_skel->mmaped = map->mmaped;
would be a bit cleaner and easier to follow and no need for extra
mutable variable
> }
>
> 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 [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] selftests/bpf: add tests for the arena offset of globals
2025-12-03 16:26 [PATCH v2 0/4] libbpf: move arena variables out of the zero Emil Tsalapatis
` (2 preceding siblings ...)
2025-12-03 16:26 ` [PATCH v2 3/4] libbpf: move arena globals to the end of the arena Emil Tsalapatis
@ 2025-12-03 16:26 ` Emil Tsalapatis
2025-12-06 0:11 ` [PATCH v2 0/4] libbpf: move arena variables out of the zero Andrii Nakryiko
4 siblings, 0 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2025-12-03 16:26 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 all three cases: The globals being small enough
to be placed at the maximum possible offset, being as large as
the arena itself and being placed at the very beginning, and
requiring an intermediate offset into the arena.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
.../selftests/bpf/prog_tests/verifier.c | 4 ++
.../bpf/progs/verifier_arena_globals1.c | 58 +++++++++++++++++++
.../bpf/progs/verifier_arena_globals2.c | 49 ++++++++++++++++
3 files changed, 111 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..3e68a5e83db8
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_arena_globals1.c
@@ -0,0 +1,58 @@
+// 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 (64)
+#define GLOBAL_PAGES (16)
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARENA);
+ __uint(map_flags, BPF_F_MMAPABLE);
+ __uint(max_entries, ARENA_PAGES); /* Arena of 64 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 small enough that we can apply the maximum
+ * offset into the arena. Userspace will also use this to
+ * ensure the offset doesn't unexpectedly change from
+ * under us.
+ */
+char __arena global_data[PAGE_SIZE][GLOBAL_PAGES];
+
+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 placed GLOBALS_PGOFF pages in. */
+ ret = bpf_arena_reserve_pages(&arena, globals, 1);
+ if (!ret)
+ return 2;
+#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..9b6a08135de5
--- /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); /* Arena of 32 pages (standard offset is 16 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] 11+ messages in thread* Re: [PATCH v2 0/4] libbpf: move arena variables out of the zero
2025-12-03 16:26 [PATCH v2 0/4] libbpf: move arena variables out of the zero Emil Tsalapatis
` (3 preceding siblings ...)
2025-12-03 16:26 ` [PATCH v2 4/4] selftests/bpf: add tests for the arena offset of globals Emil Tsalapatis
@ 2025-12-06 0:11 ` Andrii Nakryiko
4 siblings, 0 replies; 11+ messages in thread
From: Andrii Nakryiko @ 2025-12-06 0:11 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, andrii, eddyz87, ast, daniel, john.fastabend, memxor,
yonghong.song
On Wed, Dec 3, 2025 at 8:26 AM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>
> Modify libbpf to place arena globals in a small offset inside the arena
> mapping instead of the very beginning. This allows programs to leave
Description is now out of sync with the actual approach?
> 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
> =========
>
> 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 (4):
> selftests/bpf: explicitly account for globals in verifier_arena_large
> bpf/verifier: do not limit maximum direct offset into arena map
> libbpf: move arena globals to the end of the arena
> selftests/bpf: add tests for the arena offset of globals
>
> kernel/bpf/verifier.c | 8 +--
> tools/lib/bpf/libbpf.c | 19 ++++--
> .../selftests/bpf/prog_tests/verifier.c | 4 ++
> .../bpf/progs/verifier_arena_globals1.c | 58 +++++++++++++++++++
> .../bpf/progs/verifier_arena_globals2.c | 49 ++++++++++++++++
> .../bpf/progs/verifier_arena_large.c | 21 +++++--
> 6 files changed, 147 insertions(+), 12 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] 11+ messages in thread