[PATCH bpf-next 0/3] bpf: arena: handle memory.max on fault-in with reclaim/OOM

[PATCH bpf-next 0/3] bpf: arena: handle memory.max on fault-in with reclaim/OOM

[Jiayuan Chen]

Since commit e66fe1bc6d25 ("bpf: arena: Reintroduce memcg accounting"),
arena pages are charged to the memcg of the process that created the arena.
That accounting exposes two problems in the arena user page fault path.

1. The fault-in allocation runs under arena->spinlock, so it can only use the
   non-blocking allocator, which never reclaims. Once memory.current is at
   memory.max the allocation simply fails. Reaching memory.max is completely
   normal for a healthy application - e.g. reading a large file fills
   memory.current with page cache - so the process ends up killed for no real
   reason.

2. That failure is turned into VM_FAULT_SIGSEGV, which is misleading: the
   faulting address is a perfectly valid arena address. When we know it is an
   out-of-memory condition we can return VM_FAULT_OOM and let the memcg OOM
   path handle it properly.

Preallocate the page outside the lock (patch 2), the way do_anonymous_page()
does, so the allocation can sleep and go through reclaim and the OOM path.
This needs a sleepable allocator (patch 1), because can_alloc_pages() is a
conservative guess for BPF program context and always forces the non-blocking
allocator under PREEMPT_RT. patch 3 adds a selftest that faults an arena in
under a memory.max limit: without the fix the child gets SIGSEGV on a valid
address, with it the child is killed by the memcg OOM killer.

Jiayuan Chen (3):
  bpf: Add a sleepable page allocator for map memory
  bpf: arena: allocate the fault-in page outside the lock
  selftests/bpf: Add a test for arena fault-in under memory.max

 include/linux/bpf.h                           |   1 +
 kernel/bpf/arena.c                            |  83 ++++++---
 kernel/bpf/syscall.c                          |  21 ++-
 .../selftests/bpf/prog_tests/arena_memcg.c    | 158 ++++++++++++++++++
 .../testing/selftests/bpf/progs/arena_memcg.c |  24 +++
 5 files changed, 263 insertions(+), 24 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/arena_memcg.c
 create mode 100644 tools/testing/selftests/bpf/progs/arena_memcg.c

-- 
2.43.0

[PATCH bpf-next 1/3] bpf: Add a sleepable page allocator for map memory

[Jiayuan Chen]

bpf_map_alloc_pages() picks the allocator via can_alloc_pages(), a
conservative guess for BPF program context that is always false under
PREEMPT_RT. So even a caller that really is sleepable gets the
non-blocking allocator, which never reclaims and never engages the OOM
machinery.

Add bpf_map_alloc_page_sleepable() for callers that know they are
sleepable. The next patch uses it from the arena page fault handler.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 include/linux/bpf.h  |  1 +
 kernel/bpf/syscall.c | 21 +++++++++++++++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index d9542127dfdf..9ba03622709b 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2779,6 +2779,7 @@ struct bpf_prog *bpf_prog_get_curr_or_next(u32 *id);
 
 int bpf_map_alloc_pages(const struct bpf_map *map, int nid,
 			unsigned long nr_pages, struct page **page_array);
+struct page *bpf_map_alloc_page_sleepable(const struct bpf_map *map, int nid);
 #ifdef CONFIG_MEMCG
 void bpf_map_memcg_enter(const struct bpf_map *map, struct mem_cgroup **old_memcg,
 			 struct mem_cgroup **new_memcg);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 0ff9e3aa293d..cec88450e4ce 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -602,15 +602,14 @@ static bool can_alloc_pages(void)
 		!IS_ENABLED(CONFIG_PREEMPT_RT);
 }
 
+#define BPF_PAGE_GFP (GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT | __GFP_NOWARN)
+
 static struct page *__bpf_alloc_page(int nid)
 {
 	if (!can_alloc_pages())
 		return alloc_pages_nolock(__GFP_ACCOUNT, nid, 0);
 
-	return alloc_pages_node(nid,
-				GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT
-				| __GFP_NOWARN,
-				0);
+	return alloc_pages_node(nid, BPF_PAGE_GFP, 0);
 }
 
 int bpf_map_alloc_pages(const struct bpf_map *map, int nid,
@@ -636,6 +635,20 @@ int bpf_map_alloc_pages(const struct bpf_map *map, int nid,
 	return ret;
 }
 
+/*
+ * For callers that know they run in a sleepable context, e.g. a user page
+ * fault handler. can_alloc_pages() is a conservative guess made for BPF
+ * program context - notably it is always false on PREEMPT_RT - so going
+ * through bpf_map_alloc_pages() there would needlessly pick the
+ * non-blocking allocator, which never reclaims and never engages the OOM
+ * machinery.
+ */
+struct page *bpf_map_alloc_page_sleepable(const struct bpf_map *map, int nid)
+{
+	might_sleep();
+	return alloc_pages_node(nid, BPF_PAGE_GFP, 0);
+}
+
 
 static int btf_field_cmp(const void *a, const void *b)
 {
-- 
2.43.0

[PATCH bpf-next 2/3] bpf: arena: allocate the fault-in page outside the lock

[Jiayuan Chen]

arena_vm_fault() allocated the page while holding arena->spinlock, so it
could only use the non-blocking allocator. Once the memcg is at
memory.max that allocation just fails, the fault turns into
VM_FAULT_SIGSEGV, and the process gets a SIGSEGV on a perfectly valid
arena address. Hitting memory.max is routine (e.g. page cache from
reading a big file), so this kills innocent processes.

Preallocate the page before taking the lock, like do_anonymous_page()
does, so the allocation can sleep and go through reclaim and the OOM
path, and return VM_FAULT_OOM on failure so the memcg OOM handler runs
instead of a fake segfault. Also tidy up the error labels.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 kernel/bpf/arena.c | 83 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 63 insertions(+), 20 deletions(-)

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 34f023a537fe..22a41e3c53b8 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -481,7 +481,8 @@ static vm_fault_t arena_vm_fault(struct vm_fault *vmf)
 	struct bpf_map *map = vmf->vma->vm_file->private_data;
 	struct bpf_arena *arena = container_of(map, struct bpf_arena, map);
 	struct mem_cgroup *new_memcg, *old_memcg;
-	struct page *page;
+	struct page *page, *new_page = NULL;
+	vm_fault_t fault_ret;
 	long kbase, kaddr;
 	unsigned long flags;
 	int ret;
@@ -489,55 +490,97 @@ static vm_fault_t arena_vm_fault(struct vm_fault *vmf)
 	kbase = bpf_arena_get_kern_vm_start(arena);
 	kaddr = kbase + (u32)(vmf->address);
 
-	if (raw_res_spin_lock_irqsave(&arena->spinlock, flags))
+	page = vmalloc_to_page((void *)kaddr);
+	if (!page) {
+		/*
+		 * Preallocate outside the lock so the allocation can sleep and go
+		 * through reclaim (both memcg and global), the way do_anonymous_page()
+		 * does. Under arena->spinlock only the non-blocking allocator is
+		 * available, which never reclaims.
+		 *
+		 * This has to be the sleepable variant: VM_FAULT_OOM below is only
+		 * meaningful if the OOM machinery was actually engaged. A failure
+		 * from the non-blocking allocator engages nothing, so the fault
+		 * would be retried forever.
+		 */
+		bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
+		new_page = bpf_map_alloc_page_sleepable(map, NUMA_NO_NODE);
+		bpf_map_memcg_exit(old_memcg, new_memcg);
+		if (!new_page)
+			return VM_FAULT_OOM;
+	}
+
+	if (raw_res_spin_lock_irqsave(&arena->spinlock, flags)) {
 		/* Make a reasonable effort to address impossible case */
-		return VM_FAULT_RETRY;
+		fault_ret = VM_FAULT_RETRY;
+		goto out_err;
+	}
 
 	page = vmalloc_to_page((void *)kaddr);
 	if (page) {
-		if (page == arena->scratch_page)
+		if (page == arena->scratch_page) {
 			/* BPF triggered scratch here; don't lazy-alloc over it */
-			goto out_sigsegv;
+			fault_ret = VM_FAULT_SIGSEGV;
+			goto out_err_locked;
+		}
 		/* already have a page vmap-ed */
 		goto out;
 	}
 
+	/*
+	 * The lockless probe was racy: it saw a page, so nothing was
+	 * preallocated, but the re-check under the lock finds it gone - a
+	 * concurrent free must have run in between. There is nothing to
+	 * install and we cannot allocate under the lock, so retry the fault
+	 * and preallocate next time.
+	 */
+	if (!new_page) {
+		fault_ret = VM_FAULT_RETRY;
+		goto out_err_locked;
+	}
+
 	bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
 
-	if (arena->map.map_flags & BPF_F_SEGV_ON_FAULT)
+	if (arena->map.map_flags & BPF_F_SEGV_ON_FAULT) {
 		/* User space requested to segfault when page is not allocated by bpf prog */
-		goto out_sigsegv_memcg;
+		fault_ret = VM_FAULT_SIGSEGV;
+		goto out_err_locked_memcg;
+	}
 
 	ret = range_tree_clear(&arena->rt, vmf->pgoff, 1);
-	if (ret)
-		goto out_sigsegv_memcg;
-
-	struct apply_range_data data = { .arena = arena, .pages = &page, .i = 0 };
-	/* Account into memcg of the process that created bpf_arena */
-	ret = bpf_map_alloc_pages(map, NUMA_NO_NODE, 1, &page);
 	if (ret) {
-		range_tree_set(&arena->rt, vmf->pgoff, 1);
-		goto out_sigsegv_memcg;
+		fault_ret = VM_FAULT_OOM;
+		goto out_err_locked_memcg;
 	}
+	struct apply_range_data data = { .arena = arena, .pages = &new_page, .i = 0 };
 
 	ret = apply_to_page_range(&init_mm, kaddr, PAGE_SIZE, apply_range_set_cb, &data);
 	if (ret) {
 		range_tree_set(&arena->rt, vmf->pgoff, 1);
-		free_pages_nolock(page, 0);
-		goto out_sigsegv_memcg;
+		fault_ret = VM_FAULT_SIGSEGV;
+		goto out_err_locked_memcg;
 	}
 	flush_vmap_cache(kaddr, PAGE_SIZE);
 	bpf_map_memcg_exit(old_memcg, new_memcg);
+	/* new_page was consumed */
+	page = new_page;
+	new_page = NULL;
 out:
 	page_ref_add(page, 1);
 	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
+	if (new_page)
+		free_pages_nolock(new_page, 0);
 	vmf->page = page;
 	return 0;
-out_sigsegv_memcg:
+
+out_err_locked_memcg:
 	bpf_map_memcg_exit(old_memcg, new_memcg);
-out_sigsegv:
+out_err_locked:
 	raw_res_spin_unlock_irqrestore(&arena->spinlock, flags);
-	return VM_FAULT_SIGSEGV;
+out_err:
+	if (new_page)
+		free_pages_nolock(new_page, 0);
+	return fault_ret;
 }
 
 static const struct vm_operations_struct arena_vm_ops = {
-- 
2.43.0

[PATCH bpf-next 3/3] selftests/bpf: Add a test for arena fault-in under memory.max

[Jiayuan Chen]

A child joins a memcg capped at 64M and faults an arena in until it runs
out of the budget. Without the kernel fix the child dies with SIGSEGV on
a valid arena address; with it, the child is killed by the memcg OOM
killer.

With the fix:

  serial_test_arena_memcg:PASS:child killed by signal
  serial_test_arena_memcg:PASS:not killed by SIGSEGV
  #5       arena_memcg:OK

  # dmesg
   arena_vm_fault+0x655/0xa90
  Memory cgroup out of memory: Killed process 512, file-rss:67920kB

Without the fix:

  serial_test_arena_memcg:PASS:child killed by signal
  serial_test_arena_memcg:FAIL:not killed by SIGSEGV: actual 11
  #5       arena_memcg:FAIL

  # dmesg
  test_progs[508]: segfault at 100004025000 ...

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 .../selftests/bpf/prog_tests/arena_memcg.c    | 158 ++++++++++++++++++
 .../testing/selftests/bpf/progs/arena_memcg.c |  24 +++
 2 files changed, 182 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/arena_memcg.c
 create mode 100644 tools/testing/selftests/bpf/progs/arena_memcg.c

diff --git a/tools/testing/selftests/bpf/prog_tests/arena_memcg.c b/tools/testing/selftests/bpf/prog_tests/arena_memcg.c
new file mode 100644
index 000000000000..9665946fa29e
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/arena_memcg.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <sys/user.h>
+#ifndef PAGE_SIZE /* on some archs it comes in sys/user.h */
+#include <unistd.h>
+#define PAGE_SIZE getpagesize()
+#endif
+
+#include "cgroup_helpers.h"
+#include "arena_memcg.skel.h"
+
+#define CG_PATH		"/arena_memcg"
+
+/* Budget the arena gets on top of whatever is already charged after load. */
+#define ARENA_BUDGET	(64 * 1024 * 1024)
+
+/*
+ * cgroup_helpers builds paths from getpid(), but the work dir belongs to the
+ * process that set the environment up. The child references it through that
+ * pid, so build the path explicitly.
+ */
+static void cg_file_path(char *buf, size_t sz, pid_t owner, const char *file)
+{
+	snprintf(buf, sz, "/mnt/cgroup-test-work-dir%d%s/%s", owner, CG_PATH, file);
+}
+
+static long cg_read_ulong(pid_t owner, const char *file)
+{
+	char path[PATH_MAX], buf[64];
+	long val = -1;
+	FILE *f;
+
+	cg_file_path(path, sizeof(path), owner, file);
+	f = fopen(path, "r");
+	if (!f)
+		return -1;
+	if (fgets(buf, sizeof(buf), f))
+		val = strtol(buf, NULL, 10);
+	fclose(f);
+	return val;
+}
+
+static int cg_write(pid_t owner, const char *file, const char *val)
+{
+	char path[PATH_MAX];
+	int fd, len, ret = -1;
+
+	cg_file_path(path, sizeof(path), owner, file);
+	fd = open(path, O_WRONLY);
+	if (fd < 0)
+		return -1;
+	len = strlen(val);
+	if (write(fd, val, len) == len)
+		ret = 0;
+	close(fd);
+	return ret;
+}
+
+void serial_test_arena_memcg(void)
+{
+	int cgroup_fd = -1, status;
+	const long ps = PAGE_SIZE;
+	pid_t owner, pid;
+
+	if (setup_cgroup_environment())
+		return;
+	owner = getpid();
+
+	cgroup_fd = create_and_get_cgroup(CG_PATH);
+	if (!ASSERT_OK_FD(cgroup_fd, "create_and_get_cgroup"))
+		goto out;
+
+	/* No memory controller -> nothing to test. */
+	if (cg_read_ulong(owner, "memory.current") < 0) {
+		test__skip();
+		goto out;
+	}
+
+	pid = fork();
+	if (!ASSERT_GE(pid, 0, "fork"))
+		goto out;
+	if (pid == 0) {
+		struct arena_memcg *cskel;
+		__u32 i, npages;
+		char buf[32];
+		char *base;
+		size_t sz;
+		long cur;
+
+		/*
+		 * Do everything from the child: the arena vma is VM_DONTCOPY so
+		 * it would not survive fork(), only the child should be under the
+		 * limit so that a memcg OOM cannot pick test_progs, and a map is
+		 * charged to the memcg of the task that creates it - so join
+		 * before load. Errors are reported to the parent through the exit
+		 * code, since ASSERT_* in a forked child does not reach it.
+		 */
+		snprintf(buf, sizeof(buf), "%d", getpid());
+		if (cg_write(owner, "cgroup.procs", buf))
+			_exit(2);
+
+		cskel = arena_memcg__open_and_load();
+		if (!cskel)
+			_exit(3);
+
+		base = bpf_map__initial_value(cskel->maps.arena, &sz);
+		if (!base)
+			_exit(4);
+		npages = bpf_map__max_entries(cskel->maps.arena);
+
+		/*
+		 * Cap only now, after load: everything but the fault-in is
+		 * charged, so the arena gets a fixed budget regardless of what
+		 * the load itself cost, and the load can never hit the limit.
+		 */
+		cur = cg_read_ulong(owner, "memory.current");
+		if (cur < 0)
+			_exit(5);
+		snprintf(buf, sizeof(buf), "%ld", cur + ARENA_BUDGET);
+		if (cg_write(owner, "memory.max", buf))
+			_exit(6);
+
+		for (i = 0; i < npages; i++)
+			base[(size_t)i * ps] = 1;
+		_exit(0);
+	}
+
+	if (!ASSERT_EQ(waitpid(pid, &status, 0), pid, "waitpid"))
+		goto out;
+
+	/* A non-zero exit means the child failed to set up; the code says where. */
+	if (WIFEXITED(status) && WEXITSTATUS(status)) {
+		ASSERT_OK(WEXITSTATUS(status), "child setup");
+		goto out;
+	}
+
+	/*
+	 * Faulting a valid arena address until memory.max is hit must not look
+	 * like an invalid access. Without the fix the fault path allocated with
+	 * the non-blocking allocator, turned its -ENOMEM into VM_FAULT_SIGSEGV,
+	 * and the child died with SIGSEGV on a valid address; now it is handled
+	 * by the memcg OOM path and the child is killed by SIGKILL instead. A
+	 * clean exit means the child failed to set up (see the exit codes).
+	 */
+	if (!ASSERT_TRUE(WIFSIGNALED(status), "child killed by signal"))
+		goto out;
+	ASSERT_NEQ(WTERMSIG(status), SIGSEGV, "not killed by SIGSEGV");
+out:
+	if (cgroup_fd >= 0)
+		close(cgroup_fd);
+	cleanup_cgroup_environment();
+}
diff --git a/tools/testing/selftests/bpf/progs/arena_memcg.c b/tools/testing/selftests/bpf/progs/arena_memcg.c
new file mode 100644
index 000000000000..adecd9e8463e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/arena_memcg.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_arena_common.h"
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARENA);
+	__uint(map_flags, BPF_F_MMAPABLE);
+	__uint(max_entries, 100000); /* number of pages */
+#ifdef __TARGET_ARCH_arm64
+	__ulong(map_extra, 0x1ull << 32); /* start of mmap() region */
+#else
+	__ulong(map_extra, 0x1ull << 44); /* start of mmap() region */
+#endif
+} arena SEC(".maps");
+
+SEC("syscall")
+int noop(void *ctx)
+{
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.43.0

Re: [PATCH bpf-next 1/3] bpf: Add a sleepable page allocator for map memory

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] NUMA Mempolicy Bypass in bpf_map_alloc_page_sleepable() and __bpf_alloc_page()
--

--- Patch [1]: [PATCH bpf-next 1/3] bpf: Add a sleepable page allocator for map memory ---
commit 7a6023ef7f19fc884e94ffe457ae83ff0d4b5e1e
Author: Jiayuan Chen <jiayuan.chen@linux.dev>

bpf: Add a sleepable page allocator for map memory

This commit introduces bpf_map_alloc_page_sleepable() to provide a blocking
page allocator for callers known to be in a sleepable context, such as user
page fault handlers. This avoids the conservative, non-blocking behavior of
can_alloc_pages() which is always false under PREEMPT_RT.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
[ ... ]
> @@ -602,15 +602,14 @@ static bool can_alloc_pages(void)
>  		!IS_ENABLED(CONFIG_PREEMPT_RT);
>  }
>  
> +#define BPF_PAGE_GFP (GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT | __GFP_NOWARN)
> +
>  static struct page *__bpf_alloc_page(int nid)
>  {
>  	if (!can_alloc_pages())
>  		return alloc_pages_nolock(__GFP_ACCOUNT, nid, 0);
>  
> -	return alloc_pages_node(nid,
> -				GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT
> -				| __GFP_NOWARN,
> -				0);
> +	return alloc_pages_node(nid, BPF_PAGE_GFP, 0);

[Severity: Medium]
This isn't a bug introduced by this patch, but does using alloc_pages_node()
here bypass the task's NUMA mempolicy?

Because alloc_pages_node() ignores task mempolicy, pages might land on the
current CPU's local NUMA node instead of the node requested by the task.
Could this branch to use a mempolicy-aware allocation when nid is
NUMA_NO_NODE?

>  }
[ ... ]
> +/*
> + * For callers that know they run in a sleepable context, e.g. a user page
> + * fault handler. can_alloc_pages() is a conservative guess made for BPF
> + * program context - notably it is always false on PREEMPT_RT - so going
> + * through bpf_map_alloc_pages() there would needlessly pick the
> + * non-blocking allocator, which never reclaims and never engages the OOM
> + * machinery.
> + */
> +struct page *bpf_map_alloc_page_sleepable(const struct bpf_map *map, int nid)
> +{
> +	might_sleep();
> +	return alloc_pages_node(nid, BPF_PAGE_GFP, 0);

[Severity: Medium]
Since this function is intended for user space page fault handlers, will
unconditionally calling alloc_pages_node() cause the allocation to ignore
the user task's mempolicy?

Similar to __bpf_alloc_page(), if nid is NUMA_NO_NODE, replacing this with a
mempolicy-aware allocation might prevent silent performance regressions from
misaligned memory placement during page faults.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727062521.376231-1-jiayuan.chen@linux.dev?part=1

Re: [PATCH bpf-next 2/3] bpf: arena: allocate the fault-in page outside the lock

[sashiko-bot]

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

New issues:
- [High] Unnecessary sleepable memory allocation introduces a Denial of Service and OOM vector when BPF_F_SEGV_ON_FAULT is set.

Pre-existing issues:
- [High] Missing lock release before returning VM_FAULT_RETRY causes a lock leak and deadlocks.
--

--- Patch [2]: [PATCH bpf-next 2/3] bpf: arena: allocate the fault-in page outside the lock ---
commit 618b434e77e16e8a5277e1b3cc85c17fc5ef5e7a
Author: Jiayuan Chen <jiayuan.chen@linux.dev>

bpf: arena: allocate the fault-in page outside the lock

arena_vm_fault() allocated the page while holding arena->spinlock, so it
could only use the non-blocking allocator. Once the memcg is at
memory.max that allocation just fails, the fault turns into
VM_FAULT_SIGSEGV, and the process gets a SIGSEGV on a perfectly valid
arena address. Hitting memory.max is routine (e.g. page cache from
reading a big file), so this kills innocent processes.

Preallocate the page before taking the lock, like do_anonymous_page()
does, so the allocation can sleep and go through reclaim and the OOM
path, and return VM_FAULT_OOM on failure so the memcg OOM handler runs
instead of a fake segfault. Also tidy up the error labels.

> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 34f023a537fe6..22a41e3c53b81 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -481,7 +481,8 @@ static vm_fault_t arena_vm_fault(struct vm_fault *vmf)
>  	struct bpf_map *map = vmf->vma->vm_file->private_data;
>  	struct bpf_arena *arena = container_of(map, struct bpf_arena, map);
>  	struct mem_cgroup *new_memcg, *old_memcg;
> -	struct page *page;
> +	struct page *page, *new_page = NULL;
> +	vm_fault_t fault_ret;
>  	long kbase, kaddr;
>  	unsigned long flags;
>  	int ret;
> @@ -489,55 +490,97 @@ static vm_fault_t arena_vm_fault(struct vm_fault *vmf)
>  	kbase = bpf_arena_get_kern_vm_start(arena);
>  	kaddr = kbase + (u32)(vmf->address);
>  
> -	if (raw_res_spin_lock_irqsave(&arena->spinlock, flags))
> +	page = vmalloc_to_page((void *)kaddr);
> +	if (!page) {
> +		/*
> +		 * Preallocate outside the lock so the allocation can sleep and go
> +		 * through reclaim (both memcg and global), the way do_anonymous_page()
> +		 * does. Under arena->spinlock only the non-blocking allocator is
> +		 * available, which never reclaims.
> +		 *
> +		 * This has to be the sleepable variant: VM_FAULT_OOM below is only
> +		 * meaningful if the OOM machinery was actually engaged. A failure
> +		 * from the non-blocking allocator engages nothing, so the fault
> +		 * would be retried forever.
> +		 */
> +		bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
> +		new_page = bpf_map_alloc_page_sleepable(map, NUMA_NO_NODE);

[Severity: High]
Is there a potential Denial of Service vector when BPF_F_SEGV_ON_FAULT is set?

This unconditionally preallocates a page using
bpf_map_alloc_page_sleepable() before checking the BPF_F_SEGV_ON_FAULT
flag later in the function.

This allocation can block, trigger direct memory reclaim, and invoke the
OOM killer. Only after this expensive allocation does the code lock, check
the flag, return VM_FAULT_SIGSEGV, and discard the page.

Could this allow an unprivileged user to force severe memory pressure by
repeatedly triggering page faults on unallocated addresses?

> +		bpf_map_memcg_exit(old_memcg, new_memcg);
> +		if (!new_page)
> +			return VM_FAULT_OOM;
> +	}
> +
> +	if (raw_res_spin_lock_irqsave(&arena->spinlock, flags)) {
>  		/* Make a reasonable effort to address impossible case */
> -		return VM_FAULT_RETRY;
> +		fault_ret = VM_FAULT_RETRY;
> +		goto out_err;
> +	}

[Severity: High]
This is a pre-existing issue, but does returning VM_FAULT_RETRY here
without releasing the fault lock cause a lock leak?

The memory management subsystem strictly assumes the lock was released
if VM_FAULT_RETRY is returned. Retrying the fault will leak the read lock
reference, and if any thread subsequently attempts to acquire the mmap_lock
for writing, the system could permanently deadlock.

>  	page = vmalloc_to_page((void *)kaddr);
>  	if (page) {
> -		if (page == arena->scratch_page)
> +		if (page == arena->scratch_page) {
>  			/* BPF triggered scratch here; don't lazy-alloc over it */
> -			goto out_sigsegv;
> +			fault_ret = VM_FAULT_SIGSEGV;
> +			goto out_err_locked;
> +		}
>  		/* already have a page vmap-ed */
>  		goto out;
>  	}
>  
> +	/*
> +	 * The lockless probe was racy: it saw a page, so nothing was
> +	 * preallocated, but the re-check under the lock finds it gone - a
> +	 * concurrent free must have run in between. There is nothing to
> +	 * install and we cannot allocate under the lock, so retry the fault
> +	 * and preallocate next time.
> +	 */
> +	if (!new_page) {
> +		fault_ret = VM_FAULT_RETRY;
> +		goto out_err_locked;
> +	}

[Severity: High]
Could returning VM_FAULT_RETRY here in this new race handling path also
leak the fault lock and risk deadlocks, similar to the lock acquisition
failure above?

> +
>  	bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
>  
> -	if (arena->map.map_flags & BPF_F_SEGV_ON_FAULT)
> +	if (arena->map.map_flags & BPF_F_SEGV_ON_FAULT) {
>  		/* User space requested to segfault when page is not allocated by bpf prog */
> -		goto out_sigsegv_memcg;
> +		fault_ret = VM_FAULT_SIGSEGV;
> +		goto out_err_locked_memcg;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727062521.376231-1-jiayuan.chen@linux.dev?part=2

Re: [PATCH bpf-next 2/3] bpf: arena: allocate the fault-in page outside the lock

[bot+bpf-ci]

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

> bpf: arena: allocate the fault-in page outside the lock
>
> arena_vm_fault() allocated the page while holding arena->spinlock, so it
> could only use the non-blocking allocator. Once the memcg is at
> memory.max that allocation just fails, the fault turns into
> VM_FAULT_SIGSEGV, and the process gets a SIGSEGV on a perfectly valid
> arena address. Hitting memory.max is routine (e.g. page cache from
> reading a big file), so this kills innocent processes.
>
> Preallocate the page before taking the lock, like do_anonymous_page()
> does, so the allocation can sleep and go through reclaim and the OOM
> path, and return VM_FAULT_OOM on failure so the memcg OOM handler runs
> instead of a fake segfault. Also tidy up the error labels.
>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>

This looks like a fix for a real bug that could send a SIGSEGV to a
process on a valid arena address once the memcg hits memory.max.

The allocation was moved under arena->spinlock, and the error path
switched to VM_FAULT_SIGSEGV, when arena_vm_fault() replaced its
guard(mutex)(&arena->lock) with raw_res_spin_lock_irqsave(&arena->spinlock,
flags). Should this carry a Fixes: tag pointing at that change?

  Fixes: b8467290edab ("bpf: arena: make arena kfuncs any context safe")


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

Re: [PATCH bpf-next 1/3] bpf: Add a sleepable page allocator for map memory

[Jiayuan Chen]

On 7/27/26 2:37 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] NUMA Mempolicy Bypass in bpf_map_alloc_page_sleepable() and __bpf_alloc_page()
> --
>
> --- Patch [1]: [PATCH bpf-next 1/3] bpf: Add a sleepable page allocator for map memory ---
> commit 7a6023ef7f19fc884e94ffe457ae83ff0d4b5e1e
> Author: Jiayuan Chen <jiayuan.chen@linux.dev>
>
> bpf: Add a sleepable page allocator for map memory
>
> This commit introduces bpf_map_alloc_page_sleepable() to provide a blocking
> page allocator for callers known to be in a sleepable context, such as user
> page fault handlers. This avoids the conservative, non-blocking behavior of
> can_alloc_pages() which is always false under PREEMPT_RT.
>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
> [ ... ]
>> @@ -602,15 +602,14 @@ static bool can_alloc_pages(void)
>>   		!IS_ENABLED(CONFIG_PREEMPT_RT);
>>   }
>>   
>> +#define BPF_PAGE_GFP (GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT | __GFP_NOWARN)
>> +
>>   static struct page *__bpf_alloc_page(int nid)
>>   {
>>   	if (!can_alloc_pages())
>>   		return alloc_pages_nolock(__GFP_ACCOUNT, nid, 0);
>>   
>> -	return alloc_pages_node(nid,
>> -				GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT
>> -				| __GFP_NOWARN,
>> -				0);
>> +	return alloc_pages_node(nid, BPF_PAGE_GFP, 0);
> [Severity: Medium]
> This isn't a bug introduced by this patch, but does using alloc_pages_node()
> here bypass the task's NUMA mempolicy?
>
> Because alloc_pages_node() ignores task mempolicy, pages might land on the
> current CPU's local NUMA node instead of the node requested by the task.
> Could this branch to use a mempolicy-aware allocation when nid is
> NUMA_NO_NODE?
>
>>   }
> [ ... ]
>> +/*
>> + * For callers that know they run in a sleepable context, e.g. a user page
>> + * fault handler. can_alloc_pages() is a conservative guess made for BPF
>> + * program context - notably it is always false on PREEMPT_RT - so going
>> + * through bpf_map_alloc_pages() there would needlessly pick the
>> + * non-blocking allocator, which never reclaims and never engages the OOM
>> + * machinery.
>> + */
>> +struct page *bpf_map_alloc_page_sleepable(const struct bpf_map *map, int nid)
>> +{
>> +	might_sleep();
>> +	return alloc_pages_node(nid, BPF_PAGE_GFP, 0);
> [Severity: Medium]
> Since this function is intended for user space page fault handlers, will
> unconditionally calling alloc_pages_node() cause the allocation to ignore
> the user task's mempolicy?
>
> Similar to __bpf_alloc_page(), if nid is NUMA_NO_NODE, replacing this with a
> mempolicy-aware allocation might prevent silent performance regressions from
> misaligned memory placement during page faults.
>
>> +}


You remind me. But I don't think this is really about (task) mempolicy.
We should respect map.numa_node/map->numa_node, just like other maps do
when allocating their elements.

Re: [PATCH bpf-next 2/3] bpf: arena: allocate the fault-in page outside the lock

[Jiayuan Chen]

On 7/27/26 2:42 PM, sashiko-bot@kernel.org wrote:
[...]
>> @@ -489,55 +490,97 @@ static vm_fault_t arena_vm_fault(struct vm_fault *vmf)
>>   	kbase = bpf_arena_get_kern_vm_start(arena);
>>   	kaddr = kbase + (u32)(vmf->address);
>>   
>> -	if (raw_res_spin_lock_irqsave(&arena->spinlock, flags))
>> +	page = vmalloc_to_page((void *)kaddr);
>> +	if (!page) {
>> +		/*
>> +		 * Preallocate outside the lock so the allocation can sleep and go
>> +		 * through reclaim (both memcg and global), the way do_anonymous_page()
>> +		 * does. Under arena->spinlock only the non-blocking allocator is
>> +		 * available, which never reclaims.
>> +		 *
>> +		 * This has to be the sleepable variant: VM_FAULT_OOM below is only
>> +		 * meaningful if the OOM machinery was actually engaged. A failure
>> +		 * from the non-blocking allocator engages nothing, so the fault
>> +		 * would be retried forever.
>> +		 */
>> +		bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
>> +		new_page = bpf_map_alloc_page_sleepable(map, NUMA_NO_NODE);
> [Severity: High]
> Is there a potential Denial of Service vector when BPF_F_SEGV_ON_FAULT is set?
>
> This unconditionally preallocates a page using
> bpf_map_alloc_page_sleepable() before checking the BPF_F_SEGV_ON_FAULT
> flag later in the function.
>
> This allocation can block, trigger direct memory reclaim, and invoke the
> OOM killer. Only after this expensive allocation does the code lock, check
> the flag, return VM_FAULT_SIGSEGV, and discard the page.
>
> Could this allow an unprivileged user to force severe memory pressure by
> repeatedly triggering page faults on unallocated addresses?

Once VM_FAULT_SIGSEGV is returned, the process gets a SIGSEGV and by 
default is killed,
so this isn't a realistic DoS.

But, I think it's worth moving the "arena->map.map_flags & 
BPF_F_SEGV_ON_FAULT" check before the allocation.


>> +		bpf_map_memcg_exit(old_memcg, new_memcg);
>> +		if (!new_page)
>> +			return VM_FAULT_OOM;
>> +	}
>> +
>> +	if (raw_res_spin_lock_irqsave(&arena->spinlock, flags)) {
>>   		/* Make a reasonable effort to address impossible case */
>> -		return VM_FAULT_RETRY;
>> +		fault_ret = VM_FAULT_RETRY;
>> +		goto out_err;
>> +	}
> [Severity: High]
> This is a pre-existing issue, but does returning VM_FAULT_RETRY here
> without releasing the fault lock cause a lock leak?
>
> The memory management subsystem strictly assumes the lock was released
> if VM_FAULT_RETRY is returned. Retrying the fault will leak the read lock
> reference, and if any thread subsequently attempts to acquire the mmap_lock
> for writing, the system could permanently deadlock.


Yes, it's true. arena_vm_fault() never touches mmap_lock, so returning 
VM_FAULT_RETRY violates the contract.

'''
do_user_addr_fault()
{
     fault = handle_mm_fault(...);          // call arena_vm_fault
     ...
     if (unlikely(fault & VM_FAULT_RETRY)) {
         flags |= FAULT_FLAG_TRIED;
         goto retry;                    // lock_mm_and_find_vma() will 
call mmap_read_lock again !
     }
     mmap_read_unlock(mm);
}
'''

I think I should fix it as a separate patch with high priority ?