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

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

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

The arena range tree can be left inconsistent when kmalloc_nolock()
fails mid-operation. Patch 1 fixes range_tree_clear(), patch 2 fixes
range_tree_set(), patch 3 makes arena_free_pages() and
arena_free_worker() handle range_tree_set() failures.

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 (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      | 18 ++++++++++--
 kernel/bpf/range_tree.c | 61 ++++++++++++++++++++++++++++++-----------
 2 files changed, 60 insertions(+), 19 deletions(-)

-- 
2.54.0

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

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

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

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

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

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

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

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

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

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

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

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

arena_free_pages() and arena_free_worker() now handle range_tree_set()
errors. arena_free_pages() aborts the free on error, and
arena_free_worker() moves range_tree_set() before PTE clearing so that a
failed tree update leaves the PTEs intact instead of freeing pages that
the arena free tree does not track.

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

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 555ee2531ef9..b374ebf2a4de 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -881,7 +881,12 @@ static void arena_free_pages(struct bpf_arena *arena, long uaddr, long page_cnt,
 	if (ret)
 		goto defer;
 
-	range_tree_set(&arena->rt, pgoff, page_cnt);
+	ret = range_tree_set(&arena->rt, pgoff, page_cnt);
+	if (ret) {
+		raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
+		bpf_map_memcg_exit(old_memcg, new_memcg);
+		return;
+	}
 
 	init_llist_head(&free_pages);
 	cdata.arena = arena;
@@ -1004,11 +1009,18 @@ static void arena_free_worker(struct work_struct *work)
 		kaddr = arena_vm_start + s->uaddr;
 		pgoff = compute_pgoff(arena, s->uaddr);
 
+		/*
+		 * Mark the range as free before clearing PTEs so that
+		 * if the range tree update fails we leave the PTEs
+		 * intact, avoiding an unrecoverable state where pages
+		 * are freed but the arena free tree does not track them.
+		 */
+		if (range_tree_set(&arena->rt, pgoff, page_cnt))
+			continue;
+
 		/* clear ptes and collect pages in free_pages llist */
 		apply_to_existing_page_range(&init_mm, kaddr, page_cnt << PAGE_SHIFT,
 					     apply_range_clear_cb, &cdata);
-
-		range_tree_set(&arena->rt, pgoff, page_cnt);
 	}
 	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
 
-- 
2.54.0

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

[sashiko-bot]

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

Pre-existing issues:
- [High] `range_tree_set()` leaves the interval tree in a corrupted state if node allocation fails.
- [Medium] Ignored return value from `range_tree_set()` leads to permanent virtual space leak in the BPF arena on OOM.
--

--- Patch [1]: [PATCH bpf-next v2 1/3] bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure ---
commit d0c75ee613a9a640e30aac20ab84b5820201a99d
Author: Yuan Chen <chenyuan@kylinos.cn>

bpf, arena: fix range_tree_clear inconsistency on kmalloc_nolock failure

This commit modifies range_tree_clear() to pre-allocate the right-half node
before modifying the tree. This ensures that an allocation failure returns
-ENOMEM without leaving the range tree in an altered state.

> diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
> index 2f28886f3ff75..15b588377a769 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;
> +

[Severity: High]
This is a pre-existing issue, but does range_tree_set() suffer from a
similar state corruption if its node allocation fails?

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

If range_tree_set() fails to allocate 'left' due to OOM, it returns -ENOMEM
after range_tree_clear() has already destructively removed sub-ranges.
Should range_tree_set() also pre-allocate its nodes before modifying the
tree to prevent state corruption?


[Severity: Medium]
This is also a pre-existing issue, but are the return values of
range_tree_set() intentionally ignored in the arena code?

In kernel/bpf/arena.c:arena_alloc_pages():
...
        ret = bpf_map_alloc_pages(&arena->map, node_id, this_batch, pages);
        if (ret)
            goto out;
    ...
    out:
        range_tree_set(&arena->rt, pgoff + mapped, page_cnt - mapped);

And similarly in kernel/bpf/arena.c:arena_free_pages():
    ...
        range_tree_set(&arena->rt, pgoff, page_cnt);

If range_tree_set() encounters an allocation failure and returns -ENOMEM,
the failure is silently ignored. Could this cause the freed pages' virtual
space to be permanently leaked and inaccessible?

>  			/* 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);

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] A race condition between dropping the arena spinlock and flushing the TLB allows concurrent reallocation and stale TLB access, leading to an information leak.
--

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

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.

> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 7b6847200b431..4597d9becb97a 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -881,7 +881,12 @@ static void arena_free_pages(struct bpf_arena *arena, long uaddr, long page_cnt,
>  	if (ret)
>  		goto defer;
>  
> -	range_tree_set(&arena->rt, pgoff, page_cnt);
> +	ret = range_tree_set(&arena->rt, pgoff, page_cnt);
> +	if (ret) {
> +		raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
> +		bpf_map_memcg_exit(old_memcg, new_memcg);
> +		return;
> +	}

[Severity: High]
This is a pre-existing issue, but does marking the range as free here,
before dropping the spinlock and flushing the TLB, create a race condition?

If CPU 1 calls arena_free_pages(), marks the pages as free via
range_tree_set(), clears the PTEs, and drops the spinlock:

        ret = range_tree_set(&arena->rt, pgoff, page_cnt);
        ...
        raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);

Before CPU 1 executes flush_tlb_kernel_range(), could CPU 2 allocate the
same virtual address via bpf_arena_alloc_pages() and populate new PTEs? 

If CPU 2 (or another CPU) previously accessed this virtual address, its
TLB might still point to the old physical page. Writes to this address
would then incorrectly modify the old physical page.

CPU 1 would then flush the TLB and free the old physical page to the buddy
allocator, potentially leaking sensitive data to other processes.

[ ... ]

> @@ -1004,11 +1009,18 @@ static void arena_free_worker(struct work_struct *work)
>  		kaddr = arena_vm_start + s->uaddr;
>  		pgoff = compute_pgoff(arena, s->uaddr);
>  
> +		/*
> +		 * Mark the range as free before clearing PTEs so that
> +		 * if the range tree update fails we leave the PTEs
> +		 * intact, avoiding an unrecoverable state where pages
> +		 * are freed but the arena free tree does not track them.
> +		 */
> +		if (range_tree_set(&arena->rt, pgoff, page_cnt))
> +			continue;

[Severity: High]
This isn't a bug introduced by this patch, but does the same race
condition exist here in the asynchronous worker?

The worker marks the range as free via range_tree_set() and drops the
spinlock:

        if (range_tree_set(&arena->rt, pgoff, page_cnt))
                continue;
        ...
        raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);

Before iterating the list again to call flush_tlb_kernel_range(), this
seems to expose the old physical pages to the same concurrent reallocation
race window as arena_free_pages().

> +
>  		/* clear ptes and collect pages in free_pages llist */
>  		apply_to_existing_page_range(&init_mm, kaddr, page_cnt << PAGE_SHIFT,
>  					     apply_range_clear_cb, &cdata);
> -
> -		range_tree_set(&arena->rt, pgoff, page_cnt);
>  	}
>  	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
>

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

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2199 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 could the changelog also state what went wrong
with the old ordering? The sibling patch d453da244bfd ("bpf, arena:
fix range_tree_set inconsistency on kmalloc_nolock failure") spells
out the consequence ("previously the overlapping nodes were already
removed by range_tree_clear() before the allocation was attempted,
permanently losing the cleared sub-ranges"), while this changelog only
describes the new behavior. Without explaining that the right sub-range
[last + 1, old_last] was permanently lost from the free tree when
kmalloc_nolock() failed after the left half had already been shrunk,
the two changelogs read inconsistently.

This looks like a bug fix for the original range_tree implementation.
Should this include:

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


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

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

[bot+bpf-ci]

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

> commit bfa5738a7c316a45ba16a067c92606ef3610c0b9
> Author: Yuan Chen <chenyuan@kylinos.cn>
> Date:   Mon Aug 10 17:46:34 2026 +0800
>
>     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.
>
>     Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>

This commit fixes missing error handling for range_tree_set() that can
lead to memory tracking inconsistencies. Should this include a Fixes: tag
pointing to the original commit that introduced bpf_arena?

  Fixes: 317460317a02 ("bpf: Introduce bpf_arena.")


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

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2203 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>

Should this include a Fixes: tag?

The commit message describes a concrete bug: on allocation failure the
range tree was left with overlapping free sub-ranges already removed,
permanently losing that arena free space.

The clear-then-allocate ordering has been present since
kernel/bpf/range_tree.c was introduced. The original code had exactly
the same structure, only with bpf_mem_alloc() instead of
kmalloc_nolock():

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

Concrete reproduction of the pre-patch bug: with the tree holding a
single free range {[10,20]} and a call to range_tree_set(rt, 5, 20)
(i.e. start=5, last=24), range_tree_clear() takes the 'in the middle of
the clearing range' branch, removing and freeing [10,20]. The subsequent
allocation then fails, range_tree_set() returns -ENOMEM, and the tree is
left empty -- the [10,20] free range is gone for the lifetime of the
arena.

Four of the five callers (arena_alloc_pages() rollback at
kernel/bpf/arena.c:774, arena_free_pages() at :884, arena_free_worker()
at :1011, arena_vm_fault() rollback at :523/:529) discard
range_tree_set()'s return value, so the loss is silent.

Should this include:

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


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

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

[Puranjay Mohan]

sashiko-bot@kernel.org writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] A race condition between dropping the arena spinlock and flushing the TLB allows concurrent reallocation and stale TLB access, leading to an information leak.
> --
>
> --- Patch [3]: [PATCH bpf-next v2 3/3] bpf, arena: check range_tree_set return in arena_free_pages and arena_free_worker ---
> commit d52cc998404e8cd2b8a37ee307bab5c587ce8336
> Author: Yuan Chen <chenyuan@kylinos.cn>
>
> 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.
>
>> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
>> index 7b6847200b431..4597d9becb97a 100644
>> --- a/kernel/bpf/arena.c
>> +++ b/kernel/bpf/arena.c
>> @@ -881,7 +881,12 @@ static void arena_free_pages(struct bpf_arena *arena, long uaddr, long page_cnt,
>>  	if (ret)
>>  		goto defer;
>>  
>> -	range_tree_set(&arena->rt, pgoff, page_cnt);
>> +	ret = range_tree_set(&arena->rt, pgoff, page_cnt);
>> +	if (ret) {
>> +		raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
>> +		bpf_map_memcg_exit(old_memcg, new_memcg);
>> +		return;
>> +	}
>
> [Severity: High]
> This is a pre-existing issue, but does marking the range as free here,
> before dropping the spinlock and flushing the TLB, create a race condition?
>
> If CPU 1 calls arena_free_pages(), marks the pages as free via
> range_tree_set(), clears the PTEs, and drops the spinlock:
>
>         ret = range_tree_set(&arena->rt, pgoff, page_cnt);
>         ...
>         raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
>
> Before CPU 1 executes flush_tlb_kernel_range(), could CPU 2 allocate the
> same virtual address via bpf_arena_alloc_pages() and populate new PTEs? 
>
> If CPU 2 (or another CPU) previously accessed this virtual address, its
> TLB might still point to the old physical page. Writes to this address
> would then incorrectly modify the old physical page.
>
> CPU 1 would then flush the TLB and free the old physical page to the buddy
> allocator, potentially leaking sensitive data to other processes.
>
> [ ... ]
>
>> @@ -1004,11 +1009,18 @@ static void arena_free_worker(struct work_struct *work)
>>  		kaddr = arena_vm_start + s->uaddr;
>>  		pgoff = compute_pgoff(arena, s->uaddr);
>>  
>> +		/*
>> +		 * Mark the range as free before clearing PTEs so that
>> +		 * if the range tree update fails we leave the PTEs
>> +		 * intact, avoiding an unrecoverable state where pages
>> +		 * are freed but the arena free tree does not track them.
>> +		 */
>> +		if (range_tree_set(&arena->rt, pgoff, page_cnt))
>> +			continue;
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does the same race
> condition exist here in the asynchronous worker?
>
> The worker marks the range as free via range_tree_set() and drops the
> spinlock:
>
>         if (range_tree_set(&arena->rt, pgoff, page_cnt))
>                 continue;
>         ...
>         raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
>
> Before iterating the list again to call flush_tlb_kernel_range(), this
> seems to expose the old physical pages to the same concurrent reallocation
> race window as arena_free_pages().
>

Both these issues are real and I am working on a fix for these. I
already saw them while fuzzing the BPF JITs through litmus tests.

Will post the patch soon.

Thanks,
Puranjay