Re: [PATCH 2/3] bpf: Add sleepable arena page allocation path

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hard lockup via integer overflow: `data.i << PAGE_SHIFT` passes an astronomically large negative size to `flush_vmap_cache` on mapping failure.
--

--- Patch [2]: [PATCH 2/3] bpf: Add sleepable arena page allocation path ---
commit a93c0f542ad1ecf8f00cde8cf86699ce952035a7
Author: Emil Tsalapatis <[email protected]>

bpf: Add sleepable arena page allocation path

This commit adds a separate sleepable allocation path for BPF arena memory.
It preallocates memory before taking the critical section to prevent
frequent allocation failures from sleepable BPF contexts.

> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index da356989786a0..126ddae763f9a 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c

[ ... ]

> +static long arena_alloc_pages_sleepable(struct bpf_arena *arena, long page_cnt,
> +		long uaddr, long pgoff, int node_id)
> +{

[ ... ]

> +out_unmap:
> +	/* data.i pages were mapped, undo only those who failed to map */
> +	flush_vmap_cache(kern_vm_start + uaddr32, data.i << PAGE_SHIFT);

[Severity: High]
Can this shift operation overflow?

If apply_to_page_range() maps a large number of pages (e.g. over 524,287)
before failing, data.i << PAGE_SHIFT will overflow since data.i is a
signed 32-bit int.

Because flush_vmap_cache() expects an unsigned long size, a negative value
here would sign-extend to a massive 64-bit size, potentially causing a CPU
stall or lockup in the flush routine.

Would casting data.i to unsigned long before the shift prevent this?

> +	range_tree_set(&arena->rt, pgoff + data.i, page_cnt - data.i);
> +
> +	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2