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

[PATCH bpf-next v2 0/4] 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&4 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.


v1 -> v2:
   - Rebase on the separate deadlock fix (found by the Sashiko AI review),
     now applied to bpf-next.
   - Honor the map's NUMA node on fault-in.
   - Return VM_FAULT_SIGBUS for the non-recoverable faults (lock, range-tree
     and page-table failures); a scratch-page hole stays VM_FAULT_SIGSEGV
     only under BPF_F_SEGV_ON_FAULT. (Kumar Kartikeya Dwivedi)
   - Add read_cgroup_file() to cgroup_helpers instead of open-coding the
     /mnt/... path in the test. (Emil Tsalapatis)
   - Dump the cgroup memory stats on test failure to ease debugging.

v1:
https://lore.kernel.org/bpf/20260727062521.376231-1-jiayuan.chen@linux.dev/

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

 include/linux/bpf.h                           |   1 +
 kernel/bpf/arena.c                            |  92 +++++++++---
 kernel/bpf/syscall.c                          |  21 ++-
 tools/testing/selftests/bpf/cgroup_helpers.c  |  67 +++++++++
 tools/testing/selftests/bpf/cgroup_helpers.h  |   4 +
 .../selftests/bpf/prog_tests/arena_memcg.c    | 139 ++++++++++++++++++
 .../testing/selftests/bpf/progs/arena_memcg.c |  24 +++
 7 files changed, 323 insertions(+), 25 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 v2 1/4] 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. It allocates from the map's numa_node, like the other map
allocators. The next patch uses it from the arena page fault handler.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
---
 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 73bacfc6444d..0dc51c37c25c 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2784,6 +2784,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);
 #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 8d111da88655..67d8157c6623 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)
+{
+	might_sleep();
+	return alloc_pages_node(map->numa_node, BPF_PAGE_GFP, 0);
+}
+
 
 static int btf_field_cmp(const void *a, const void *b)
 {
-- 
2.43.0

[PATCH bpf-next v2 2/4] 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.

Rework the fault handler:

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

- A lockless probe skips that preallocation when a page is already mapped
  (e.g. allocated by the bpf program), so the common case wastes no
  allocation. The rare race where such a page is freed before we take the
  lock falls back to the non-blocking allocator under the lock.

- Return VM_FAULT_SIGBUS for the non-recoverable errors (lock failure,
  range-tree and page-table failures) instead of VM_FAULT_SIGSEGV; only
  BPF_F_SEGV_ON_FAULT, and a scratch-page hole under that flag, is a real
  user addressing error and keeps VM_FAULT_SIGSEGV.

- Tidy up the error labels.

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

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 555ee2531ef9..09a718ca4c8b 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,59 +490,108 @@ 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 && !(arena->map.map_flags & BPF_F_SEGV_ON_FAULT)) {
+		/*
+		 * We run in process context here, so preallocate the page
+		 * outside the lock with an explicitly sleepable allocator. It
+		 * can then go through reclaim (both memcg and global) and the
+		 * OOM path, the way do_anonymous_page() does; under
+		 * arena->spinlock only the non-blocking allocator is available,
+		 * which never reclaims. That also decides the return value:
+		 * VM_FAULT_OOM below is only meaningful if the OOM machinery was
+		 * actually engaged, which the non-blocking allocator never does.
+		 */
+		bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
+		new_page = bpf_map_alloc_page_sleepable(map);
+		bpf_map_memcg_exit(old_memcg, new_memcg);
+		if (!new_page)
+			return VM_FAULT_OOM;
+	}
+
+	if (raw_res_spin_lock_irqsave(&arena->spinlock, flags)) {
 		/*
 		 * A failed lock means a possible deadlock was detected. Don't
 		 * return VM_FAULT_RETRY: this handler never took mmap_lock, but
 		 * the fault path would re-take it on retry and deadlock. Fail.
 		 */
+		if (new_page)
+			free_pages_nolock(new_page, 0);
 		return VM_FAULT_SIGBUS;
+	}
 
 	page = vmalloc_to_page((void *)kaddr);
 	if (page) {
-		if (page == arena->scratch_page)
-			/* BPF triggered scratch here; don't lazy-alloc over it */
-			goto out_sigsegv;
+		if (page == arena->scratch_page) {
+			/*
+			 * A scratch page marks a hole. Segfault only if the user
+			 * asked for it; otherwise we could lazy-allocate but
+			 * choose not to over a hole, so report a bus error.
+			 */
+			fault_ret = (arena->map.map_flags & BPF_F_SEGV_ON_FAULT) ?
+				    VM_FAULT_SIGSEGV : VM_FAULT_SIGBUS;
+			goto out_err_locked;
+		}
 		/* already have a page vmap-ed */
 		goto out;
 	}
 
+	if (arena->map.map_flags & BPF_F_SEGV_ON_FAULT) {
+		/* User space requested to segfault when page is not allocated by bpf prog */
+		fault_ret = VM_FAULT_SIGSEGV;
+		goto out_err_locked;
+	}
+
 	bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
 
-	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;
+	if (!new_page) {
+		/*
+		 * Very rare race: the bpf program had allocated a page here, so
+		 * the lockless probe saw it and we skipped preallocation, but it
+		 * freed the page before we took the lock. Now we do need one;
+		 * sleeping is not allowed here, so fall back to the non-blocking
+		 * allocator and give up if it fails.
+		 */
+		ret = bpf_map_alloc_pages(map, map->numa_node, 1, &new_page);
+		if (ret) {
+			fault_ret = VM_FAULT_SIGBUS;
+			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_SIGBUS;
+		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_SIGBUS;
+		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;
+	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 v2 3/4] selftests/bpf: Add read_cgroup_file() to cgroup_helpers

[Jiayuan Chen]

cgroup_helpers has write_cgroup_file()/write_cgroup_file_parent() but no
read counterpart. Add read_cgroup_file() and read_cgroup_file_parent() so
a forked child can read a cgroup file (e.g. memory.current) from the work
dir owned by the parent that set the environment up, without hand-building
the /mnt/... path.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 tools/testing/selftests/bpf/cgroup_helpers.c | 67 ++++++++++++++++++++
 tools/testing/selftests/bpf/cgroup_helpers.h |  4 ++
 2 files changed, 71 insertions(+)

diff --git a/tools/testing/selftests/bpf/cgroup_helpers.c b/tools/testing/selftests/bpf/cgroup_helpers.c
index 45cd0b479fe3..4183ff6150c2 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.c
+++ b/tools/testing/selftests/bpf/cgroup_helpers.c
@@ -188,6 +188,73 @@ int write_cgroup_file_parent(const char *relative_path, const char *file,
 	return __write_cgroup_file(cgroup_path, file, buf);
 }
 
+static int __read_cgroup_file(const char *cgroup_path, const char *file,
+			      char *buf, size_t len)
+{
+	char file_path[PATH_MAX + 1];
+	ssize_t got;
+	int fd;
+
+	snprintf(file_path, sizeof(file_path), "%s/%s", cgroup_path, file);
+	fd = open(file_path, O_RDONLY);
+	if (fd < 0) {
+		log_err("Opening %s", file_path);
+		return 1;
+	}
+
+	got = read(fd, buf, len - 1);
+	if (got < 0) {
+		log_err("Reading %s", file_path);
+		close(fd);
+		return 1;
+	}
+	buf[got] = '\0';
+	close(fd);
+	return 0;
+}
+
+/**
+ * read_cgroup_file() - Read from a cgroup file
+ * @relative_path: The cgroup path, relative to the workdir
+ * @file: The name of the file in cgroupfs to read from
+ * @buf: Buffer to read into, NUL-terminated on success
+ * @len: Size of @buf
+ *
+ * Read from a file in the given cgroup's directory.
+ *
+ * If successful, 0 is returned.
+ */
+int read_cgroup_file(const char *relative_path, const char *file,
+		     char *buf, size_t len)
+{
+	char cgroup_path[PATH_MAX - 24];
+
+	format_cgroup_path(cgroup_path, relative_path);
+	return __read_cgroup_file(cgroup_path, file, buf, len);
+}
+
+/**
+ * read_cgroup_file_parent() - Read from a cgroup file in the parent process
+ *                             workdir
+ * @relative_path: The cgroup path, relative to the parent process workdir
+ * @file: The name of the file in cgroupfs to read from
+ * @buf: Buffer to read into, NUL-terminated on success
+ * @len: Size of @buf
+ *
+ * Read from a file in the given cgroup's directory under the parent process
+ * workdir.
+ *
+ * If successful, 0 is returned.
+ */
+int read_cgroup_file_parent(const char *relative_path, const char *file,
+			    char *buf, size_t len)
+{
+	char cgroup_path[PATH_MAX - 24];
+
+	format_parent_cgroup_path(cgroup_path, relative_path);
+	return __read_cgroup_file(cgroup_path, file, buf, len);
+}
+
 /**
  * setup_cgroup_environment() - Setup the cgroup environment
  *
diff --git a/tools/testing/selftests/bpf/cgroup_helpers.h b/tools/testing/selftests/bpf/cgroup_helpers.h
index 3857304be874..d42d2e13044e 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.h
+++ b/tools/testing/selftests/bpf/cgroup_helpers.h
@@ -15,6 +15,10 @@ int write_cgroup_file(const char *relative_path, const char *file,
 		      const char *buf);
 int write_cgroup_file_parent(const char *relative_path, const char *file,
 			     const char *buf);
+int read_cgroup_file(const char *relative_path, const char *file,
+		     char *buf, size_t len);
+int read_cgroup_file_parent(const char *relative_path, const char *file,
+			    char *buf, size_t len);
 int cgroup_setup_and_join(const char *relative_path);
 int get_root_cgroup(void);
 int create_and_get_cgroup(const char *relative_path);
-- 
2.43.0

[PATCH bpf-next v2 4/4] 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    | 139 ++++++++++++++++++
 .../testing/selftests/bpf/progs/arena_memcg.c |  24 +++
 2 files changed, 163 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..ca039ebd3d67
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/arena_memcg.c
@@ -0,0 +1,139 @@
+// 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)
+
+static void dump_memcg(int (*rd)(const char *, const char *, char *, size_t))
+{
+	char buf[512];
+
+	/*
+	 * memory.current reads 0 once the child has left the cgroup, so it only
+	 * carries information when dumped from the live child; memory.peak and
+	 * memory.events survive the child and tell the story either way.
+	 */
+	if (!rd(CG_PATH, "memory.current", buf, sizeof(buf)))
+		fprintf(stderr, "memory.current: %s", buf);
+	if (!rd(CG_PATH, "memory.max", buf, sizeof(buf)))
+		fprintf(stderr, "memory.max: %s", buf);
+	if (!rd(CG_PATH, "memory.peak", buf, sizeof(buf)))
+		fprintf(stderr, "memory.peak: %s", buf);
+	if (!rd(CG_PATH, "memory.events", buf, sizeof(buf)))
+		fprintf(stderr, "memory.events:\n%s", buf);
+	fflush(NULL); /* _exit() in the child would not flush stdio otherwise */
+}
+
+void serial_test_arena_memcg(void)
+{
+	int cgroup_fd = -1, status;
+	const long ps = PAGE_SIZE;
+	char buf[64];
+	pid_t pid;
+
+	if (setup_cgroup_environment())
+		return;
+
+	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 (read_cgroup_file(CG_PATH, "memory.current", buf, sizeof(buf))) {
+		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 *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. The cgroup work dir belongs to the parent that set
+		 * the environment up, so reach it with the _parent() helpers.
+		 * 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 (write_cgroup_file_parent(CG_PATH, "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.
+		 */
+		if (read_cgroup_file_parent(CG_PATH, "memory.current", buf, sizeof(buf)))
+			_exit(5);
+		cur = strtol(buf, NULL, 10);
+		snprintf(buf, sizeof(buf), "%ld", cur + ARENA_BUDGET);
+		if (write_cgroup_file_parent(CG_PATH, "memory.max", buf))
+			_exit(6);
+
+		for (i = 0; i < npages; i++)
+			base[(size_t)i * ps] = 1;
+		/* Faulted everything without dying: no pressure built, dump why. */
+		dump_memcg(read_cgroup_file_parent);
+		_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.
+	 */
+	if (!ASSERT_TRUE(WIFSIGNALED(status), "child killed by signal"))
+		goto out;
+	if (!ASSERT_NEQ(WTERMSIG(status), SIGSEGV, "not killed by SIGSEGV"))
+		dump_memcg(read_cgroup_file);
+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 v2 4/4] selftests/bpf: Add a test for arena fault-in under memory.max

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The test passes on unexpected crashes because it only asserts that the termination signal is not SIGSEGV, rather than strictly expecting SIGKILL from the memcg OOM killer.
--

--- Patch [4]: [PATCH bpf-next v2 4/4] selftests/bpf: Add a test for arena fault-in under memory.max ---
commit d5e6a0b924679b20cf7f391518ec1a560ae725e3
Author: Jiayuan Chen <jiayuan.chen@linux.dev>

selftests/bpf: Add a test for arena fault-in under memory.max

This commit adds a BPF selftest for arena fault-in under a constrained memory
cgroup. It creates a child process to fault in an arena and verifies it is
killed by the memcg OOM killer rather than incorrectly dying from SIGSEGV.

> 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 0000000000000..ca039ebd3d674
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/arena_memcg.c
[ ... ]
> +void serial_test_arena_memcg(void)
> +{
[ ... ]
> +	/*
> +	 * 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.
> +	 */
> +	if (!ASSERT_TRUE(WIFSIGNALED(status), "child killed by signal"))
> +		goto out;
> +	if (!ASSERT_NEQ(WTERMSIG(status), SIGSEGV, "not killed by SIGSEGV"))
> +		dump_memcg(read_cgroup_file);

[Severity: Medium]
Could this mask other failures by accepting any signal other than SIGSEGV?

If the child process crashes due to an unrelated issue like a libbpf internal
assertion (SIGABRT) or an invalid memory access (SIGBUS), this assertion would
evaluate to true and the test would silently pass.

Should this explicitly expect SIGKILL from the memcg OOM killer using
ASSERT_EQ(WTERMSIG(status), SIGKILL)?

> +out:
> +	if (cgroup_fd >= 0)
> +		close(cgroup_fd);
> +	cleanup_cgroup_environment();
> +}

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

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

[sashiko-bot]

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

New issues:
- [High] Using __GFP_ZERO to zero pages intended for user-space mapping skips required dcache flushes on cache-aliasing architectures.
- [Medium] bpf_map_alloc_page_sleepable() and __bpf_alloc_page() silently bypass task/VMA NUMA mempolicy when the requested node is NUMA_NO_NODE.

Pre-existing issues:
- [High] The fallback allocation path in __bpf_alloc_page() omits __GFP_ZERO, returning uninitialized kernel memory that can be exposed to user space.
--

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

bpf: Add a sleepable page allocator for map memory

This patch adds bpf_map_alloc_page_sleepable() to provide a blocking page
allocator for sleepable contexts. This avoids using the non-blocking
allocator, which never reclaims or engages the OOM machinery, for contexts
that can actually sleep.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 8d111da886553..67d8157c6623e 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);

[Severity: High]
This is a pre-existing issue, but does this fallback path leak uninitialized
kernel memory? 

The call to alloc_pages_nolock() passes only __GFP_ACCOUNT and completely
omits __GFP_ZERO. If a BPF map is allocated on PREEMPT_RT where
can_alloc_pages() returns false, this could expose uninitialized pages to
user space.

>  
> -	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]
When nid is NUMA_NO_NODE, does alloc_pages_node() bypass the task's NUMA
mempolicy?

Since alloc_pages_node() automatically resolves NUMA_NO_NODE to the local
CPU node rather than defaulting to the task's mempolicy, this could cause
pages to land on the wrong nodes for user applications.

>  }
>  
[ ... ]
> @@ -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)
> +{
> +	might_sleep();
> +	return alloc_pages_node(map->numa_node, BPF_PAGE_GFP, 0);

[Severity: High]
Does using __GFP_ZERO (via BPF_PAGE_GFP) skip required dcache flushes on
cache-aliasing architectures? 

When allocating pages destined for user-space mapping (like the arena page
fault handler), __GFP_ZERO uses clear_page() which leaves zeros in the
kernel's dcache. Could user processes reading from these pages see stale,
uninitialized physical memory instead of zeros?

[Severity: Medium]
Similar to the issue in __bpf_alloc_page(), does using alloc_pages_node()
here bypass the task mempolicy when map->numa_node is NUMA_NO_NODE?

> +}
> +

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

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

[Jiayuan Chen]

On 8/5/26 5:32 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [High] Using __GFP_ZERO to zero pages intended for user-space mapping skips required dcache flushes on cache-aliasing architectures.
> - [Medium] bpf_map_alloc_page_sleepable() and __bpf_alloc_page() silently bypass task/VMA NUMA mempolicy when the requested node is NUMA_NO_NODE.
>
> Pre-existing issues:
> - [High] The fallback allocation path in __bpf_alloc_page() omits __GFP_ZERO, returning uninitialized kernel memory that can be exposed to user space.
> --
>
> --- Patch [1]: [PATCH bpf-next v2 1/4] bpf: Add a sleepable page allocator for map memory ---
> commit 2a4210c942ca919834ce20218ab8bedc6cbb40a8
> Author: Jiayuan Chen <jiayuan.chen@linux.dev>
>
> bpf: Add a sleepable page allocator for map memory
>
> This patch adds bpf_map_alloc_page_sleepable() to provide a blocking page
> allocator for sleepable contexts. This avoids using the non-blocking
> allocator, which never reclaims or engages the OOM machinery, for contexts
> that can actually sleep.
>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 8d111da886553..67d8157c6623e 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);
> [Severity: High]
> This is a pre-existing issue, but does this fallback path leak uninitialized
> kernel memory?
>
> The call to alloc_pages_nolock() passes only __GFP_ACCOUNT and completely
> omits __GFP_ZERO. If a BPF map is allocated on PREEMPT_RT where
> can_alloc_pages() returns false, this could expose uninitialized pages to
> user space.


Not ture.

alloc_pages_nolock::alloc_frozen_pages_nolock_noprof append __GFP_ZERO 
to alloc_gfp by default.


>>   
>> -	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]
> When nid is NUMA_NO_NODE, does alloc_pages_node() bypass the task's NUMA
> mempolicy?
>
> Since alloc_pages_node() automatically resolves NUMA_NO_NODE to the local
> CPU node rather than defaulting to the task's mempolicy, this could cause
> pages to land on the wrong nodes for user applications.


Already replied to v1.

This matches every other bpf map allocation via alloc_pages_node(); none 
honor the task mempolicy.

>>   }
>>   
> [ ... ]
>> @@ -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)
>> +{
>> +	might_sleep();
>> +	return alloc_pages_node(map->numa_node, BPF_PAGE_GFP, 0);
> [Severity: High]
> Does using __GFP_ZERO (via BPF_PAGE_GFP) skip required dcache flushes on
> cache-aliasing architectures?
>
> When allocating pages destined for user-space mapping (like the arena page
> fault handler), __GFP_ZERO uses clear_page() which leaves zeros in the
> kernel's dcache. Could user processes reading from these pages see stale,
> uninitialized physical memory instead of zeros?


The patch doesn't change zeroing, we already used  __GFP_ZERO before.


I asked claude to check the review and it told me the review was 
overthinking:

     no arena-capable arch selects ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE, so 
flush_dcache_page()
     is a no-op and there's no aliasing concern.

But I think this needs to be remembered.


> [Severity: Medium]
> Similar to the issue in __bpf_alloc_page(), does using alloc_pages_node()
> here bypass the task mempolicy when map->numa_node is NUMA_NO_NODE?
>
>> +}
>> +

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

[Jiayuan Chen]

On 8/5/26 5:29 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The test passes on unexpected crashes because it only asserts that the termination signal is not SIGSEGV, rather than strictly expecting SIGKILL from the memcg OOM killer.
> --
>
> --- Patch [4]: [PATCH bpf-next v2 4/4] selftests/bpf: Add a test for arena fault-in under memory.max ---
> commit d5e6a0b924679b20cf7f391518ec1a560ae725e3
> Author: Jiayuan Chen <jiayuan.chen@linux.dev>
>
> selftests/bpf: Add a test for arena fault-in under memory.max
>
> This commit adds a BPF selftest for arena fault-in under a constrained memory
> cgroup. It creates a child process to fault in an arena and verifies it is
> killed by the memcg OOM killer rather than incorrectly dying from SIGSEGV.
>
>> 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 0000000000000..ca039ebd3d674
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/arena_memcg.c
> [ ... ]
>> +void serial_test_arena_memcg(void)
>> +{
> [ ... ]
>> +	/*
>> +	 * 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.
>> +	 */
>> +	if (!ASSERT_TRUE(WIFSIGNALED(status), "child killed by signal"))
>> +		goto out;
>> +	if (!ASSERT_NEQ(WTERMSIG(status), SIGSEGV, "not killed by SIGSEGV"))
>> +		dump_memcg(read_cgroup_file);
> [Severity: Medium]
> Could this mask other failures by accepting any signal other than SIGSEGV?
>
> If the child process crashes due to an unrelated issue like a libbpf internal
> assertion (SIGABRT) or an invalid memory access (SIGBUS), this assertion would
> evaluate to true and the test would silently pass.
>
> Should this explicitly expect SIGKILL from the memcg OOM killer using
> ASSERT_EQ(WTERMSIG(status), SIGKILL)?


I think checking "oom_kill" in memory.events would be more accurate.


That said, I'm not sure patches 3 and 4 are necessary, they only 
exercise the memcg behavior,

so we could drop them when merging (if the series is accepted).


Both ok for me.