> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 7b6847200b43..fa462a0ff1ad 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
[ ... ]
> @@ -489,59 +490,106 @@ static vm_fault_t arena_vm_fault(struct vm_fault *vmf)
> kbase = bpf_arena_get_kern_vm_start(arena);
> kaddr = kbase + (u32)(vmf->address);
>
> + page = vmalloc_to_page((void *)kaddr);
> + if (!page && !(arena->map.map_flags & BPF_F_SEGV_ON_FAULT)) {
> + /*
> + * Preallocate outside the lock with a sleepable allocator so it
> + * can reclaim and run the memcg OOM killer, which the
> + * non-blocking allocator under arena->spinlock cannot. A NULL
> + * return is non-recoverable, so fail with VM_FAULT_SIGBUS;
> + * VM_FAULT_OOM would be retried by the fault path and can
> + * livelock when the charged memcg is not the faulting task's.
> + */
> + 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_SIGBUS;
> + }
> +
> + if (raw_res_spin_lock_irqsave(&arena->spinlock, flags)) {
[ ... ]
> page = vmalloc_to_page((void *)kaddr);
> if (page) {
> + 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;
> + }
This isn't a bug, but could this comment spell out why a scratch-marked
hole is not lazy-allocated even though an untouched hole is? A reader might
wonder why the two cases are treated differently since both are equally
free as far as the range tree is concerned.
> /* 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 (!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);
This isn't a bug, but would it be clearer to describe the condition
generically (the lockless probe's result was invalidated before the lock
was taken) rather than naming only the bpf-alloc-then-free case? The
fallback is also reached if the probe observed arena->scratch_page and the
PTE was cleared by arena_free_pages() before the lock was taken, which is
not a bpf program page allocation.
[ ... ]
---
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/32713810092
