* Re: [PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access
2026-04-26 17:25 ` [PATCH bpf v2] " Junyoung Jang
@ 2026-04-26 18:17 ` sashiko-bot
2026-04-26 19:16 ` Emil Tsalapatis
2026-04-26 19:17 ` Emil Tsalapatis
2026-05-09 23:20 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-04-26 18:17 UTC (permalink / raw)
To: Junyoung Jang; +Cc: bpf
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access
2026-04-26 18:17 ` sashiko-bot
@ 2026-04-26 19:16 ` Emil Tsalapatis
2026-04-26 19:58 ` Alexei Starovoitov
0 siblings, 1 reply; 8+ messages in thread
From: Emil Tsalapatis @ 2026-04-26 19:16 UTC (permalink / raw)
To: sashiko, Junyoung Jang; +Cc: bpf
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;
>> }
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access
2026-04-26 19:16 ` Emil Tsalapatis
@ 2026-04-26 19:58 ` Alexei Starovoitov
0 siblings, 0 replies; 8+ messages in thread
From: Alexei Starovoitov @ 2026-04-26 19:58 UTC (permalink / raw)
To: Emil Tsalapatis; +Cc: sashiko, Junyoung Jang, bpf
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access
2026-04-26 17:25 ` [PATCH bpf v2] " Junyoung Jang
2026-04-26 18:17 ` sashiko-bot
@ 2026-04-26 19:17 ` Emil Tsalapatis
2026-05-09 23:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 8+ messages in thread
From: Emil Tsalapatis @ 2026-04-26 19:17 UTC (permalink / raw)
To: Junyoung Jang, ast, daniel, andrii, eddyz87, memxor
Cc: martin.lau, song, yonghong.song, jolsa, bpf, linux-kernel, greg,
security
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;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access
2026-04-26 17:25 ` [PATCH bpf v2] " Junyoung Jang
2026-04-26 18:17 ` sashiko-bot
2026-04-26 19:17 ` Emil Tsalapatis
@ 2026-05-09 23:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-05-09 23:20 UTC (permalink / raw)
To: Junyoung Jang
Cc: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, bpf, linux-kernel, greg, security
Hello:
This patch was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Mon, 27 Apr 2026 02:25:05 +0900 you 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.
>
> [...]
Here is the summary with links:
- [bpf,v2] bpf: Fix off-by-one boundary validation in arena direct-value access
https://git.kernel.org/bpf/bpf/c/3ac1a467e376
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 8+ messages in thread