Re: [PATCH v3] bpf: Fix u32 overflow issue in map batch operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing size_t casts in queue_stack_maps.c cause pointer arithmetic to overflow u32, corrupting map data.
- [High] qs->size is typed as u32, which can silently overflow to 0 when max_entries is U32_MAX.
--

commit aa1d0d53f1c01402c01cb62c0b2ef581ca4869bd
Author: Masoud Aghasi <maghasi@disroot.org>

bpf: Fix u32 overflow issue in map batch operations

This patch fixes a u32 overflow in several map batch operations that can occur
when the map size exceeds 4GB. It adds size_t casts to prevent the offset and
size calculations from overflowing during memory operations.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6874ba1424af0..731388ae00ad1 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
[ ... ]
> @@ -2179,12 +2179,12 @@ int generic_map_lookup_batch(struct bpf_map *map,
>  		if (err)
>  			goto free_buf;
>  
> -		if (copy_to_user(keys + cp * map->key_size, key,
> +		if (copy_to_user(keys + (size_t)cp * map->key_size, key,
>  				 map->key_size)) {
>  			err = -EFAULT;
>  			goto free_buf;
>  		}
> -		if (copy_to_user(values + cp * value_size, value, value_size)) {
> +		if (copy_to_user(values + (size_t)cp * value_size, value, value_size)) {
>  			err = -EFAULT;
>  			goto free_buf;
>  		}

[Severity: Critical]
This is a pre-existing issue, but since this patch fixes u32 overflows for
map element offsets, could we also address similar overflows in
queue_stack_maps.c?

If a queue or stack map is created where the capacity (max_entries *
value_size) exceeds 4GB, do these calculations wrap the element offset when
head, tail, or index counters multiply with map.value_size?

kernel/bpf/queue_stack_maps.c:__queue_map_get() {
    ...
    ptr = &qs->elements[qs->tail * qs->map.value_size];
    ...
}

kernel/bpf/queue_stack_maps.c:__stack_map_get() {
    ...
    ptr = &qs->elements[index * qs->map.value_size];
    ...
}

kernel/bpf/queue_stack_maps.c:queue_stack_map_push_elem() {
    ...
    dst = &qs->elements[qs->head * qs->map.value_size];
    ...
}

[Severity: High]
This is also a pre-existing issue, but I noticed a potential truncation 
when allocating these maps:

kernel/bpf/queue_stack_maps.c:queue_stack_map_alloc() {
    ...
    qs->size = size;
    ...
}

If a user allocates a queue or stack map with max_entries set exactly to
U32_MAX, the required size calculation (u64)attr->max_entries + 1 evaluates
to 0x100000000. Because qs->size is typed as u32, does this assignment
silently truncate the size to 0, resulting in a map that behaves like it has
0 capacity?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903082734.623904-1-maghasi@disroot.org?part=1