[PATCH bpf-next v4 0/3] bpf, arena: fix range_tree consistency on allocation failure

[PATCH bpf-next v4 0/3] bpf, arena: fix range_tree consistency on allocation failure

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

The arena range tree can be left inconsistent when kmalloc_nolock()
fails mid-operation. Patch 1 fixes range_tree_clear(), patch 2 fixes
range_tree_set(), patch 3 makes the arena free paths handle
range_tree_set() failures and checks the return value in
arena_alloc_pages()'s partial-allocation error path.

Changes in v4:
  - arena_free_worker(): keep a span whose range_tree_set() failed on
    arena->free_spans and retry it on a later worker run, instead of
    leaving it in the drained list where the second loop would still
    zap user VMAs and free the span (dropping the free request), as
    pointed out by Emil Tsalapatis.

Changes in v3:
  - Check range_tree_set() return value in arena_alloc_pages()'s error
    path, which restores the unpopulated tail of a partially allocated
    range (previously ignored), as pointed out in review.

Changes in v2:
  - Fix multi-line comment style in patches 1 and 3 (opening /* on its
    own line), as pointed out in review.

Note: arena_vm_fault()'s two recovery paths (restoring the range to the
free tree after allocation/mapping failure) also call range_tree_set()
without checking the return value; that is addressed in a separate
series.

Yuan Chen (3):
  bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock
    failure
  bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  bpf, arena: check range_tree_set return in arena_free_pages and
    arena_free_worker

 kernel/bpf/arena.c      | 43 ++++++++++++++++++++++++-----
 kernel/bpf/range_tree.c | 61 ++++++++++++++++++++++++++++++-----------
 2 files changed, 81 insertions(+), 23 deletions(-)

-- 
2.54.0

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

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

range_tree_clear() pre-allocates the right-half node before modifying
the tree, so an allocation failure returns -ENOMEM without altering the
range tree.

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/range_tree.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
index 2f28886f3ff7..15b588377a76 100644
--- a/kernel/bpf/range_tree.c
+++ b/kernel/bpf/range_tree.c
@@ -143,16 +143,22 @@ 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);
 			rn->rn_last = start - 1;
 			range_it_insert(rn, rt);
 
-			/* Add a range */
-			new_rn = kmalloc_nolock(sizeof(struct range_node), __GFP_ACCOUNT,
-						NUMA_NO_NODE);
-			if (!new_rn)
-				return -ENOMEM;
+			/* Add right-half range */
 			new_rn->rn_start = last + 1;
 			new_rn->rn_last = old_last;
 			range_it_insert(new_rn, rt);
-- 
2.54.0

[PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

range_tree_set() pre-allocates the node needed for a brand-new range
before calling range_tree_clear(), so an allocation failure returns
-ENOMEM without having modified the tree (previously the overlapping
nodes were already removed by range_tree_clear() before the allocation
was attempted, permanently losing the cleared sub-ranges).

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/range_tree.c | 45 +++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
index 15b588377a76..54055b1fe541 100644
--- a/kernel/bpf/range_tree.c
+++ b/kernel/bpf/range_tree.c
@@ -199,6 +199,7 @@ int is_range_tree_set(struct range_tree *rt, u32 start, u32 len)
 int range_tree_set(struct range_tree *rt, u32 start, u32 len)
 {
 	u32 last = start + len - 1;
+	struct range_node *new_rn = NULL;
 	struct range_node *right;
 	struct range_node *left;
 	int err;
@@ -208,20 +209,40 @@ int range_tree_set(struct range_tree *rt, u32 start, u32 len)
 	if (left && left->rn_start <= start && left->rn_last >= last)
 		return 0;
 
+	/*
+	 * A new node is needed only when the range has no adjacent free
+	 * range on either side.  This is known before clearing: any range
+	 * covering start - 1 or last + 1 survives the clear as an adjacent
+	 * piece.  Allocate only in that case, before modifying the tree, so
+	 * a failure leaves the range tree unmodified
+	 */
+	left = range_it_iter_first(rt, start - 1, start - 1);
+	right = range_it_iter_first(rt, last + 1, last + 1);
+	if (!left && !right) {
+		new_rn = kmalloc_nolock(sizeof(struct range_node),
+					__GFP_ACCOUNT, NUMA_NO_NODE);
+		if (!new_rn)
+			return -ENOMEM;
+	}
+
 	/* Clear out everything in the range we want to set. */
 	err = range_tree_clear(rt, start, len);
 	if (err)
-		return err;
+		goto out_free_new;
 
 	/* Do we have a left-adjacent range ? */
 	left = range_it_iter_first(rt, start - 1, start - 1);
-	if (left && left->rn_last + 1 != start)
-		return -EFAULT;
+	if (left && left->rn_last + 1 != start) {
+		err = -EFAULT;
+		goto out_free_new;
+	}
 
 	/* Do we have a right-adjacent range ? */
 	right = range_it_iter_first(rt, last + 1, last + 1);
-	if (right && right->rn_start != last + 1)
-		return -EFAULT;
+	if (right && right->rn_start != last + 1) {
+		err = -EFAULT;
+		goto out_free_new;
+	}
 
 	if (left && right) {
 		/* Combine left and right adjacent ranges */
@@ -241,14 +262,16 @@ int range_tree_set(struct range_tree *rt, u32 start, u32 len)
 		right->rn_start = start;
 		range_it_insert(right, rt);
 	} else {
-		left = kmalloc_nolock(sizeof(struct range_node), __GFP_ACCOUNT, NUMA_NO_NODE);
-		if (!left)
-			return -ENOMEM;
-		left->rn_start = start;
-		left->rn_last = last;
-		range_it_insert(left, rt);
+		/* No adjacent ranges; use the pre-allocated node */
+		new_rn->rn_start = start;
+		new_rn->rn_last = last;
+		range_it_insert(new_rn, rt);
 	}
 	return 0;
+
+out_free_new:
+	kfree_nolock(new_rn);
+	return err;
 }
 
 void range_tree_destroy(struct range_tree *rt)
-- 
2.54.0

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

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

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.

range_tree_set() is failure-atomic (it pre-allocates the node before
touching the tree), so on -ENOMEM the range stays tracked as allocated
and the pages remain mapped and accessible. A failed free is therefore
retryable, and arena_map_free() reclaims any retained pages at map
destruction; aborting the free avoids clearing PTEs for pages the arena
free tree does not track.

In arena_free_worker() a failed tree update used to leave the span in
the drained list, where the second loop would still flush TLB entries,
zap user VMAs, and free the span itself: the free request was dropped,
user mappings were destroyed for a free that never happened, and the
pages stayed mapped until map destruction. Keep failed spans on
arena->free_spans instead and retry them on a later worker run; only
spans whose PTE clearing actually ran are flushed, zapped, and released.

Suggested-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/arena.c | 43 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 555ee2531ef9..1315872941e1 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,18 @@ 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) {
+		/*
+		 * range_tree_set() is failure-atomic, so -ENOMEM leaves the
+		 * range allocated and the pages mapped. Abort the free rather
+		 * than returning pages the free tree does not track; a later
+		 * free of the same range can succeed.
+		 */
+		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;
@@ -977,12 +990,13 @@ static void arena_free_worker(struct work_struct *work)
 	struct llist_node *list, *pos, *t;
 	struct arena_free_span *s;
 	u64 arena_vm_start, user_vm_start;
-	struct llist_head free_pages;
+	struct llist_head free_pages, cleared;
 	struct clear_range_data cdata;
 	struct page *page;
 	unsigned long full_uaddr;
 	long kaddr, page_cnt, pgoff;
 	unsigned long flags;
+	bool retry = false;
 
 	if (raw_res_spin_lock_irqsave(&arena->spinlock, flags)) {
 		schedule_work(work);
@@ -992,28 +1006,43 @@ static void arena_free_worker(struct work_struct *work)
 	bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
 
 	init_llist_head(&free_pages);
+	init_llist_head(&cleared);
 	cdata.arena = arena;
 	cdata.free_pages = &free_pages;
 	arena_vm_start = bpf_arena_get_kern_vm_start(arena);
 	user_vm_start = bpf_arena_get_user_vm_start(arena);
 
 	list = llist_del_all(&arena->free_spans);
-	llist_for_each(pos, list) {
+	llist_for_each_safe(pos, t, list) {
 		s = llist_entry(pos, struct arena_free_span, node);
 		page_cnt = s->page_cnt;
 		kaddr = arena_vm_start + s->uaddr;
 		pgoff = compute_pgoff(arena, s->uaddr);
 
+		/*
+		 * Set the range free before clearing PTEs, and requeue the
+		 * span on failure: the PTEs stay intact and the free is
+		 * retried later. Only spans moved to @cleared (PTE clearing
+		 * actually ran) reach the flush/zap/release loop below.
+		 */
+		if (range_tree_set(&arena->rt, pgoff, page_cnt)) {
+			llist_add(&s->node, &arena->free_spans);
+			retry = true;
+			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);
+		llist_add(&s->node, &cleared);
 	}
 	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
 
+	if (retry)
+		irq_work_queue(&arena->free_irq);
+
 	/* Iterate the list again without holding spinlock to do the tlb flush and zap_pages */
-	llist_for_each_safe(pos, t, list) {
+	llist_for_each_safe(pos, t, cleared.first) {
 		s = llist_entry(pos, struct arena_free_span, node);
 		page_cnt = s->page_cnt;
 		full_uaddr = clear_lo32(user_vm_start) + s->uaddr;
-- 
2.54.0

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

[sashiko-bot]

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

New issues:
- [Critical] Re-arming irq_work in arena_free_worker() bypasses teardown synchronization, leading to UAF of the bpf_arena structure on map destruction.
- [High] Immediate, unconditional retry in arena_free_worker() when range_tree_set() fails creates a tight CPU busy-loop and livelock.
- [High] Aborting arena_free_pages() on range_tree_set() failure silently leaks physical memory and memcg quota during the arena's lifetime.

Pre-existing issues:
- [High] Calling apply_to_existing_page_range() while holding raw_res_spin_lock_irqsave violates lock wait-type nesting rules (LD_WAIT_SPIN > LD_WAIT_CONFIG).
--

--- Patch [3]: [PATCH bpf-next v4 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker ---
commit 5874fa92054ab81b6a7b82ddf6fae39167bcb8d3
Author: Yuan Chen <chenyuan@kylinos.cn>
Subject: bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker

This commit handles errors from range_tree_set() in arena_free_pages() and
arena_free_worker(). It aborts the free operation on failure to avoid clearing
PTEs for untracked pages, and modifies arena_free_worker() to requeue failed
spans for later retry.

> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 7b6847200b431..169b2e71e9cbb 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c

[ ... ]

> @@ -881,7 +883,18 @@ 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) {
> +		/*
> +		 * range_tree_set() is failure-atomic, so -ENOMEM leaves the
> +		 * range allocated and the pages mapped. Abort the free rather
> +		 * than returning pages the free tree does not track; a later
> +		 * free of the same range can succeed.
> +		 */
> +		raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
> +		bpf_map_memcg_exit(old_memcg, new_memcg);
> +		return;
> +	}

[Severity: High]
When range_tree_set() fails here, does returning early permanently leak
physical pages and memcg quota if this is called from the unwind path in
arena_alloc_pages()?

If arena_alloc_pages() fails partway through, it calls arena_free_pages()
to unwind. If this early return is hit, the physical pages remain mapped and
allocated. Since the original allocation returns 0, the BPF program cannot
free these pages later.

>  
>  	init_llist_head(&free_pages);
>  	cdata.arena = arena;

[ ... ]

> @@ -992,28 +1006,43 @@ static void arena_free_worker(struct work_struct *work)
>  	bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
>  
>  	init_llist_head(&free_pages);
> +	init_llist_head(&cleared);
>  	cdata.arena = arena;
>  	cdata.free_pages = &free_pages;
>  	arena_vm_start = bpf_arena_get_kern_vm_start(arena);
>  	user_vm_start = bpf_arena_get_user_vm_start(arena);
>  
>  	list = llist_del_all(&arena->free_spans);
> -	llist_for_each(pos, list) {
> +	llist_for_each_safe(pos, t, list) {
>  		s = llist_entry(pos, struct arena_free_span, node);
>  		page_cnt = s->page_cnt;
>  		kaddr = arena_vm_start + s->uaddr;
>  		pgoff = compute_pgoff(arena, s->uaddr);
>  
> +		/*
> +		 * Set the range free before clearing PTEs, and requeue the
> +		 * span on failure: the PTEs stay intact and the free is
> +		 * retried later. Only spans moved to @cleared (PTE clearing
> +		 * actually ran) reach the flush/zap/release loop below.
> +		 */
> +		if (range_tree_set(&arena->rt, pgoff, page_cnt)) {
> +			llist_add(&s->node, &arena->free_spans);
> +			retry = true;
> +			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);

[Severity: High]
This isn't a bug introduced by this patch, but does calling
apply_to_existing_page_range() here violate lock wait-type nesting rules?

This code executes while holding the raw_res_spin_lock_irqsave() raw
spinlock. apply_to_existing_page_range() walks page tables and acquires the
page table lock (PTL). The PTL is a spinlock_t, which maps to a sleepable
rt_mutex on PREEMPT_RT kernels. Acquiring a sleepable lock inside a raw lock
can cause a scheduling while atomic bug.

> -
> -		range_tree_set(&arena->rt, pgoff, page_cnt);
> +		llist_add(&s->node, &cleared);
>  	}
>  	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
>  
> +	if (retry)
> +		irq_work_queue(&arena->free_irq);

[Severity: High]
Can this unconditional retry create a tight CPU busy-loop and livelock?

If range_tree_set() fails (e.g., from -ENOMEM due to memory cgroup limits),
it sets retry = true and queues arena->free_irq without any backoff. This
immediately reschedules the worker. Since no memory was freed, it seems it
will likely hit -ENOMEM again indefinitely.

[Severity: Critical]
Does re-arming irq_work here bypass teardown synchronization and lead to a
use-after-free of the bpf_arena structure?

During map destruction, arena_map_free() calls flush_work(&arena->free_work)
to wait for the worker to finish. However, flush_work() does not wait for
newly queued irq_work. 

If arena_map_free() proceeds to free the arena, the pending IPI might
execute arena_free_irq() and schedule_work(), dereferencing the freed
arena pointer.

> +
>  	/* Iterate the list again without holding spinlock to do the tlb flush and zap_pages */
> -	llist_for_each_safe(pos, t, list) {
> +	llist_for_each_safe(pos, t, cleared.first) {
>  		s = llist_entry(pos, struct arena_free_span, node);
>  		page_cnt = s->page_cnt;
>  		full_uaddr = clear_lo32(user_vm_start) + s->uaddr;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824134016.2006188-1-chenyuan_fl@163.com?part=3

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]

> diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
> index 2f28886f3ff7..15b588377a76 100644
> --- a/kernel/bpf/range_tree.c
> +++ b/kernel/bpf/range_tree.c
> @@ -143,16 +143,22 @@ 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);
>  			rn->rn_last = start - 1;
>  			range_it_insert(rn, rt);
>
> -			/* Add a range */
> -			new_rn = kmalloc_nolock(sizeof(struct range_node), __GFP_ACCOUNT,
> -						NUMA_NO_NODE);
> -			if (!new_rn)
> -				return -ENOMEM;
> +			/* Add right-half range */
>  			new_rn->rn_start = last + 1;
>  			new_rn->rn_last = old_last;
>  			range_it_insert(new_rn, rt);

This isn't a bug, but would it read better to keep the "Overlaps with
the entire clearing range" comment at the top of the branch, next to
the condition it describes, rather than having it appear in the middle
after the new allocation block?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32736729475

Re: [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1422 bytes --]

> bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
>
> range_tree_set() pre-allocates the node needed for a brand-new range
> before calling range_tree_clear(), so an allocation failure returns
> -ENOMEM without having modified the tree (previously the overlapping
> nodes were already removed by range_tree_clear() before the allocation
> was attempted, permanently losing the cleared sub-ranges).
>
> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>

This isn't a bug, but would it be worth adding a Fixes: tag for the commit
that introduced the clear-then-allocate ordering, so the stable backport
scope is clear? The introducing commit could not be determined from this
checkout.

Also, could the body be split into two sentences so the "previously ..."
rationale is not buried in a parenthetical? For example:

  range_tree_set() pre-allocates the node needed for a brand-new range
  before calling range_tree_clear(), so an allocation failure returns
  -ENOMEM without having modified the tree. Previously the overlapping
  nodes were already removed by range_tree_clear() before the allocation
  was attempted, permanently losing the cleared sub-ranges.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32736729475

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2114 bytes --]

> bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker
>
> 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.
>
> range_tree_set() is failure-atomic (it pre-allocates the node before
> touching the tree), so on -ENOMEM the range stays tracked as allocated
> and the pages remain mapped and accessible. A failed free is therefore
> retryable, and arena_map_free() reclaims any retained pages at map
> destruction; aborting the free avoids clearing PTEs for pages the arena
> free tree does not track.
>
> In arena_free_worker() a failed tree update used to leave the span in
> the drained list, where the second loop would still flush TLB entries,
> zap user VMAs, and free the span itself: the free request was dropped,
> user mappings were destroyed for a free that never happened, and the
> pages stayed mapped until map destruction. Keep failed spans on
> arena->free_spans instead and retry them on a later worker run; only
> spans whose PTE clearing actually ran are flushed, zapped, and released.
>
> Suggested-by: Emil Tsalapatis <emil@etsalapatis.com>
> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>

This looks like a bug fix for error handling in the arena code introduced
in an earlier commit. Should this include a Fixes: tag pointing to:

  Fixes: dc11a4dba246 ("bpf: Recover arena kernel faults with scratch page")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32736729475

Re: [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure

[Alexei Starovoitov]

On Mon, Aug 24, 2026 at 6:40 AM <chenyuan_fl@163.com> wrote:
>
> From: Yuan Chen <chenyuan@kylinos.cn>
>
> range_tree_set() pre-allocates the node needed for a brand-new range
> before calling range_tree_clear(), so an allocation failure returns
> -ENOMEM without having modified the tree (previously the overlapping
> nodes were already removed by range_tree_clear() before the allocation
> was attempted, permanently losing the cleared sub-ranges).
>
> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
> ---
>  kernel/bpf/range_tree.c | 45 +++++++++++++++++++++++++++++++----------
>  1 file changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
> index 15b588377a76..54055b1fe541 100644
> --- a/kernel/bpf/range_tree.c
> +++ b/kernel/bpf/range_tree.c
> @@ -199,6 +199,7 @@ int is_range_tree_set(struct range_tree *rt, u32 start, u32 len)
>  int range_tree_set(struct range_tree *rt, u32 start, u32 len)
>  {
>         u32 last = start + len - 1;
> +       struct range_node *new_rn = NULL;
>         struct range_node *right;
>         struct range_node *left;
>         int err;
> @@ -208,20 +209,40 @@ int range_tree_set(struct range_tree *rt, u32 start, u32 len)
>         if (left && left->rn_start <= start && left->rn_last >= last)
>                 return 0;
>
> +       /*
> +        * A new node is needed only when the range has no adjacent free
> +        * range on either side.  This is known before clearing: any range
> +        * covering start - 1 or last + 1 survives the clear as an adjacent
> +        * piece.

If this is true, why do a 2nd call to left = range_it_iter_first() ?


>  Allocate only in that case, before modifying the tree, so
> +        * a failure leaves the range tree unmodified
> +        */
> +       left = range_it_iter_first(rt, start - 1, start - 1);
> +       right = range_it_iter_first(rt, last + 1, last + 1);
> +       if (!left && !right) {
> +               new_rn = kmalloc_nolock(sizeof(struct range_node),
> +                                       __GFP_ACCOUNT, NUMA_NO_NODE);
> +               if (!new_rn)
> +                       return -ENOMEM;
> +       }
> +
>         /* Clear out everything in the range we want to set. */
>         err = range_tree_clear(rt, start, len);
>         if (err)
> -               return err;
> +               goto out_free_new;
>
>         /* Do we have a left-adjacent range ? */
>         left = range_it_iter_first(rt, start - 1, start - 1);

pw-bot: cr

Re:Re: [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure

[chenyuan]

The two lookups answer different questions, so the second one is
not redundant:

- The pre-clear lookup only consumes the boolean result.  Nodes are
  disjoint, and a node covering both start - 1 and last + 1 would
  fully cover [start, last], which is rejected by the early return
  above.  Hence range_tree_clear() can only remove or truncate nodes
  overlapping [start, last]: a node covering start - 1 either ends
  there (untouched) or straddles start and is truncated to
  [rn_start, start - 1].  Adjacency on either side is therefore
  invariant across the clear, and "no adjacent node on either side"
  before the clear is exactly the condition for the else-branch --
  the only case needing a fresh node.  It must be evaluated before
  any tree modification to keep the -ENOMEM path side-effect free.

- The post-clear lookup fetches the node handles used by the
  merge/extend branches.  The pre-clear handles cannot be reused:
  an adjacent node may straddle the range and get truncated (e.g.
  [start - 1, start + 3] becomes [start - 1, start - 1]), so both
  its bounds and its position in the tree change.  Re-looking it up
  keeps range_tree_set() independent of how range_tree_clear()
  implements truncation, and leaves the -EFAULT checks below as a
  sanity check of the clear itself.
 
The comment indeed fails to spell this out (and "adjacent free
range" is backwards); I'll reword it in v5.


At 2026-08-27 10:56:04, "Alexei Starovoitov" <alexei.starovoitov@gmail.com> wrote:
>On Mon, Aug 24, 2026 at 6:40 AM <chenyuan_fl@163.com> wrote:
>>
>> From: Yuan Chen <chenyuan@kylinos.cn>
>>
>> range_tree_set() pre-allocates the node needed for a brand-new range
>> before calling range_tree_clear(), so an allocation failure returns
>> -ENOMEM without having modified the tree (previously the overlapping
>> nodes were already removed by range_tree_clear() before the allocation
>> was attempted, permanently losing the cleared sub-ranges).
>>
>> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
>> ---
>>  kernel/bpf/range_tree.c | 45 +++++++++++++++++++++++++++++++----------
>>  1 file changed, 34 insertions(+), 11 deletions(-)
>>
>> diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
>> index 15b588377a76..54055b1fe541 100644
>> --- a/kernel/bpf/range_tree.c
>> +++ b/kernel/bpf/range_tree.c
>> @@ -199,6 +199,7 @@ int is_range_tree_set(struct range_tree *rt, u32 start, u32 len)
>>  int range_tree_set(struct range_tree *rt, u32 start, u32 len)
>>  {
>>         u32 last = start + len - 1;
>> +       struct range_node *new_rn = NULL;
>>         struct range_node *right;
>>         struct range_node *left;
>>         int err;
>> @@ -208,20 +209,40 @@ int range_tree_set(struct range_tree *rt, u32 start, u32 len)
>>         if (left && left->rn_start <= start && left->rn_last >= last)
>>                 return 0;
>>
>> +       /*
>> +        * A new node is needed only when the range has no adjacent free
>> +        * range on either side.  This is known before clearing: any range
>> +        * covering start - 1 or last + 1 survives the clear as an adjacent
>> +        * piece.
>
>If this is true, why do a 2nd call to left = range_it_iter_first() ?
>
>
>>  Allocate only in that case, before modifying the tree, so
>> +        * a failure leaves the range tree unmodified
>> +        */
>> +       left = range_it_iter_first(rt, start - 1, start - 1);
>> +       right = range_it_iter_first(rt, last + 1, last + 1);
>> +       if (!left && !right) {
>> +               new_rn = kmalloc_nolock(sizeof(struct range_node),
>> +                                       __GFP_ACCOUNT, NUMA_NO_NODE);
>> +               if (!new_rn)
>> +                       return -ENOMEM;
>> +       }
>> +
>>         /* Clear out everything in the range we want to set. */
>>         err = range_tree_clear(rt, start, len);
>>         if (err)
>> -               return err;
>> +               goto out_free_new;
>>
>>         /* Do we have a left-adjacent range ? */
>>         left = range_it_iter_first(rt, start - 1, start - 1);
>
>pw-bot: cr

[PATCH bpf-next v5 0/3] bpf, arena: fix range_tree consistency on allocation failure

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

The arena range tree can be left inconsistent when kmalloc_nolock()
fails mid-operation. Patch 1 fixes range_tree_clear(), patch 2 fixes
range_tree_set(), patch 3 makes the arena free paths handle
range_tree_set() failures, checks the return value in
arena_alloc_pages()'s partial-allocation error path, and makes
arena_map_free() drain the deferred-free machinery safely now that the
worker can requeue failed spans and queue irq_work on retry.

Changes in v5:
  - arena_map_free(): set a dying flag and steal orphaned spans before
    draining, and drain with flush_work() + irq_work_sync() +
    flush_work().  The worker retry queues arena->free_irq, which the
    old irq_work_sync() + flush_work() order could miss: the irq_work
    fired after the arena was freed and its callback scheduled
    free_work on freed memory.
  - arena_map_free(): retry the spinlock acquisition a bounded number
    of times (-EDEADLK is not retried) and WARN with the error code,
    instead of a bare WARN_ON_ONCE(1) and an immediate leak of the
    arena.
  - range_tree_set(): reword the comment describing the two lookups,
    as suggested by Alexei Starovoitov.  The pre-clear probe only
    decides whether a fresh node must be allocated, so that -ENOMEM
    leaves the tree unmodified; the post-clear lookup fetches the
    merge handles without depending on how range_tree_clear()
    truncates overlapping nodes.

Changes in v4:
  - arena_free_worker(): keep a span whose range_tree_set() failed on
    arena->free_spans and retry it on a later worker run, instead of
    leaving it in the drained list where the second loop would still
    zap user VMAs and free the span (dropping the free request), as
    pointed out by Emil Tsalapatis.

Changes in v3:
  - Check range_tree_set() return value in arena_alloc_pages()'s error
    path, which restores the unpopulated tail of a partially allocated
    range (previously ignored), as pointed out in review.

Changes in v2:
  - Fix multi-line comment style in patches 1 and 3 (opening /* on its
    own line), as pointed out in review.

Note: arena_vm_fault()'s two recovery paths (restoring the range to the
free tree after allocation/mapping failure) also call range_tree_set()
without checking the return value; that is addressed in a separate
series.

Yuan Chen (3):
  bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock
    failure
  bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  bpf, arena: handle range_tree_set failures in alloc/free paths

 kernel/bpf/arena.c      | 95 ++++++++++++++++++++++++++++++++++----
 kernel/bpf/range_tree.c | 61 +++++++++++++++++-------
 2 files changed, 132 insertions(+), 24 deletions(-)

-- 
2.54.0

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

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

range_tree_clear() pre-allocates the right-half node before modifying
the tree, so an allocation failure returns -ENOMEM without altering the
range tree.

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/range_tree.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
index 2f28886f3ff7..15b588377a76 100644
--- a/kernel/bpf/range_tree.c
+++ b/kernel/bpf/range_tree.c
@@ -143,16 +143,22 @@ 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);
 			rn->rn_last = start - 1;
 			range_it_insert(rn, rt);
 
-			/* Add a range */
-			new_rn = kmalloc_nolock(sizeof(struct range_node), __GFP_ACCOUNT,
-						NUMA_NO_NODE);
-			if (!new_rn)
-				return -ENOMEM;
+			/* Add right-half range */
 			new_rn->rn_start = last + 1;
 			new_rn->rn_last = old_last;
 			range_it_insert(new_rn, rt);
-- 
2.54.0

[PATCH bpf-next v5 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

range_tree_set() pre-allocates the node needed for a brand-new range
before calling range_tree_clear(), so an allocation failure returns
-ENOMEM without having modified the tree (previously the overlapping
nodes were already removed by range_tree_clear() before the allocation
was attempted, permanently losing the cleared sub-ranges).

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/range_tree.c | 45 +++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
index 15b588377a76..0420ab715f20 100644
--- a/kernel/bpf/range_tree.c
+++ b/kernel/bpf/range_tree.c
@@ -199,6 +199,7 @@ int is_range_tree_set(struct range_tree *rt, u32 start, u32 len)
 int range_tree_set(struct range_tree *rt, u32 start, u32 len)
 {
 	u32 last = start + len - 1;
+	struct range_node *new_rn = NULL;
 	struct range_node *right;
 	struct range_node *left;
 	int err;
@@ -208,20 +209,40 @@ int range_tree_set(struct range_tree *rt, u32 start, u32 len)
 	if (left && left->rn_start <= start && left->rn_last >= last)
 		return 0;
 
+	/*
+	 * The pre-clear probe only decides whether a fresh node is needed;
+	 * adjacency on either side is invariant across the clear, so it can
+	 * run before the tree is modified and -ENOMEM leaves it untouched.
+	 * The merge below re-fetches its handles after the clear instead of
+	 * relying on the overlapping nodes being updated in place.
+	 */
+	left = range_it_iter_first(rt, start - 1, start - 1);
+	right = range_it_iter_first(rt, last + 1, last + 1);
+	if (!left && !right) {
+		new_rn = kmalloc_nolock(sizeof(struct range_node),
+					__GFP_ACCOUNT, NUMA_NO_NODE);
+		if (!new_rn)
+			return -ENOMEM;
+	}
+
 	/* Clear out everything in the range we want to set. */
 	err = range_tree_clear(rt, start, len);
 	if (err)
-		return err;
+		goto out_free_new;
 
 	/* Do we have a left-adjacent range ? */
 	left = range_it_iter_first(rt, start - 1, start - 1);
-	if (left && left->rn_last + 1 != start)
-		return -EFAULT;
+	if (left && left->rn_last + 1 != start) {
+		err = -EFAULT;
+		goto out_free_new;
+	}
 
 	/* Do we have a right-adjacent range ? */
 	right = range_it_iter_first(rt, last + 1, last + 1);
-	if (right && right->rn_start != last + 1)
-		return -EFAULT;
+	if (right && right->rn_start != last + 1) {
+		err = -EFAULT;
+		goto out_free_new;
+	}
 
 	if (left && right) {
 		/* Combine left and right adjacent ranges */
@@ -241,14 +262,16 @@ int range_tree_set(struct range_tree *rt, u32 start, u32 len)
 		right->rn_start = start;
 		range_it_insert(right, rt);
 	} else {
-		left = kmalloc_nolock(sizeof(struct range_node), __GFP_ACCOUNT, NUMA_NO_NODE);
-		if (!left)
-			return -ENOMEM;
-		left->rn_start = start;
-		left->rn_last = last;
-		range_it_insert(left, rt);
+		/* No adjacent ranges; use the pre-allocated node */
+		new_rn->rn_start = start;
+		new_rn->rn_last = last;
+		range_it_insert(new_rn, rt);
 	}
 	return 0;
+
+out_free_new:
+	kfree_nolock(new_rn);
+	return err;
 }
 
 void range_tree_destroy(struct range_tree *rt)
-- 
2.54.0

[PATCH bpf-next v5 3/3] bpf, arena: handle range_tree_set failures in alloc/free paths

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

arena_alloc_pages(), 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.

range_tree_set() is failure-atomic (it pre-allocates the node before
touching the tree), so on -ENOMEM the range stays tracked as allocated
and the pages remain mapped and accessible. A failed free is therefore
retryable, and arena_map_free() reclaims any retained pages at map
destruction; aborting the free avoids clearing PTEs for pages the
arena free tree does not track.

In arena_free_worker() a failed tree update used to leave the span in
the drained list, where the second loop would still flush TLB entries,
zap user VMAs, and free the span itself: the free request was dropped,
user mappings were destroyed for a free that never happened, and the
pages stayed mapped until map destruction. Keep failed spans on
arena->free_spans instead and retry them on a later worker run; only
spans whose PTE clearing actually ran are flushed, zapped, and
released.

The retry queues arena->free_irq while the map can concurrently be
freed. arena_map_free() relied on irq_work_sync() + flush_work(),
which miss an irq_work queued by the running worker between the two
calls: the irq_work can fire after the arena is freed and its callback
schedules free_work on freed memory. Set arena->dying under the arena
spinlock before draining, so the worker stops requeueing, steal the
orphaned spans (their pages are reclaimed by existing_page_cb()), and
drain with flush_work() + irq_work_sync() + flush_work().

Setting @dying requires the arena spinlock. raw_res_spin_lock_irqsave()
can fail (-EDEADLK on a proven deadlock cycle, -ETIMEDOUT after a long
hold), and proceeding without the lock would race the worker. Retry a
bounded number of times for a long but finite hold and do not retry
-EDEADLK; on exhaustion leak the arena with a WARN carrying the error
code rather than hang map free.

Suggested-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/arena.c | 95 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 87 insertions(+), 8 deletions(-)

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 7b6847200b43..b0d1f0facfb2 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -5,6 +5,7 @@
 #include <linux/cacheflush.h>
 #include <linux/err.h>
 #include <linux/irq_work.h>
+#include <linux/delay.h>
 #include "linux/filter.h"
 #include <linux/llist.h>
 #include <linux/btf_ids.h>
@@ -67,6 +68,8 @@ struct bpf_arena {
 	struct irq_work     free_irq;
 	struct work_struct  free_work;
 	struct llist_head   free_spans;
+	/* set under spinlock during map free; stops the worker retry loop */
+	bool dying;
 };
 
 static void arena_free_worker(struct work_struct *work);
@@ -370,6 +373,9 @@ static int existing_page_cb(pte_t *ptep, unsigned long addr, void *data)
 static void arena_map_free(struct bpf_map *map)
 {
 	struct bpf_arena *arena = container_of(map, struct bpf_arena, map);
+	struct llist_node *list, *pos, *t;
+	unsigned long flags;
+	int ret, i;
 
 	/*
 	 * Check that user vma-s are not around when bpf map is freed.
@@ -380,7 +386,40 @@ static void arena_map_free(struct bpf_map *map)
 	if (WARN_ON_ONCE(!list_empty(&arena->vma_list)))
 		return;
 
-	/* Ensure no pending deferred frees */
+	/*
+	 * No fallback if this fails, so retry a few times for a long but
+	 * finite hold; -EDEADLK can't be waited out. Cap the retries:
+	 * leaking the arena is better than hanging map free.
+	 */
+	for (i = 0; i < 10; i++) {
+		ret = raw_res_spin_lock_irqsave(&arena->spinlock, flags);
+		if (!ret || ret == -EDEADLK)
+			break;
+		msleep(1);
+	}
+	if (ret) {
+		WARN_ONCE(1, "bpf_arena: spinlock acquire failed %d\n", ret);
+		return;
+	}
+	/*
+	 * Set @dying before draining: the worker checks it under this
+	 * spinlock before requeueing, so a failed span is either stolen
+	 * here or dropped by the worker.
+	 */
+	arena->dying = true;
+	list = llist_del_all(&arena->free_spans);
+	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
+
+	llist_for_each_safe(pos, t, list)
+		kfree_nolock(llist_entry(pos, struct arena_free_span, node));
+
+	/*
+	 * flush_work() lets the running worker observe @dying so it stops
+	 * requeueing; irq_work_sync() retires anything queued before that;
+	 * the final flush_work() runs the instance which the retired
+	 * irq_work's callback may have scheduled.
+	 */
+	flush_work(&arena->free_work);
 	irq_work_sync(&arena->free_irq);
 	flush_work(&arena->free_work);
 
@@ -766,7 +805,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: leak range %ld+%ld on failed 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 +922,20 @@ 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) {
+		/*
+		 * range_tree_set() is failure-atomic, so -ENOMEM leaves the
+		 * range allocated and the pages mapped; abort the free rather
+		 * than release pages the tree does not track. Nothing retries
+		 * the free; the program can free the range again.
+		 */
+		pr_warn_ratelimited("bpf_arena: free of %lx+%ld failed\n",
+				    uaddr, page_cnt);
+		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;
@@ -977,12 +1031,13 @@ static void arena_free_worker(struct work_struct *work)
 	struct llist_node *list, *pos, *t;
 	struct arena_free_span *s;
 	u64 arena_vm_start, user_vm_start;
-	struct llist_head free_pages;
+	struct llist_head free_pages, cleared;
 	struct clear_range_data cdata;
 	struct page *page;
 	unsigned long full_uaddr;
 	long kaddr, page_cnt, pgoff;
 	unsigned long flags;
+	bool retry = false;
 
 	if (raw_res_spin_lock_irqsave(&arena->spinlock, flags)) {
 		schedule_work(work);
@@ -992,28 +1047,52 @@ static void arena_free_worker(struct work_struct *work)
 	bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
 
 	init_llist_head(&free_pages);
+	init_llist_head(&cleared);
 	cdata.arena = arena;
 	cdata.free_pages = &free_pages;
 	arena_vm_start = bpf_arena_get_kern_vm_start(arena);
 	user_vm_start = bpf_arena_get_user_vm_start(arena);
 
 	list = llist_del_all(&arena->free_spans);
-	llist_for_each(pos, list) {
+	llist_for_each_safe(pos, t, list) {
 		s = llist_entry(pos, struct arena_free_span, node);
 		page_cnt = s->page_cnt;
 		kaddr = arena_vm_start + s->uaddr;
 		pgoff = compute_pgoff(arena, s->uaddr);
 
+		/*
+		 * Set the range free before clearing PTEs, and requeue the
+		 * span on failure: the PTEs stay intact and the free is
+		 * retried later. Only spans moved to @cleared (PTE clearing
+		 * actually ran) reach the flush/zap/release loop below.
+		 */
+		if (range_tree_set(&arena->rt, pgoff, page_cnt)) {
+			if (arena->dying) {
+				/*
+				 * The map is being freed. PTEs stay intact
+				 * and the pages are reclaimed by
+				 * arena_map_free() via existing_page_cb().
+				 */
+				kfree_nolock(s);
+				continue;
+			}
+			llist_add(&s->node, &arena->free_spans);
+			retry = true;
+			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);
+		llist_add(&s->node, &cleared);
 	}
 	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
 
+	if (retry)
+		irq_work_queue(&arena->free_irq);
+
 	/* Iterate the list again without holding spinlock to do the tlb flush and zap_pages */
-	llist_for_each_safe(pos, t, list) {
+	llist_for_each_safe(pos, t, cleared.first) {
 		s = llist_entry(pos, struct arena_free_span, node);
 		page_cnt = s->page_cnt;
 		full_uaddr = clear_lo32(user_vm_start) + s->uaddr;
-- 
2.54.0

Re: [PATCH bpf-next v5 3/3] bpf, arena: handle range_tree_set failures in alloc/free paths

[sashiko-bot]

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

New issues:
- [High] If `arena_map_free()` fails to acquire the lock and aborts, it leaks the arena and leaves the worker in an infinite retry loop on `-ENOMEM`.
- [Low] The `if (arena->dying)` check in `arena_free_worker` is mathematically dead code.

Pre-existing issues:
- [High] Virtual memory range is marked free before TLB flush and user PTE zap, allowing use-after-free and data loss.
--

commit 9b91373ca20b3a032ab090590fde24790048b705
Author: Yuan Chen <chenyuan@kylinos.cn>
bpf, arena: handle range_tree_set failures in alloc/free paths

This commit adds error handling for range_tree_set() in the arena allocation
and free paths. It includes a bounded retry loop for arena_map_free() to
acquire the spinlock and introduces arena->dying to stop the worker thread's
retry loop during map teardown.

> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 7b6847200b431..b0d1f0facfb2c 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
[ ... ]
> @@ -370,6 +373,9 @@ static int existing_page_cb(pte_t *ptep, unsigned long addr, void *data)
>  static void arena_map_free(struct bpf_map *map)
>  {
>  	struct bpf_arena *arena = container_of(map, struct bpf_arena, map);
> +	struct llist_node *list, *pos, *t;
> +	unsigned long flags;
> +	int ret, i;
>  
>  	/*
>  	 * Check that user vma-s are not around when bpf map is freed.
> @@ -380,7 +386,40 @@ static void arena_map_free(struct bpf_map *map)
>  	if (WARN_ON_ONCE(!list_empty(&arena->vma_list)))
>  		return;
>  
> -	/* Ensure no pending deferred frees */
> +	/*
> +	 * No fallback if this fails, so retry a few times for a long but
> +	 * finite hold; -EDEADLK can't be waited out. Cap the retries:
> +	 * leaking the arena is better than hanging map free.
> +	 */
> +	for (i = 0; i < 10; i++) {
> +		ret = raw_res_spin_lock_irqsave(&arena->spinlock, flags);
> +		if (!ret || ret == -EDEADLK)
> +			break;
> +		msleep(1);
> +	}
> +	if (ret) {
> +		WARN_ONCE(1, "bpf_arena: spinlock acquire failed %d\n", ret);
> +		return;
> +	}

[Severity: High]
If this early return path triggers, it intentionally leaks the arena, but
doesn't set arena->dying. Could this strand the worker in an infinite
retry loop?

If the worker's range_tree_set() call fails continuously (e.g., due to memory
exhaustion), it will endlessly requeue the span and queue an irq_work to
retry. Because arena->dying remains false, this could turn into an infinite
busy loop.

> +	/*
> +	 * Set @dying before draining: the worker checks it under this
> +	 * spinlock before requeueing, so a failed span is either stolen
> +	 * here or dropped by the worker.
> +	 */
> +	arena->dying = true;
[ ... ]
> @@ -977,12 +1031,13 @@ static void arena_free_worker(struct work_struct *work)
>  	struct llist_node *list, *pos, *t;
>  	struct arena_free_span *s;
>  	u64 arena_vm_start, user_vm_start;
> -	struct llist_head free_pages;
> +	struct llist_head free_pages, cleared;
>  	struct clear_range_data cdata;
>  	struct page *page;
>  	unsigned long full_uaddr;
>  	long kaddr, page_cnt, pgoff;
>  	unsigned long flags;
> +	bool retry = false;
>  
>  	if (raw_res_spin_lock_irqsave(&arena->spinlock, flags)) {
>  		schedule_work(work);
> @@ -992,28 +1047,52 @@ static void arena_free_worker(struct work_struct *work)
>  	bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
>  
>  	init_llist_head(&free_pages);
> +	init_llist_head(&cleared);
>  	cdata.arena = arena;
>  	cdata.free_pages = &free_pages;
>  	arena_vm_start = bpf_arena_get_kern_vm_start(arena);
>  	user_vm_start = bpf_arena_get_user_vm_start(arena);
>  
>  	list = llist_del_all(&arena->free_spans);
> -	llist_for_each(pos, list) {
> +	llist_for_each_safe(pos, t, list) {
>  		s = llist_entry(pos, struct arena_free_span, node);
>  		page_cnt = s->page_cnt;
>  		kaddr = arena_vm_start + s->uaddr;
>  		pgoff = compute_pgoff(arena, s->uaddr);
>  
> +		/*
> +		 * Set the range free before clearing PTEs, and requeue the
> +		 * span on failure: the PTEs stay intact and the free is
> +		 * retried later. Only spans moved to @cleared (PTE clearing
> +		 * actually ran) reach the flush/zap/release loop below.
> +		 */
> +		if (range_tree_set(&arena->rt, pgoff, page_cnt)) {
> +			if (arena->dying) {
> +				/*
> +				 * The map is being freed. PTEs stay intact
> +				 * and the pages are reclaimed by
> +				 * arena_map_free() via existing_page_cb().
> +				 */
> +				kfree_nolock(s);
> +				continue;
> +			}

[Severity: Low]
Is this check mathematically dead code?

arena->dying is set to true by arena_map_free() while holding arena->spinlock,
after which arena->free_spans is immediately drained via llist_del_all().

In arena_free_worker(), the list is also drained at the very beginning of its
arena->spinlock critical section. Because both functions drain the list under
the same spinlock, if arena_map_free() runs first, the worker's list is empty
and the loop never executes. If the worker runs first, dying is guaranteed to
be false throughout its critical section.

> +			llist_add(&s->node, &arena->free_spans);
> +			retry = true;
> +			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);
> +		llist_add(&s->node, &cleared);
>  	}
>  	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
>  
> +	if (retry)
> +		irq_work_queue(&arena->free_irq);
> +
>  	/* Iterate the list again without holding spinlock to do the tlb flush and zap_pages */
> -	llist_for_each_safe(pos, t, list) {
> +	llist_for_each_safe(pos, t, cleared.first) {
>  		s = llist_entry(pos, struct arena_free_span, node);
>  		page_cnt = s->page_cnt;
>  		full_uaddr = clear_lo32(user_vm_start) + s->uaddr;
> +		kaddr = arena_vm_start + s->uaddr;
> +
> +		/* ensure no stale TLB entries */
> +		flush_tlb_kernel_range(kaddr, kaddr + (page_cnt * PAGE_SIZE));

[Severity: High]
This isn't a bug introduced by this patch, but dropping the spinlock before
the TLB flush opens a race window. Can a concurrent arena_alloc_pages()
allocate the newly freed virtual range and map new physical pages to it?

Since the TLB flush for the old pages has not yet executed, accesses by the
new owner could hit stale TLB entries (or user PTEs). Writes would go to the
old physical pages which are later freed, resulting in a use-after-free and
data loss. The synchronous free path in arena_free_pages() appears to have
the same preexisting vulnerability.

>  
> -		/* remove pages from user vmas */
> -		zap_pages(arena, full_uaddr, page_cnt);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902093740.2338724-1-chenyuan_fl@163.com?part=3