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

On 8/21/26 1:59 PM, [email protected] wrote:
>> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
>> index 7b6847200b43..50dac9ba0f5e 100644
>> --- a/kernel/bpf/arena.c
>> +++ b/kernel/bpf/arena.c
> [ ... ]
>
>> @@ -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);
>>
>> +	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;
>> +	}
> Can this return VM_FAULT_OOM when the allocation is charged to a foreign
> memcg?
>
> The preallocation charges the arena creator's memcg, not current's:
>
>      bpf_map_memcg_enter(&arena->map, &old_memcg, &new_memcg);
>      /* set_active_memcg(bpf_map_get_memcg(map)) */
>
>      new_page = bpf_map_alloc_page_sleepable(map);
>      /* GFP_KERNEL|__GFP_ZERO|__GFP_ACCOUNT|__GFP_NOWARN */
>
> For an order-0 GFP_KERNEL allocation the only realistic NULL is a memcg
> charge rejection. mem_cgroup_oom() calls out_of_memory() with oom_control
> pointing at the arena owner's memcg, so the OOM killer can only select
> victims inside that cgroup.
>
> VM_FAULT_OOM reaches pagefault_out_of_memory() (mm/oom_kill.c:1180-1193),
> which only resolves current->memcg_in_oom; otherwise it prints the
> rate-limited "Huh VM_FAULT_OOM leaked out to the #PF handler. Retrying PF"
> message and returns. Because the over-limit memcg is foreign, killing
> tasks there never makes fatal_signal_pending(current) true, so
> try_charge_memcg()'s force-charge escape hatch never fires.
>
> If the owner memcg's charge is dominated by memory its tasks cannot free
> (for example the arena's own pages, which are only freed by the bpf program
> or by map destruction and are unreclaimable), or if that cgroup has no
> OOM-eligible tasks left, mem_cgroup_oom() returns false, try_charge_memcg()
> returns -ENOMEM, and the instruction is retried in a loop.
>
> This creates an unbounded fault/retry livelock burning CPU in the faulting
> task plus dmesg spam, where the pre-image returned a bounded
> VM_FAULT_SIGSEGV. Before this patch the in-lock allocator never engaged
> reclaim or the OOM killer, so a user touching an arena page could not kill
> anything. Now any task that can mmap the arena (the fd can be passed via
> SCM_RIGHTS or a bpffs pin, so the faulting task need not be in the owner's
> cgroup at all) can drive the memcg OOM killer in the owner's cgroup one
> task at a time simply by faulting in pages.


Thanks, you're right — this is a real bug, and I could reproduce it.

The root cause is simply that we return VM_FAULT_OOM.

That return value does nothing useful for arena: all reclaim and OOM 
handling (both memcg and global) already happens inside

bpf_map_alloc_page_sleepable().


The fix is to return VM_FAULT_SIGBUS instead of VM_FAULT_OOM.