Re: [PATCH] mm/vmalloc: Bail out if in softirq context

[Uladzislau Rezki <urezki@gmail.com>]

On Tue, Oct 27, 2026 at 10:25:23AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] mm/vmalloc: ABBA deadlock when calling vmalloc() with BH or IRQs disabled
> - [Medium] mm/vmalloc: __get_vm_area_node() irq bailout triggers warnings and retry
> --
> 
> commit 9fea18279139941aa247a1a577dd5e101116871a
> Author: Uladzislau Rezki (Sony) <urezki@gmail.com>
> 
> mm/vmalloc: Bail out if in softirq context
> 
> This commit restores a softirq check in __get_vm_area_node() using
> in_serving_softirq() so that vmalloc() bails out when invoked from softirq
> context while still permitting callers with disabled bottom halves. It also
> replaces BUG_ON(in_nmi() || in_hardirq()) with a WARN_ON_ONCE() check that
> returns NULL.
> 
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index 44e0eb32708f1..5a949525b9db8 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -3271,7 +3271,13 @@ struct vm_struct *__get_vm_area_node(unsigned long size,
> >  	struct vm_struct *area;
> >  	unsigned long requested_size = size;
> >  
> > -	BUG_ON(in_nmi() || in_hardirq());
> > +	/* IRQ re-entrancy -> irq_exit() -> process softirq. */
> > +	if (in_serving_softirq())
> > +		return NULL;
> > +
> > +	if (WARN_ON_ONCE(in_nmi() || in_hardirq()))
> > +		return NULL;
> 
> [Severity: High]
> Can allowing vmalloc() in task context with bottom halves or interrupts
> disabled still cause a multi-CPU ABBA deadlock on vmalloc's internal locks?
> 
> In mm/vmalloc.c, alloc_vmap_area() acquires vn->pool_lock,
> free_vmap_area_lock, and vn->busy.lock (and __pte_alloc_kernel() in
> mm/memory.c acquires init_mm.page_table_lock) with plain spin_lock(), leaving
> bottom halves and hardware interrupts enabled:
> 
> mm/vmalloc.c:alloc_vmap_area() {
>     ...
> 	spin_lock(&vn->busy.lock);
> 	insert_vmap_area(va, &vn->busy.root, &vn->busy.head);
> 	spin_unlock(&vn->busy.lock);
>     ...
> }
> 
> Because __get_vm_area_node() only checks in_serving_softirq(), in_nmi(), and
> in_hardirq(), a task-context caller holding a lock via spin_lock_bh() (which
> sets SOFTIRQ_DISABLE_OFFSET rather than SOFTIRQ_OFFSET) or
> spin_lock_irqsave() (where irqs_disabled() is true while in_hardirq() is
> false) will proceed into alloc_vmap_area().
> 
> Using the bridge FDB path referenced by commit 04aa71da5f35 ("mm/vmalloc: do
> not trigger BUG() on BH disabled context"):
> 
> net/bridge/br_fdb.c:br_fdb_add_local() {
>     ...
> 	spin_lock_bh(&br->hash_lock);
> 	ret = fdb_add_local(br, source, addr, vid);
> 	spin_unlock_bh(&br->hash_lock);
>     ...
> }
> 
> net/bridge/br_fdb.c:br_fdb_update() {
>     ...
> 	} else {
> 		spin_lock(&br->hash_lock);
> 		fdb = fdb_create(br, source, addr, vid, flags);
>     ...
> }
> 
> If CPU 0 is in task context inside alloc_vmap_area() holding
> spin_lock(&vn->busy.lock) or spin_lock(&free_vmap_area_lock), and CPU 1 is in
> task context inside br_fdb_add_local() holding spin_lock_bh(&br->hash_lock):
> 
> 1. A hardware interrupt arrives on CPU 0, and on irq_exit() NET_RX_SOFTIRQ
>    runs br_fdb_update(), which spins on spin_lock(&br->hash_lock) before ever
>    reaching fdb_create() or __get_vm_area_node().
>
> 2. CPU 1 proceeds through
>    fdb_add_local() -> fdb_create() -> rhashtable_lookup_insert_fast() ->
>    rhashtable_insert_rehash() -> bucket_table_alloc(GFP_ATOMIC | __GFP_NOWARN)
>    -> kvmalloc() -> vmalloc() -> __get_vm_area_node().
> 3. Because in_serving_softirq() is false on CPU 1, it enters
>    alloc_vmap_area() and spins on free_vmap_area_lock or vn->busy.lock held
>    by CPU 0.
> 
My patch fixes self-deadlock on re-entry. 

<snip>
CPU 0:
 task
  vmalloc()
   lock(X)
     IRQ
      irq_exit()
       softirq
        vmalloc()
         lock(X) -> self-deadlock
<snip>

and we used to bail out in this case. So it just restores previous
behaviour which forbids re-entry.

> Similarly, a task-context caller holding a lock with spin_lock_irqsave() can
> enter alloc_vmap_area() with irqs_disabled() and invert lock ordering against
> any hardirq handler acquiring that same lock (and sleep with interrupts
> disabled on CONFIG_PREEMPT_RT).
> 
> Should __get_vm_area_node() also bail out when in_softirq() or
> irqs_disabled() is true?
> 
I think it is another patch:

    if (irqs_disabled() && !early_boot_irqs_disabled)
        return NULL;

> [Severity: Medium]
> When __get_vm_area_node() returns NULL due to in_serving_softirq(), in_nmi(),
> or in_hardirq(), how does __vmalloc_node_range_noprof() handle the NULL
> return?
> 
> Looking at __vmalloc_node_range_noprof() in mm/vmalloc.c:
> 
> mm/vmalloc.c:__vmalloc_node_range_noprof() {
>     ...
> 	area = __get_vm_area_node(real_size, align, shift, VM_ALLOC |
> 				  VM_UNINITIALIZED | vm_flags, start, end, node,
> 				  gfp_mask, caller);
> 	if (!area) {
> 		bool nofail = gfp_mask & __GFP_NOFAIL;
> 		warn_alloc(gfp_mask, NULL,
> 			"vmalloc error: size %lu, align 0x%lx, vm_struct allocation failed%s",
> 			size, align, (nofail) ? ". Retrying." : "");
> 		if (nofail) {
> 			schedule_timeout_uninterruptible(1);
> 			goto again;
> 		}
> 		goto fail;
> 	}
>     ...
> }
> 
> Since commit c6307674ed82 ("mm: kvmalloc: add non-blocking support for
> vmalloc"), __kvmalloc_node_noprof() in mm/slub.c falls back to
> __vmalloc_node_range_noprof() for non-blocking allocations (such as
> GFP_ATOMIC or GFP_NOWAIT) when size > PAGE_SIZE and __do_kmalloc_node()
> fails, while kmalloc_gfp_adjust() only adds __GFP_NOWARN to the kmalloc
> attempt:
> 
> mm/slub.c:__kvmalloc_node_noprof() {
>     ...
> 	return __vmalloc_node_range_noprof(size, align, VMALLOC_START, VMALLOC_END,
> 			flags, PAGE_KERNEL, allow_block ? VM_ALLOW_HUGE_VMAP:0,
> 			node, __builtin_return_address(0));
> }
> 
> Doesn't bailing out in __get_vm_area_node() lead to a few issues in callers:
> 
> 1. In softirq, hardirq, or NMI context, when !area is reached in
>    __vmalloc_node_range_noprof(), it unconditionally calls warn_alloc()
>    whenever __GFP_NOWARN was not passed by the caller (and if shift >
>    PAGE_SHIFT, goto fail resets shift = PAGE_SHIFT and jumps to goto again,
>    repeating the call and warning a second time).
>
Calling vmalloc() from nmi, irq-context or on re-entry is a wrong behaviour.

> 2. If a caller in hardirq or NMI context invokes
>    kvmalloc(size, GFP_ATOMIC | __GFP_NOWARN) (for example,
>    rhashtable_insert_rehash() -> bucket_table_alloc() in lib/rhashtable.c,
>    reachable from irq-safe rhashtable users and BPF_MAP_TYPE_RHASHTABLE
>    updates in rhtab_map_update_elem() in kernel/bpf/hashtab.c) and the
>    kmalloc attempt fails for size > PAGE_SIZE, __kvmalloc_node_noprof() falls
>    back to __vmalloc_node_range_noprof() -> __get_vm_area_node() and triggers
>    WARN_ON_ONCE(in_nmi() || in_hardirq()) even though the caller passed
>    GFP_ATOMIC | __GFP_NOWARN and handles NULL.
>
Caller is not allowed to invoke vmalloc() from NMI or hard-irq context.

> 3. In __vmalloc_node_range_noprof(), nofail is not cleared when
>    !gfpflags_allow_blocking(gfp_mask) (unlike in __vmalloc_area_node()), so
>    if __GFP_NOFAIL is set when __get_vm_area_node() returns NULL in
>    interrupt context, it will call schedule_timeout_uninterruptible(1) in
>    atomic context and loop indefinitely at goto again.
> 
__GFP_NOFAIL and NOWAIT or ATOMIC can not coexist together:

 * %__GFP_NOFAIL: The VM implementation _must_ retry infinitely: the caller
 * cannot handle allocation failures. The allocation could block
 * indefinitely but will never return with failure. Testing for
 * failure is pointless.
 * It _must_ be blockable and used together with __GFP_DIRECT_RECLAIM.

--
Uladzislau Rezki