[PATCH bpf] bpf: Fix off-by-one boundary validation in arena direct-value access

[PATCH bpf] bpf: Fix off-by-one boundary validation in arena direct-value access

[Junyoung Jang]

BPF_MAP_TYPE_ARENA accepts BPF_PSEUDO_MAP_VALUE offsets at exactly
the end of the arena mapping (off == arena_size). The boundary check
in arena_map_direct_value_addr() uses `>` instead of `>=`, which
incorrectly allows a one-past-end pointer to be accepted.

Change the condition to `>=` to correctly reject offsets that fall
outside the valid arena user_vm range.

Signed-off-by: Junyoung Jang <graypanda.inzag@gmail.com>
---
 kernel/bpf/arena.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 802656c6fd3c..49a8f7b1beef 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -511,7 +511,7 @@ 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)
+	if ((u64)off >= arena->user_vm_end - arena->user_vm_start)
 		return -ERANGE;
 	*imm = (unsigned long)arena->user_vm_start;
 	return 0;
-- 
2.43.0

Re: [PATCH bpf] bpf: Fix off-by-one boundary validation in arena direct-value access

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

>= to correctly reject offsets outside the valid arena user_vm range.

This commit fixes a bug but lacks a Fixes: tag. The commit being fixed
appears to be:

Fixes: 5a2f726fe6d0 ("adding ci files")

Should this tag be added to help with backporting and tracking the
original issue?


---
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/24960599139

[PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access

[Junyoung Jang]

BPF_MAP_TYPE_ARENA accepts BPF_PSEUDO_MAP_VALUE offsets at exactly
the end of the arena mapping (off == arena_size). The boundary check
in arena_map_direct_value_addr() uses `>` instead of `>=`, which
incorrectly allows a one-past-end pointer to be accepted.

Change the condition to `>=` to correctly reject offsets that fall
outside the valid arena user_vm range.

Fixes: 317460317a02 ("bpf: Introduce bpf_arena.")
Signed-off-by: Junyoung Jang <graypanda.inzag@gmail.com>
---
 kernel/bpf/arena.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 802656c6fd3c..49a8f7b1beef 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -511,7 +511,7 @@ 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)
+	if ((u64)off >= arena->user_vm_end - arena->user_vm_start)
 		return -ERANGE;
 	*imm = (unsigned long)arena->user_vm_start;
 	return 0;
-- 
2.43.0

Re: [PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [Medium] Data race and bounds check bypass in arena_map_direct_value_addr() due to lockless reads
--

commit a9b5d3016475abf10be02a320dc627c95b306d92
Author: Junyoung Jang <graypanda.inzag@gmail.com>

bpf: Fix off-by-one boundary validation in arena direct-value access

This commit fixes an off-by-one error in the arena map boundary check
where offsets exactly at the end of the mapping were incorrectly accepted.

> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 802656c6fd3c..49a8f7b1beef 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -511,7 +511,7 @@ 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)
> +	if ((u64)off >= arena->user_vm_end - arena->user_vm_start)

Could this evaluation be vulnerable to a data race?

This function reads arena->user_vm_end and arena->user_vm_start without
holding arena->lock or using READ_ONCE().

Concurrently, a user-space process calling mmap() on the arena map FD
can invoke arena_map_mmap(), which updates these fields under arena->lock.
Because user_vm_start is updated first, a concurrent reader here might
observe the new user_vm_start (a high address) and the old user_vm_end
(which might be 0).

In that scenario, arena->user_vm_end - arena->user_vm_start would
underflow to a large 64-bit value, which would allow arbitrary offsets
to bypass this bounds check.

Does this logic require synchronization or READ_ONCE() annotations to
ensure the verifier's boundary validation remains effective?

>  		return -ERANGE;
>  	*imm = (unsigned long)arena->user_vm_start;
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260426172505.1947915-1-graypanda.inzag@gmail.com?part=1

Re: [PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access

[Emil Tsalapatis]

On Sun Apr 26, 2026 at 2:17 PM EDT, sashiko-bot wrote:
> Sashiko AI review found 1 potential issue(s):
> - [Medium] Data race and bounds check bypass in arena_map_direct_value_addr() due to lockless reads
> --
>
> commit a9b5d3016475abf10be02a320dc627c95b306d92
> Author: Junyoung Jang <graypanda.inzag@gmail.com>
>
> bpf: Fix off-by-one boundary validation in arena direct-value access
>
> This commit fixes an off-by-one error in the arena map boundary check
> where offsets exactly at the end of the mapping were incorrectly accepted.
>
>> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
>> index 802656c6fd3c..49a8f7b1beef 100644
>> --- a/kernel/bpf/arena.c
>> +++ b/kernel/bpf/arena.c
>> @@ -511,7 +511,7 @@ 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)
>> +	if ((u64)off >= arena->user_vm_end - arena->user_vm_start)
>
> Could this evaluation be vulnerable to a data race?
>
> This function reads arena->user_vm_end and arena->user_vm_start without
> holding arena->lock or using READ_ONCE().
>
> Concurrently, a user-space process calling mmap() on the arena map FD
> can invoke arena_map_mmap(), which updates these fields under arena->lock.
> Because user_vm_start is updated first, a concurrent reader here might
> observe the new user_vm_start (a high address) and the old user_vm_end
> (which might be 0).
>
> In that scenario, arena->user_vm_end - arena->user_vm_start would
> underflow to a large 64-bit value, which would allow arbitrary offsets
> to bypass this bounds check.

This seems like a valid concern, though it is unrelated to the fix here
which seems correct. Can you sent a followup patch using an arena lock
guard in map_direct_value_addr? I don't see any issues with taking the
lock since the method is only used at verification time.

>
> Does this logic require synchronization or READ_ONCE() annotations to
> ensure the verifier's boundary validation remains effective?
>
>>  		return -ERANGE;
>>  	*imm = (unsigned long)arena->user_vm_start;
>>  	return 0;
>>  }

Re: [PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access

[Emil Tsalapatis]

On Sun Apr 26, 2026 at 1:25 PM EDT, Junyoung Jang wrote:
> BPF_MAP_TYPE_ARENA accepts BPF_PSEUDO_MAP_VALUE offsets at exactly
> the end of the arena mapping (off == arena_size). The boundary check
> in arena_map_direct_value_addr() uses `>` instead of `>=`, which
> incorrectly allows a one-past-end pointer to be accepted.
>
> Change the condition to `>=` to correctly reject offsets that fall
> outside the valid arena user_vm range.
>
> Fixes: 317460317a02 ("bpf: Introduce bpf_arena.")
> Signed-off-by: Junyoung Jang <graypanda.inzag@gmail.com>
> ---

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

>  kernel/bpf/arena.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 802656c6fd3c..49a8f7b1beef 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -511,7 +511,7 @@ 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)
> +	if ((u64)off >= arena->user_vm_end - arena->user_vm_start)
>  		return -ERANGE;
>  	*imm = (unsigned long)arena->user_vm_start;
>  	return 0;

Re: [PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access

[Alexei Starovoitov]

On Sun, Apr 26, 2026 at 12:16 PM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>
> On Sun Apr 26, 2026 at 2:17 PM EDT, sashiko-bot wrote:
> > Sashiko AI review found 1 potential issue(s):
> > - [Medium] Data race and bounds check bypass in arena_map_direct_value_addr() due to lockless reads
> > --
> >
> > commit a9b5d3016475abf10be02a320dc627c95b306d92
> > Author: Junyoung Jang <graypanda.inzag@gmail.com>
> >
> > bpf: Fix off-by-one boundary validation in arena direct-value access
> >
> > This commit fixes an off-by-one error in the arena map boundary check
> > where offsets exactly at the end of the mapping were incorrectly accepted.
> >
> >> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> >> index 802656c6fd3c..49a8f7b1beef 100644
> >> --- a/kernel/bpf/arena.c
> >> +++ b/kernel/bpf/arena.c
> >> @@ -511,7 +511,7 @@ 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)
> >> +    if ((u64)off >= arena->user_vm_end - arena->user_vm_start)
> >
> > Could this evaluation be vulnerable to a data race?
> >
> > This function reads arena->user_vm_end and arena->user_vm_start without
> > holding arena->lock or using READ_ONCE().
> >
> > Concurrently, a user-space process calling mmap() on the arena map FD
> > can invoke arena_map_mmap(), which updates these fields under arena->lock.
> > Because user_vm_start is updated first, a concurrent reader here might
> > observe the new user_vm_start (a high address) and the old user_vm_end
> > (which might be 0).
> >
> > In that scenario, arena->user_vm_end - arena->user_vm_start would
> > underflow to a large 64-bit value, which would allow arbitrary offsets
> > to bypass this bounds check.
>
> This seems like a valid concern, though it is unrelated to the fix here
> which seems correct. Can you sent a followup patch using an arena lock
> guard in map_direct_value_addr? I don't see any issues with taking the
> lock since the method is only used at verification time.

No. sashk is wrong here.
In general I suggest to ignore sashk's medium and low suggestions.
'critical' are often correct.
'high' should be considered, but acting on them needs thought.