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

["Emil Tsalapatis" <emil@etsalapatis.com>]

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;
>>  }