Re: [PATCH bpf-next v3 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker

[Emil Tsalapatis]

On Mon Aug 10, 2026 at 9:48 AM EDT, chenyuan_fl wrote:
> From: Yuan Chen <chenyuan@xxxxxxxxxx>
>
> arena_free_pages() and arena_free_worker() now handle range_tree_set()
> errors. arena_free_pages() aborts the free on error, and
> arena_free_worker() moves range_tree_set() before PTE clearing so that a
> failed tree update leaves the PTEs intact instead of freeing pages that
> the arena free tree does not track.
>
> Also check the range_tree_set() return value in arena_alloc_pages()'s
> error path, which restores the unpopulated tail of a partially allocated
> range; log a warning instead of silently leaking the virtual range when
> the tree update fails.
>
> Signed-off-by: Yuan Chen <chenyuan@xxxxxxxxxx>

Reviewed-by: Emil Tsalapatis <emil@xxxxxxxxxxxxxxx>

The fact we can fail to free a page because of an allocation failure is
...not great. Especially considering the failure is silent (we can't
turn this into a warning ofc bc it would be triggerable by user error).
Imo this is very close to a silent memory leak, but the only mitigation
I can think of is some kind of retry, and going down that route would get
really messy really fast.

> ---
>  kernel/bpf/arena.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 555ee2531ef9..d57a0b617525 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -766,7 +766,9 @@ static long arena_alloc_pages(struct bpf_arena *arena, long uaddr, long page_cnt
>  	bpf_map_memcg_exit(old_memcg, new_memcg);
>  	return clear_lo32(arena->user_vm_start) + uaddr32;
>  out:
> -	range_tree_set(&arena->rt, pgoff + mapped, page_cnt - mapped);
> +	if (range_tree_set(&arena->rt, pgoff + mapped, page_cnt - mapped))
> +		pr_warn_ratelimited("bpf_arena: failed to restore free range %ld+%ld after partial alloc\n",
> +				    pgoff + mapped, page_cnt - mapped);
>  	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
>  	if (mapped) {
>  		flush_vmap_cache(kern_vm_start + uaddr32, mapped << PAGE_SHIFT);
> @@ -881,7 +883,12 @@ static void arena_free_pages(struct bpf_arena *arena, long uaddr, long page_cnt,
>  	if (ret)
>  		goto defer;
>  
> -	range_tree_set(&arena->rt, pgoff, page_cnt);
> +	ret = range_tree_set(&arena->rt, pgoff, page_cnt);
> +	if (ret) {
> +		raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
> +		bpf_map_memcg_exit(old_memcg, new_memcg);
> +		return;
> +	}
>  
>  	init_llist_head(&free_pages);
>  	cdata.arena = arena;
> @@ -1004,11 +1011,18 @@ static void arena_free_worker(struct work_struct *work)
>  		kaddr = arena_vm_start + s->uaddr;
>  		pgoff = compute_pgoff(arena, s->uaddr);
>  
> +		/*
> +		 * Mark the range as free before clearing PTEs so that
> +		 * if the range tree update fails we leave the PTEs
> +		 * intact, avoiding an unrecoverable state where pages
> +		 * are freed but the arena free tree does not track them.
> +		 */
> +		if (range_tree_set(&arena->rt, pgoff, page_cnt))
> +			continue;
> +
>  		/* clear ptes and collect pages in free_pages llist */
>  		apply_to_existing_page_range(&init_mm, kaddr, page_cnt << PAGE_SHIFT,
>  					     apply_range_clear_cb, &cdata);
> -
> -		range_tree_set(&arena->rt, pgoff, page_cnt);
>  	}
>  	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
>