On Sat Aug 8, 2026 at 10:05 AM EDT, Jiayuan Chen wrote: > 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 <[email protected]>
Reviewed-by: Emil Tsalapatis <[email protected]> > --- > 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 = {
