mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH bpf-next v4 0/3] bpf, arena: fix range_tree consistency on allocation failure
@ 2026-08-24 13:40 chenyuan_fl
  2026-08-24 13:40 ` [PATCH bpf-next v4 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: chenyuan_fl @ 2026-08-24 13:40 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v4 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure
  2026-08-24 13:40 [PATCH bpf-next v4 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
@ 2026-08-24 13:40 ` chenyuan_fl
  2026-08-24 14:35   ` bot+bpf-ci
  2026-08-24 13:40 ` [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
  2026-08-24 13:40 ` [PATCH bpf-next v4 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker chenyuan_fl
  2 siblings, 1 reply; 26+ messages in thread
From: chenyuan_fl @ 2026-08-24 13:40 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  2026-08-24 13:40 [PATCH bpf-next v4 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
  2026-08-24 13:40 ` [PATCH bpf-next v4 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
@ 2026-08-24 13:40 ` chenyuan_fl
  2026-08-24 14:35   ` bot+bpf-ci
  2026-08-27  2:56   ` Alexei Starovoitov
  2026-08-24 13:40 ` [PATCH bpf-next v4 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker chenyuan_fl
  2 siblings, 2 replies; 26+ messages in thread
From: chenyuan_fl @ 2026-08-24 13:40 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v4 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker
  2026-08-24 13:40 [PATCH bpf-next v4 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
  2026-08-24 13:40 ` [PATCH bpf-next v4 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
  2026-08-24 13:40 ` [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
@ 2026-08-24 13:40 ` chenyuan_fl
  2026-08-24 14:35   ` bot+bpf-ci
  2 siblings, 1 reply; 26+ messages in thread
From: chenyuan_fl @ 2026-08-24 13:40 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v4 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure
  2026-08-24 13:40 ` [PATCH bpf-next v4 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
@ 2026-08-24 14:35   ` bot+bpf-ci
  0 siblings, 0 replies; 26+ messages in thread
From: bot+bpf-ci @ 2026-08-24 14:35 UTC (permalink / raw)
  To: chenyuan_fl, bpf
  Cc: linux-kernel, ast, daniel, andrii, eddyz87, memxor, martin.lau,
	song, yonghong.song, jolsa, emil, ihor.solodrai, chenyuan, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
	ihor.solodrai

[-- 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  2026-08-24 13:40 ` [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
@ 2026-08-24 14:35   ` bot+bpf-ci
  2026-08-27  2:56   ` Alexei Starovoitov
  1 sibling, 0 replies; 26+ messages in thread
From: bot+bpf-ci @ 2026-08-24 14:35 UTC (permalink / raw)
  To: chenyuan_fl, bpf
  Cc: linux-kernel, ast, daniel, andrii, eddyz87, memxor, martin.lau,
	song, yonghong.song, jolsa, emil, ihor.solodrai, chenyuan, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
	ihor.solodrai

[-- 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v4 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker
  2026-08-24 13:40 ` [PATCH bpf-next v4 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker chenyuan_fl
@ 2026-08-24 14:35   ` bot+bpf-ci
  0 siblings, 0 replies; 26+ messages in thread
From: bot+bpf-ci @ 2026-08-24 14:35 UTC (permalink / raw)
  To: chenyuan_fl, bpf
  Cc: linux-kernel, ast, daniel, andrii, eddyz87, memxor, martin.lau,
	song, yonghong.song, jolsa, emil, ihor.solodrai, chenyuan, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
	ihor.solodrai

[-- 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  2026-08-24 13:40 ` [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
  2026-08-24 14:35   ` bot+bpf-ci
@ 2026-08-27  2:56   ` Alexei Starovoitov
  2026-09-01  7:01     ` chenyuan
  2026-09-02  9:37     ` [PATCH bpf-next v5 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
  1 sibling, 2 replies; 26+ messages in thread
From: Alexei Starovoitov @ 2026-08-27  2:56 UTC (permalink / raw)
  To: Yuan Chen
  Cc: bpf, LKML, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, Yuan Chen

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re:Re: [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  2026-08-27  2:56   ` Alexei Starovoitov
@ 2026-09-01  7:01     ` chenyuan
  2026-09-02  9:37     ` [PATCH bpf-next v5 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
  1 sibling, 0 replies; 26+ messages in thread
From: chenyuan @ 2026-09-01  7:01 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: bpf, LKML, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, Yuan Chen


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v5 0/3] bpf, arena: fix range_tree consistency on allocation failure
  2026-08-27  2:56   ` Alexei Starovoitov
  2026-09-01  7:01     ` chenyuan
@ 2026-09-02  9:37     ` chenyuan_fl
  2026-09-02  9:37       ` [PATCH bpf-next v5 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
                         ` (2 more replies)
  1 sibling, 3 replies; 26+ messages in thread
From: chenyuan_fl @ 2026-09-02  9:37 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
	Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v5 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure
  2026-09-02  9:37     ` [PATCH bpf-next v5 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
@ 2026-09-02  9:37       ` chenyuan_fl
  2026-09-02  9:37       ` [PATCH bpf-next v5 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
  2026-09-02  9:37       ` [PATCH bpf-next v5 3/3] bpf, arena: handle range_tree_set failures in alloc/free paths chenyuan_fl
  2 siblings, 0 replies; 26+ messages in thread
From: chenyuan_fl @ 2026-09-02  9:37 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
	Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v5 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  2026-09-02  9:37     ` [PATCH bpf-next v5 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
  2026-09-02  9:37       ` [PATCH bpf-next v5 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
@ 2026-09-02  9:37       ` chenyuan_fl
  2026-09-02  9:37       ` [PATCH bpf-next v5 3/3] bpf, arena: handle range_tree_set failures in alloc/free paths chenyuan_fl
  2 siblings, 0 replies; 26+ messages in thread
From: chenyuan_fl @ 2026-09-02  9:37 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
	Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v5 3/3] bpf, arena: handle range_tree_set failures in alloc/free paths
  2026-09-02  9:37     ` [PATCH bpf-next v5 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
  2026-09-02  9:37       ` [PATCH bpf-next v5 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
  2026-09-02  9:37       ` [PATCH bpf-next v5 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
@ 2026-09-02  9:37       ` chenyuan_fl
  2026-09-08 15:53         ` Emil Tsalapatis
  2 siblings, 1 reply; 26+ messages in thread
From: chenyuan_fl @ 2026-09-02  9:37 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
	Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v5 3/3] bpf, arena: handle range_tree_set failures in alloc/free paths
  2026-09-02  9:37       ` [PATCH bpf-next v5 3/3] bpf, arena: handle range_tree_set failures in alloc/free paths chenyuan_fl
@ 2026-09-08 15:53         ` Emil Tsalapatis
  2026-09-22  6:58           ` [PATCH bpf-next v6 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
                             ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: Emil Tsalapatis @ 2026-09-08 15:53 UTC (permalink / raw)
  To: chenyuan_fl
  Cc: bpf, linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Ihor Solodrai, Yuan Chen

On Wed, Sep 2, 2026 at 5:38 AM <chenyuan_fl@163.com> wrote:
>
> 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);
> +       }

A hardcoded msleep is definitely not the way to go.

> +       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);

The extra IRQs are also not a great idea, considering the range tree
failures are
due to memory shortage. There is no perfectly clean way to deal with it, but imo
the way is to leak the memory instead of trying to recover, so the patch should
focus on leaving the range tree and page tables consistent.

Since this patch has drifted quite a bit since the initial revisions,
can you also
remove the Suggested-by tag from me?

pw-bot: cr

>         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
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v6 0/3] bpf, arena: fix range_tree consistency on allocation failure
  2026-09-08 15:53         ` Emil Tsalapatis
@ 2026-09-22  6:58           ` chenyuan_fl
  2026-09-22  7:20           ` [PATCH bpf-next v6 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
                             ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: chenyuan_fl @ 2026-09-22  6:58 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Ihor Solodrai, Yuan Chen

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(), and patch 3 makes the arena paths handle
range_tree_set() failures and checks the return value in
arena_alloc_pages()'s partial-allocation error path.

Thanks for the review.

Changes in v6:
  - Patch 3 only: on a failed range_tree_set() drop the span and leak the
    range, which keeps the range tree and the page tables consistent, and
    revert the arena_map_free() teardown changes that went with the
    retry. Patches 1 and 2 are unchanged from v5.

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: check range_tree_set return in arena_free_pages and
    arena_free_worker

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

-- 
2.54.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v6 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure
  2026-09-08 15:53         ` Emil Tsalapatis
  2026-09-22  6:58           ` [PATCH bpf-next v6 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
@ 2026-09-22  7:20           ` chenyuan_fl
  2026-09-22  7:21           ` [PATCH bpf-next v6 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
  2026-09-22  7:21           ` [PATCH bpf-next v6 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker chenyuan_fl
  3 siblings, 0 replies; 26+ messages in thread
From: chenyuan_fl @ 2026-09-22  7:20 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Ihor Solodrai, Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v6 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  2026-09-08 15:53         ` Emil Tsalapatis
  2026-09-22  6:58           ` [PATCH bpf-next v6 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
  2026-09-22  7:20           ` [PATCH bpf-next v6 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
@ 2026-09-22  7:21           ` chenyuan_fl
  2026-09-22  8:24             ` bot+bpf-ci
  2026-09-23  2:02             ` Alexei Starovoitov
  2026-09-22  7:21           ` [PATCH bpf-next v6 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker chenyuan_fl
  3 siblings, 2 replies; 26+ messages in thread
From: chenyuan_fl @ 2026-09-22  7:21 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Ihor Solodrai, Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v6 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker
  2026-09-08 15:53         ` Emil Tsalapatis
                             ` (2 preceding siblings ...)
  2026-09-22  7:21           ` [PATCH bpf-next v6 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
@ 2026-09-22  7:21           ` chenyuan_fl
  2026-09-23  2:02             ` Alexei Starovoitov
  3 siblings, 1 reply; 26+ messages in thread
From: chenyuan_fl @ 2026-09-22  7:21 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Ihor Solodrai, Yuan Chen

From: Yuan Chen <chenyuan@kylinos.cn>

range_tree_set() can fail with -ENOMEM, but the arena callers ignored its
return value. In arena_free_worker() it ran after PTE clearing, so a
failed update left the range marked allocated while its pages were
already unmapped and freed.

Check the return value at all three call sites: arena_alloc_pages() warns
if restoring the unpopulated tail of a partial allocation fails,
arena_free_pages() aborts the free, and arena_free_worker() moves
range_tree_set() before PTE clearing and, on failure, drops the span and
leaks the range, reclaimed later by arena_map_free().

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/arena.c | 38 +++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 7b6847200b43..c9f81d08582b 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: 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 +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: on -ENOMEM the range
+		 * stays allocated and its pages mapped. Abort the free
+		 * instead of unmapping pages the tree does not track; the
+		 * program can free the range again later.
+		 */
+		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,7 +990,7 @@ 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;
@@ -992,28 +1005,39 @@ 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);
 
+		/*
+		 * Mark the range free before clearing PTEs so a failed
+		 * update keeps the tree and the PTEs consistent. On failure
+		 * leak the range, reclaimed later by arena_map_free(); only
+		 * spans on @cleared reach the flush/zap/release loop below.
+		 */
+		if (range_tree_set(&arena->rt, pgoff, page_cnt)) {
+			kfree_nolock(s);
+			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);
 
 	/* 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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v6 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  2026-09-22  7:21           ` [PATCH bpf-next v6 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
@ 2026-09-22  8:24             ` bot+bpf-ci
  2026-09-23  2:02             ` Alexei Starovoitov
  1 sibling, 0 replies; 26+ messages in thread
From: bot+bpf-ci @ 2026-09-22  8:24 UTC (permalink / raw)
  To: chenyuan_fl, bpf
  Cc: linux-kernel, alexei.starovoitov, ast, daniel, andrii, eddyz87,
	memxor, martin.lau, song, yonghong.song, jolsa, ihor.solodrai,
	chenyuan, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, mason, ihor.solodrai

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

> diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
> index 15b588377a769..0420ab715f206 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.
> +	 */

This isn't a bug, but would it read closer to the rest of the file if
this block were trimmed to the one non-obvious point - that a node
covering start-1 or last+1 cannot disappear across the clear, so the
probe is safe to do first?

Looking at other comments in this file, they're all single terse lines
like "/* Clear out everything in the range we want to set. */" or
"/* Do we have a left-adjacent range ? */", and the middle two
sentences here describe what the code already shows (the two
range_it_iter_first() calls below and the "/* No adjacent ranges; use
the pre-allocated node */" comment later).

> +	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. */

[ ... ]


---
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/35700555218

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v6 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  2026-09-22  7:21           ` [PATCH bpf-next v6 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
  2026-09-22  8:24             ` bot+bpf-ci
@ 2026-09-23  2:02             ` Alexei Starovoitov
  1 sibling, 0 replies; 26+ messages in thread
From: Alexei Starovoitov @ 2026-09-23  2:02 UTC (permalink / raw)
  To: chenyuan_fl, bpf
  Cc: linux-kernel, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Ihor Solodrai, Yuan Chen

On Tue, Sep 22, 2026 at 03:21 PM chenyuan_fl@163.com <chenyuan_fl@163.com> wrote:
> +	/*
> +	 * 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);
[...]
>  	/* Do we have a left-adjacent range ? */
>  	left = range_it_iter_first(rt, start - 1, start - 1);

In v4 I asked why the 2nd lookup is needed. It's not.
range_tree_clear() frees only the nodes that are entirely inside
[start, last]. A node that covers start - 1 or last + 1 is trimmed
in place. The split case cannot happen here because of the
'already set' check above. So left and right found before the clear
are the same nodes that the 2nd lookup returns.
Do the lookup once and drop the comment.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v6 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker
  2026-09-22  7:21           ` [PATCH bpf-next v6 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker chenyuan_fl
@ 2026-09-23  2:02             ` Alexei Starovoitov
  2026-09-23  8:58               ` [PATCH bpf-next v7 0/2] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
  0 siblings, 1 reply; 26+ messages in thread
From: Alexei Starovoitov @ 2026-09-23  2:02 UTC (permalink / raw)
  To: chenyuan_fl, bpf
  Cc: linux-kernel, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Ihor Solodrai, Yuan Chen

On Tue, Sep 22, 2026 at 03:21 PM chenyuan_fl@163.com <chenyuan_fl@163.com> wrote:
> range_tree_set() can fail with -ENOMEM, but the arena callers ignored its
> return value. In arena_free_worker() it ran after PTE clearing, so a
> failed update left the range marked allocated while its pages were
> already unmapped and freed.

That's not a bug. The range is allocated in the range tree and has
no pages. bpf_arena_reserve_pages() creates the same state.
Nothing relies on an allocated range having pages.
The cost is page_cnt pages of address space.

[...]
> +	ret = range_tree_set(&arena->rt, pgoff, page_cnt);
> +	if (ret) {
> +		/*
> +		 * range_tree_set() is failure-atomic: on -ENOMEM the range
> +		 * stays allocated and its pages mapped. Abort the free
> +		 * instead of unmapping pages the tree does not track; the
> +		 * program can free the range again later.
> +		 */
> +		raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
> +		bpf_map_memcg_exit(old_memcg, new_memcg);
> +		return;
> +	}

No. This makes it worse.
range_tree_set() fails when the system is out of memory. Today the
pages are still unmapped and freed and only the address range is lost.
With this patch the pages stay mapped until map free, so the prog
leaks memory instead of address space.
bpf_arena_free_pages() returns void. The prog cannot know that it
has to free the range again.
arena_alloc_pages() calls arena_free_pages() to undo a partial
allocation. Now it can return NULL and keep those pages mapped.
Same for arena_free_worker().

Drop this patch. Pls respin 1 and 2 only.
No need for a separate series for arena_vm_fault() either.

pw-bot: cr

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v7 0/2] bpf, arena: fix range_tree consistency on allocation failure
  2026-09-23  2:02             ` Alexei Starovoitov
@ 2026-09-23  8:58               ` chenyuan_fl
  2026-09-23  8:58                 ` [PATCH bpf-next v7 1/2] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
  2026-09-23  8:58                 ` [PATCH bpf-next v7 2/2] bpf, arena: fix range_tree_set " chenyuan_fl
  0 siblings, 2 replies; 26+ messages in thread
From: chenyuan_fl @ 2026-09-23  8:58 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Ihor Solodrai, Yuan Chen

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

Thanks for the review.

Changes in v7:
  - Dropped the third patch (checking range_tree_set() return in
    arena_free_pages/arena_free_worker) per Alexei Starovoitov: a range
    that stays marked allocated while its pages were freed is the same
    state bpf_arena_reserve_pages() creates, and nothing relies on an
    allocated range having pages. The failed free only costs page_cnt
    pages of address space, while aborting the free would leak pages
    the prog cannot observe or retry. No separate series for
    arena_vm_fault() either.
  - range_tree_set(): do the adjacency lookup once, before the clear,
    and drop the unreachable -EFAULT checks, per Alexei Starovoitov.

Changes in v6:
  - Patch 3 only: on a failed range_tree_set() drop the span and leak the
    range, which keeps the range tree and the page tables consistent, and
    revert the arena_map_free() teardown changes that went with the
    retry. Patches 1 and 2 are unchanged from v5.

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.

Yuan Chen (2):
  bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock
    failure
  bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure

 kernel/bpf/range_tree.c | 52 ++++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v7 1/2] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure
  2026-09-23  8:58               ` [PATCH bpf-next v7 0/2] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
@ 2026-09-23  8:58                 ` chenyuan_fl
  2026-09-23  9:34                   ` bot+bpf-ci
  2026-09-23  8:58                 ` [PATCH bpf-next v7 2/2] bpf, arena: fix range_tree_set " chenyuan_fl
  1 sibling, 1 reply; 26+ messages in thread
From: chenyuan_fl @ 2026-09-23  8:58 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Ihor Solodrai, Yuan Chen

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v7 2/2] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  2026-09-23  8:58               ` [PATCH bpf-next v7 0/2] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
  2026-09-23  8:58                 ` [PATCH bpf-next v7 1/2] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
@ 2026-09-23  8:58                 ` chenyuan_fl
  2026-09-23  9:34                   ` bot+bpf-ci
  1 sibling, 1 reply; 26+ messages in thread
From: chenyuan_fl @ 2026-09-23  8:58 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, Alexei Starovoitov, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Ihor Solodrai, Yuan Chen

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).

The adjacency lookups done before the clear already return the nodes
that the merge below needs: range_tree_clear() frees only the nodes
that lie entirely inside [start, last], trims the overlapping ones in
place, and the split case cannot happen because of the 'already set'
check above. Do the lookup once and drop the unreachable -EFAULT
checks.

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

diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
index 15b588377a76..2460e78d6a55 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,19 @@ int range_tree_set(struct range_tree *rt, u32 start, u32 len)
 	if (left && left->rn_start <= start && left->rn_last >= last)
 		return 0;
 
+	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;
-
-	/* 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;
-
-	/* 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;
+		goto out_free_new;
 
 	if (left && right) {
 		/* Combine left and right adjacent ranges */
@@ -241,14 +241,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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v7 1/2] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure
  2026-09-23  8:58                 ` [PATCH bpf-next v7 1/2] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
@ 2026-09-23  9:34                   ` bot+bpf-ci
  0 siblings, 0 replies; 26+ messages in thread
From: bot+bpf-ci @ 2026-09-23  9:34 UTC (permalink / raw)
  To: chenyuan_fl, bpf
  Cc: linux-kernel, alexei.starovoitov, ast, daniel, andrii, eddyz87,
	memxor, martin.lau, song, yonghong.song, jolsa, ihor.solodrai,
	chenyuan, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, mason, ihor.solodrai

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

> bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure
>
> 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>

This appears to fix a bug where kmalloc_nolock() failure in the split
branch left the node already shrunk and reinserted, permanently losing
the [last+1, old_last] sub-range. Should this include a Fixes: tag?

The suggested tag would be:

  Fixes: f8c67d8550ee ("bpf: Use kmalloc_nolock() in range tree")


---
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/35841441642

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v7 2/2] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure
  2026-09-23  8:58                 ` [PATCH bpf-next v7 2/2] bpf, arena: fix range_tree_set " chenyuan_fl
@ 2026-09-23  9:34                   ` bot+bpf-ci
  0 siblings, 0 replies; 26+ messages in thread
From: bot+bpf-ci @ 2026-09-23  9:34 UTC (permalink / raw)
  To: chenyuan_fl, bpf
  Cc: linux-kernel, alexei.starovoitov, ast, daniel, andrii, eddyz87,
	memxor, martin.lau, song, yonghong.song, jolsa, ihor.solodrai,
	chenyuan, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 4089 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).
>
> The adjacency lookups done before the clear already return the nodes
> that the merge below needs: range_tree_clear() frees only the nodes
> that lie entirely inside [start, last], trims the overlapping ones in
> place, and the split case cannot happen because of the 'already set'
> check above. Do the lookup once and drop the unreachable -EFAULT
> checks.
>
> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>

This looks like a bug fix for the clear-before-allocate ordering in
range_tree_set(). Should this include:

  Fixes: b795379757eb ("bpf: Introduce range_tree data structure and use it in bpf arena")

> diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
> index 15b588377a76..2460e78d6a55 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,19 @@ int range_tree_set(struct range_tree *rt, u32 start, u32 len)
>  	if (left && left->rn_start <= start && left->rn_last >= last)
>  		return 0;
>
> +	left = range_it_iter_first(rt, start - 1, start - 1);
> +	right = range_it_iter_first(rt, last + 1, last + 1);

This isn't a bug, but could a short comment above these lookups record
why they are still valid after range_tree_clear() runs, the way the
changelog explains it? The correctness now rests on a non-obvious
argument: range_tree_clear() only trims a node overlapping start-1 or
last+1 in place (never frees it), so these pointers and their endpoints
are still the ones the merge branches below need. Every other decision
point in this function carries a one-line comment, so the two bare
lookups are the odd ones out, and the rationale currently lives only in
the changelog.

> +	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;

This isn't a bug, but since the changelog argues range_tree_clear()
cannot fail here (the split path is unreachable because of the 'already
set' check), is the new out_free_new path kept deliberately as
future-proofing, or could it go the same way as the -EFAULT checks? The
asymmetry is what reads oddly: the patch removes two unreachable checks
precisely for being unreachable, while keeping error propagation from an
int-returning helper that the changelog says cannot fail.

> +		goto out_free_new;
>
[ ... ]

> @@ -241,14 +241,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)


---
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/35841441642

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2026-09-23  9:34 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-24 13:40 [PATCH bpf-next v4 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
2026-08-24 13:40 ` [PATCH bpf-next v4 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
2026-08-24 14:35   ` bot+bpf-ci
2026-08-24 13:40 ` [PATCH bpf-next v4 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
2026-08-24 14:35   ` bot+bpf-ci
2026-08-27  2:56   ` Alexei Starovoitov
2026-09-01  7:01     ` chenyuan
2026-09-02  9:37     ` [PATCH bpf-next v5 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
2026-09-02  9:37       ` [PATCH bpf-next v5 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
2026-09-02  9:37       ` [PATCH bpf-next v5 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
2026-09-02  9:37       ` [PATCH bpf-next v5 3/3] bpf, arena: handle range_tree_set failures in alloc/free paths chenyuan_fl
2026-09-08 15:53         ` Emil Tsalapatis
2026-09-22  6:58           ` [PATCH bpf-next v6 0/3] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
2026-09-22  7:20           ` [PATCH bpf-next v6 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
2026-09-22  7:21           ` [PATCH bpf-next v6 2/3] bpf, arena: fix range_tree_set " chenyuan_fl
2026-09-22  8:24             ` bot+bpf-ci
2026-09-23  2:02             ` Alexei Starovoitov
2026-09-22  7:21           ` [PATCH bpf-next v6 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker chenyuan_fl
2026-09-23  2:02             ` Alexei Starovoitov
2026-09-23  8:58               ` [PATCH bpf-next v7 0/2] bpf, arena: fix range_tree consistency on allocation failure chenyuan_fl
2026-09-23  8:58                 ` [PATCH bpf-next v7 1/2] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure chenyuan_fl
2026-09-23  9:34                   ` bot+bpf-ci
2026-09-23  8:58                 ` [PATCH bpf-next v7 2/2] bpf, arena: fix range_tree_set " chenyuan_fl
2026-09-23  9:34                   ` bot+bpf-ci
2026-08-24 13:40 ` [PATCH bpf-next v4 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker chenyuan_fl
2026-08-24 14:35   ` bot+bpf-ci

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®