Skip to contentSkip to conversation

bpf: Validate node_id in arena_alloc_pages()

PATCH Applied Puranjay Mohan 5 participants 5 messages Apr 17 BPF

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 bpf: Validate node_id in arena_alloc_pages()

arena_alloc_pages() accepts a plain int node_id and forwards it through the entire allocation chain without any bounds checking.

Validate node_id before passing it down the allocation chain in arena_alloc_pages().

Fixes 317460317a02 ("bpf: Introduce bpf_arena.")
---
 kernel/bpf/arena.c | 4 ++++
 1 file changed, 4 insertions(+)
kernel/bpf/arena.c +4 -0
index 9c68c9b0b24a..523c3a61063b 100644
@@ -562,6 +562,10 @@ static long arena_alloc_pages(struct bpf_arena *arena, long uaddr, long page_cnt
 	u32 uaddr32;
 	int ret, i;
 
+	if (node_id != NUMA_NO_NODE &&
+	    ((unsigned int)node_id >= nr_node_ids || !node_online(node_id)))
+		return 0;
+
 	if (page_cnt > page_cnt_max)
 		return 0;
 

base-commit: 380044c40b1636a72fd8f188b5806be6ae564279

Signature · 1 line
2.52.0
reviewer Reviewed-by on v1 1/1
#1 lore raw
Quoted 5 lines · Puranjay Mohan

arena_alloc_pages() accepts a plain int node_id and forwards it through the entire allocation chain without any bounds checking.

Validate node_id before passing it down the allocation chain in arena_alloc_pages().

Without this, out of bounds node ids trigger VM_BUG_ON() down the allocation path.

Quoted 24 lines
Fixes 317460317a02 ("bpf: Introduce bpf_arena.")
---
 kernel/bpf/arena.c | 4 ++++
 1 file changed, 4 insertions(+)
kernel/bpf/arena.c +4 -0
index 9c68c9b0b24a..523c3a61063b 100644
@@ -562,6 +562,10 @@ static long arena_alloc_pages(struct bpf_arena *arena, long uaddr, long page_cnt
 	u32 uaddr32;
 	int ret, i;
 
+	if (node_id != NUMA_NO_NODE &&
+	    ((unsigned int)node_id >= nr_node_ids || !node_online(node_id)))
+		return 0;
+
 	if (page_cnt > page_cnt_max)
 		return 0;
 

base-commit: 380044c40b1636a72fd8f188b5806be6ae564279

#2 lore raw
In reply to Puranjay Mohan
Sashiko AI review found 2 potential issue(s): - [High] This problem wasn't introduced by this patch, but apply_to_page_range() is called within an atomic critical section with IRQs disabled.…

Sashiko AI review found 2 potential issue(s):
- [High] This problem wasn't introduced by this patch, but apply_to_page_range() is called within an atomic critical section with IRQs disabled.
- [Medium] Validation of user-provided NUMA node ID uses node_online() instead of node_state(..., N_MEMORY).
--

commit 312a17d46d0e6285c5b29bc93a445829df8c62c7
Author: Puranjay Mohan <[email protected]>

bpf: Validate node_id in arena_alloc_pages()

This commit validates the node_id parameter before passing it down the allocation chain in arena_alloc_pages().

kernel/bpf/arena.c static long arena_alloc_pages(struct bpf_arena *arena, long uaddr, long page_cnt · line 568

Quoted 12 lines
kernel/bpf/arena.c +4 -0
index 9c68c9b0b24ad..523c3a61063bf 100644
@@ -562,6 +562,10 @@ static long arena_alloc_pages(struct bpf_arena *arena, long uaddr, long page_cnt
 	u32 uaddr32;
 	int ret, i;
 
+	if (node_id != NUMA_NO_NODE &&
+	    ((unsigned int)node_id >= nr_node_ids || !node_online(node_id)))
+		return 0;
+

Should this check use node_state(node_id, N_MEMORY) instead of node_online(node_id)?

A node can be online but memory-less, such as CPU-only nodes. Passing a memory-less node down the allocation chain could ignore the user's placement intent and cause unexpected allocator fallback.

This issue isn't introduced by this patch, but looking at the surrounding code in arena_alloc_pages(), is it safe to call apply_to_page_range() while holding a raw spinlock?

arena_alloc_pages()

    if (raw_res_spin_lock_irqsave(&arena->spinlock, flags))
        goto out_free_pages;
    ...
    while (remaining) {
        ...
        ret = apply_to_page_range(...);

Since apply_to_page_range() can sleep when allocating page table entries, could this cause a sleeping function called from invalid context panic since the spinlock disables interrupts?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#3 lore raw
Quoted 30 lines · On Fri, Apr 17, 2026 at 5:55 PM <[email protected]> wrote:

Sashiko AI review found 2 potential issue(s):
- [High] This problem wasn't introduced by this patch, but apply_to_page_range() is called within an atomic critical section with IRQs disabled.
- [Medium] Validation of user-provided NUMA node ID uses node_online() instead of node_state(..., N_MEMORY).
--

commit 312a17d46d0e6285c5b29bc93a445829df8c62c7
Author: Puranjay Mohan <[email protected]>

bpf: Validate node_id in arena_alloc_pages()

This commit validates the node_id parameter before passing it down the allocation chain in arena_alloc_pages().

kernel/bpf/arena.c static long arena_alloc_pages(struct bpf_arena *arena, long uaddr, long page_cnt · line 568

Quoted 12 lines
kernel/bpf/arena.c +4 -0
index 9c68c9b0b24ad..523c3a61063bf 100644
@@ -562,6 +562,10 @@ static long arena_alloc_pages(struct bpf_arena *arena, long uaddr, long page_cnt
      u32 uaddr32;
      int ret, i;
 
+     if (node_id != NUMA_NO_NODE &&
+         ((unsigned int)node_id >= nr_node_ids || !node_online(node_id)))
+             return 0;
+

Should this check use node_state(node_id, N_MEMORY) instead of node_online(node_id)?

Quoted 3 lines

A node can be online but memory-less, such as CPU-only nodes. Passing a memory-less node down the allocation chain could ignore the user's placement intent and cause unexpected allocator fallback.

That is fine I think. it is just a hint.

Quoted 13 lines

This issue isn't introduced by this patch, but looking at the surrounding code in arena_alloc_pages(), is it safe to call apply_to_page_range() while holding a raw spinlock?

arena_alloc_pages()

    if (raw_res_spin_lock_irqsave(&arena->spinlock, flags))
        goto out_free_pages;
    ...
    while (remaining) {
        ...
        ret = apply_to_page_range(...);
Quoted 3 lines

Since apply_to_page_range() can sleep when allocating page table entries, could this cause a sleeping function called from invalid context panic since the spinlock disables interrupts?

No, the page tables are already allocated we just do set_pte_at(&init_mm, addr, pte, mk_pte(page, PAGE_KERNEL)); in the callback, so it is fine.

#4 lore raw
In reply to Puranjay Mohan
This patch was applied to bpf/bpf.git (master) by Alexei Starovoitov <[email protected]>:

Hello:

This patch was applied to bpf/bpf.git (master)
by Alexei Starovoitov <[email protected]>:

Quoted 10 lines · On Fri, 17 Apr 2026 08:21:33 -0700 you wrote:

arena_alloc_pages() accepts a plain int node_id and forwards it through the entire allocation chain without any bounds checking.

Validate node_id before passing it down the allocation chain in arena_alloc_pages().

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

[...]

Here is the summary with links:
- [bpf] bpf: Validate node_id in arena_alloc_pages()

    https://git.kernel.org/bpf/bpf/c/2845989f2eba

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html