Re: [PATCH bpf-next v1 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure

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

Pre-existing issues:
- [High] `range_tree_set()` suffers from the exact same partial-modification flaw that the patch fixes in `range_tree_clear()`, leading to permanent arena slot leaks.
--

--- Patch [1]: [PATCH bpf-next v1 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure ---
commit 8092890527e00050d54d5c7b5deedf81e64396d7
Author: Yuan Chen <[email protected]>

bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure

This patch addresses an inconsistency in range_tree_clear() where memory
allocation failures could leave the range tree partially modified. It fixes
this by pre-allocating the required node before altering the tree structure.

> diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
> index 2f28886f3ff75..24fa32d4356bb 100644
> --- a/kernel/bpf/range_tree.c
> +++ b/kernel/bpf/range_tree.c
> @@ -143,16 +143,21 @@ int range_tree_clear(struct range_tree *rt, u32 start, u32 len)
>  		if (rn->rn_start < start && rn->rn_last > last) {
>  			u32 old_last = rn->rn_last;
>  
> +			/* Pre-allocate the right-half node before modifying
> +			 * the tree. If allocation fails we return -ENOMEM
> +			 * without altering the range tree.
> +			 */
> +			new_rn = kmalloc_nolock(sizeof(struct range_node),
> +						__GFP_ACCOUNT, NUMA_NO_NODE);
> +			if (!new_rn)
> +				return -ENOMEM;
> +
>  			/* Overlaps with the entire clearing range */
>  			range_it_remove(rn, rt);

[Severity: High]
This isn't a bug introduced by this patch, but does range_tree_set()
suffer from the exact same partial-modification flaw?

Looking at range_tree_set() in kernel/bpf/range_tree.c, it also modifies the
tree before attempting an allocation:

	/* Clear out everything in the range we want to set. */
	err = range_tree_clear(rt, start, len);
	if (err)
		return err;
...
	} else {
		left = kmalloc_nolock(sizeof(struct range_node), __GFP_ACCOUNT, NUMA_NO_NODE);
		if (!left)
			return -ENOMEM;

If kmalloc_nolock() fails here, the function returns -ENOMEM but leaves the
tree with the overlapping nodes already deleted by range_tree_clear(). Any
previously free sub-ranges that were cleared could be permanently lost.

Furthermore, in kernel/bpf/arena.c, arena_free_pages() appears to ignore the
return value entirely:

	range_tree_set(&arena->rt, pgoff, page_cnt);

Could an allocation failure here result in a silent, permanent leak of arena
virtual address space?

[ ... ]

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