Re: [PATCH bpf-next] bpf: arena: fix mmap_lock deadlock on arena lock failure
Jiayuan Chen <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On 7/28/26 2:23 PM, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > > Pre-existing issues: > - [High] BPF arena page faults can spuriously crash the user process with SIGSEGV due to transient lockless allocation failures. > -- > > --- Patch [1]: [PATCH bpf-next] bpf: arena: fix mmap_lock deadlock on arena lock failure --- > commit c57623f4077e743e8c901306fcec7c67d2aafc17 > Author: Jiayuan Chen <[email protected]> > > bpf: arena: fix mmap_lock deadlock on arena lock failure > > This commit fixes a potential deadlock in the arena page fault handler by > returning VM_FAULT_SIGBUS instead of VM_FAULT_RETRY when the arena spinlock > cannot be acquired. Returning retry would cause the fault path to attempt > retaking the mmap_lock, which was never dropped. > >> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c >> --- a/kernel/bpf/arena.c >> +++ b/kernel/bpf/arena.c >> @@ -490,8 +490,12 @@ static vm_fault_t arena_vm_fault(struct vm_fault *vmf) >> kaddr = kbase + (u32)(vmf->address); >> >> if (raw_res_spin_lock_irqsave(&arena->spinlock, flags)) >> - /* Make a reasonable effort to address impossible case */ >> - return VM_FAULT_RETRY; >> + /* >> + * 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. >> + */ >> + return VM_FAULT_SIGBUS; > [Severity: High] > This is a pre-existing issue, but looking further down in arena_vm_fault(), > does a transient allocation failure unexpectedly cause a SIGSEGV? > > Since the raw spinlock is held, disabling IRQs, bpf_map_alloc_pages() > falls back to an opportunistic lockless allocation which can fail under > memory pressure: > > 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; > } > > When this lockless allocation fails, the error path jumps to > out_sigsegv_memcg and returns VM_FAULT_SIGSEGV instead of VM_FAULT_OOM or > safely dropping the lock to retry. > > Could this cause user processes mapping a BPF arena to experience spurious > crashes during memory contention? Yes I'm on it https://lore.kernel.org/bpf/[email protected]/ And current patch is from your review result of that patch...