* [PATCH bpf] bpf: Fix off-by-one boundary validation in arena direct-value access [not found] <2026042614-scowling-hankie-f84b@gregkh> @ 2026-04-26 15:27 ` Junyoung Jang 2026-04-26 16:11 ` bot+bpf-ci 2026-04-26 17:25 ` [PATCH bpf v2] " Junyoung Jang 1 sibling, 1 reply; 7+ messages in thread From: Junyoung Jang @ 2026-04-26 15:27 UTC (permalink / raw) To: ast, daniel, andrii, eddyz87, memxor Cc: martin.lau, song, yonghong.song, jolsa, bpf, linux-kernel, greg, security, 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] bpf: Fix off-by-one boundary validation in arena direct-value access 2026-04-26 15:27 ` [PATCH bpf] bpf: Fix off-by-one boundary validation in arena direct-value access Junyoung Jang @ 2026-04-26 16:11 ` bot+bpf-ci 0 siblings, 0 replies; 7+ messages in thread From: bot+bpf-ci @ 2026-04-26 16:11 UTC (permalink / raw) To: graypanda.inzag, ast, daniel, andrii, eddyz87, memxor Cc: martin.lau, song, yonghong.song, jolsa, bpf, linux-kernel, greg, security, graypanda.inzag, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai [-- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf v2] bpf: Fix off-by-one boundary validation in arena direct-value access [not found] <2026042614-scowling-hankie-f84b@gregkh> 2026-04-26 15:27 ` [PATCH bpf] bpf: Fix off-by-one boundary validation in arena direct-value access Junyoung Jang @ 2026-04-26 17:25 ` Junyoung Jang 2026-04-26 18:17 ` sashiko-bot 2026-04-26 19:17 ` Emil Tsalapatis 1 sibling, 2 replies; 7+ messages in thread From: Junyoung Jang @ 2026-04-26 17:25 UTC (permalink / raw) To: ast, daniel, andrii, eddyz87, memxor Cc: martin.lau, song, yonghong.song, jolsa, bpf, linux-kernel, greg, security, 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 ^ permalink raw reply related [flat|nested] 7+ 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:16 ` Emil Tsalapatis 2026-04-26 19:17 ` Emil Tsalapatis 1 sibling, 1 reply; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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 1 sibling, 0 replies; 7+ 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] 7+ messages in thread
end of thread, other threads:[~2026-04-26 19:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <2026042614-scowling-hankie-f84b@gregkh>
2026-04-26 15:27 ` [PATCH bpf] bpf: Fix off-by-one boundary validation in arena direct-value access Junyoung Jang
2026-04-26 16:11 ` bot+bpf-ci
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:58 ` Alexei Starovoitov
2026-04-26 19:17 ` Emil Tsalapatis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox