Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/vmalloc: Bail out if in softirq context
@ 2026-10-26 10:25 Uladzislau Rezki (Sony)
  2026-10-26 16:04 ` Andrew Morton
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Uladzislau Rezki (Sony) @ 2026-10-26 10:25 UTC (permalink / raw)
  To: linux-mm, Andrew Morton
  Cc: Baoquan He, LKML, Uladzislau Rezki, David Hildenbrand (Arm)

vmalloc() is not allowed to be invoked from the NMI,
hard-irq and softirq contexts because doing so may
deadlock on internal locks.

Commit 04aa71da5f35 ("mm/vmalloc: do not trigger BUG()
on BH disabled context") replaced the deprecated in_interrupt()
macro with explicit in_nmi() and in_hardirq() checks, but
unintentionally dropped the softirq context check.

Example:

<snip>
vzalloc() in a task
 spin_lock(&vn->busy.lock)
  <------ Hard irq
   irq_exit()
    process softirq on the same CPU
     vmalloc()
      spin_lock(&vn->busy.lock)
       spins forever
<snip>

Restore the check by checking the caller's context with
the in_serving_softirq() helper. Note that, it is still
allowed to invoke vmalloc() with disabled bottom halves.

Replace BUG_ON() with WARN_ON_ONCE() for in_nmi() and
in_hardirq() case.

Cc: David Hildenbrand (Arm) <david@kernel.org>
Fixes: 04aa71da5f35 ("mm/vmalloc: do not trigger BUG() on BH disabled context")
Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 mm/vmalloc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 7521b9b3bfd2..36e6e3be6ba4 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3252,7 +3252,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;
+
 	size = ALIGN(size, 1ul << shift);
 	if (unlikely(!size))
 		return NULL;
-- 
2.47.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm/vmalloc: Bail out if in softirq context
  2026-10-26 10:25 [PATCH] mm/vmalloc: Bail out if in softirq context Uladzislau Rezki (Sony)
@ 2026-10-26 16:04 ` Andrew Morton
  2026-10-26 17:39   ` Uladzislau Rezki
  2026-10-27  0:57 ` Baoquan He
  2026-10-27 10:25 ` sashiko-bot
  2 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2026-10-26 16:04 UTC (permalink / raw)
  To: Uladzislau Rezki (Sony)
  Cc: linux-mm, Baoquan He, LKML, David Hildenbrand (Arm)

On Mon, 26 Oct 2026 11:25:03 +0100 "Uladzislau Rezki (Sony)" <urezki@gmail.com> wrote:

> vmalloc() is not allowed to be invoked from the NMI,
> hard-irq and softirq contexts because doing so may
> deadlock on internal locks.
> 
> Commit 04aa71da5f35 ("mm/vmalloc: do not trigger BUG()
> on BH disabled context") replaced the deprecated in_interrupt()
> macro with explicit in_nmi() and in_hardirq() checks, but
> unintentionally dropped the softirq context check.
> 
> Example:
> 
> <snip>
> vzalloc() in a task
>  spin_lock(&vn->busy.lock)
>   <------ Hard irq
>    irq_exit()
>     process softirq on the same CPU
>      vmalloc()
>       spin_lock(&vn->busy.lock)
>        spins forever
> <snip>
> 
> Restore the check by checking the caller's context with
> the in_serving_softirq() helper. Note that, it is still
> allowed to invoke vmalloc() with disabled bottom halves.
> 
> Replace BUG_ON() with WARN_ON_ONCE() for in_nmi() and
> in_hardirq() case.
> 
> Cc: David Hildenbrand (Arm) <david@kernel.org>
> Fixes: 04aa71da5f35 ("mm/vmalloc: do not trigger BUG() on BH disabled context")

Sounds like a cc:stable is needed?

> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  mm/vmalloc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 7521b9b3bfd2..36e6e3be6ba4 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3252,7 +3252,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;

WARN does a lot of stuff and I rather doubt that it's all nmi-safe.  I
don't think this matters much - it'll probably work out and as long as
*something* gets printed, it did its job.

>  	size = ALIGN(size, 1ul << shift);
>  	if (unlikely(!size))
>  		return NULL;
> -- 
> 2.47.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm/vmalloc: Bail out if in softirq context
  2026-10-26 16:04 ` Andrew Morton
@ 2026-10-26 17:39   ` Uladzislau Rezki
  0 siblings, 0 replies; 7+ messages in thread
From: Uladzislau Rezki @ 2026-10-26 17:39 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Uladzislau Rezki (Sony), linux-mm, Baoquan He, LKML,
	David Hildenbrand (Arm)

On Mon, Oct 26, 2026 at 09:04:21AM -0700, Andrew Morton wrote:
> On Mon, 26 Oct 2026 11:25:03 +0100 "Uladzislau Rezki (Sony)" <urezki@gmail.com> wrote:
> 
> > vmalloc() is not allowed to be invoked from the NMI,
> > hard-irq and softirq contexts because doing so may
> > deadlock on internal locks.
> > 
> > Commit 04aa71da5f35 ("mm/vmalloc: do not trigger BUG()
> > on BH disabled context") replaced the deprecated in_interrupt()
> > macro with explicit in_nmi() and in_hardirq() checks, but
> > unintentionally dropped the softirq context check.
> > 
> > Example:
> > 
> > <snip>
> > vzalloc() in a task
> >  spin_lock(&vn->busy.lock)
> >   <------ Hard irq
> >    irq_exit()
> >     process softirq on the same CPU
> >      vmalloc()
> >       spin_lock(&vn->busy.lock)
> >        spins forever
> > <snip>
> > 
> > Restore the check by checking the caller's context with
> > the in_serving_softirq() helper. Note that, it is still
> > allowed to invoke vmalloc() with disabled bottom halves.
> > 
> > Replace BUG_ON() with WARN_ON_ONCE() for in_nmi() and
> > in_hardirq() case.
> > 
> > Cc: David Hildenbrand (Arm) <david@kernel.org>
> > Fixes: 04aa71da5f35 ("mm/vmalloc: do not trigger BUG() on BH disabled context")
> 
> Sounds like a cc:stable is needed?
> 
Yes. Should i resend and put cc:stable?

> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > ---
> >  mm/vmalloc.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index 7521b9b3bfd2..36e6e3be6ba4 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -3252,7 +3252,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;
> 
> WARN does a lot of stuff and I rather doubt that it's all nmi-safe.  I
> don't think this matters much - it'll probably work out and as long as
> *something* gets printed, it did its job.
> 
Yep. I checked before the kernel sources and saw that people use it
with WARN. I thought switching from BUG() to WARN() is better.

--
Uladzislau Rezki


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm/vmalloc: Bail out if in softirq context
  2026-10-26 10:25 [PATCH] mm/vmalloc: Bail out if in softirq context Uladzislau Rezki (Sony)
  2026-10-26 16:04 ` Andrew Morton
@ 2026-10-27  0:57 ` Baoquan He
  2026-10-27 10:25 ` sashiko-bot
  2 siblings, 0 replies; 7+ messages in thread
From: Baoquan He @ 2026-10-27  0:57 UTC (permalink / raw)
  To: Uladzislau Rezki (Sony)
  Cc: linux-mm, Andrew Morton, LKML, David Hildenbrand (Arm)

On 10/26/26 at 11:25am, Uladzislau Rezki (Sony) wrote:
> vmalloc() is not allowed to be invoked from the NMI,
> hard-irq and softirq contexts because doing so may
> deadlock on internal locks.
> 
> Commit 04aa71da5f35 ("mm/vmalloc: do not trigger BUG()
> on BH disabled context") replaced the deprecated in_interrupt()
> macro with explicit in_nmi() and in_hardirq() checks, but
> unintentionally dropped the softirq context check.
> 
> Example:
> 
> <snip>
> vzalloc() in a task
>  spin_lock(&vn->busy.lock)
>   <------ Hard irq
>    irq_exit()
>     process softirq on the same CPU
>      vmalloc()
>       spin_lock(&vn->busy.lock)
>        spins forever
> <snip>
> 
> Restore the check by checking the caller's context with
> the in_serving_softirq() helper. Note that, it is still
> allowed to invoke vmalloc() with disabled bottom halves.
> 
> Replace BUG_ON() with WARN_ON_ONCE() for in_nmi() and
> in_hardirq() case.
> 
> Cc: David Hildenbrand (Arm) <david@kernel.org>
> Fixes: 04aa71da5f35 ("mm/vmalloc: do not trigger BUG() on BH disabled context")
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  mm/vmalloc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 7521b9b3bfd2..36e6e3be6ba4 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3252,7 +3252,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;
> +

LGTM,
Reviewed-by: Baoquan He <baoquan.he@linux.dev>

>  	size = ALIGN(size, 1ul << shift);
>  	if (unlikely(!size))
>  		return NULL;
> -- 
> 2.47.3
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm/vmalloc: Bail out if in softirq context
  2026-10-26 10:25 [PATCH] mm/vmalloc: Bail out if in softirq context Uladzislau Rezki (Sony)
  2026-10-26 16:04 ` Andrew Morton
  2026-10-27  0:57 ` Baoquan He
@ 2026-10-27 10:25 ` sashiko-bot
  2026-10-27 13:14   ` Uladzislau Rezki
  2026-10-27 17:43   ` Uladzislau Rezki
  2 siblings, 2 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-27 10:25 UTC (permalink / raw)
  To: Uladzislau Rezki (Sony), linux-mm, Andrew Morton
  Cc: Baoquan He, David Hildenbrand (Arm)

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.

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?

[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).
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.
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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026102503.30860-1-urezki@gmail.com?part=1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm/vmalloc: Bail out if in softirq context
  2026-10-27 10:25 ` sashiko-bot
@ 2026-10-27 13:14   ` Uladzislau Rezki
  2026-10-27 17:43   ` Uladzislau Rezki
  1 sibling, 0 replies; 7+ messages in thread
From: Uladzislau Rezki @ 2026-10-27 13:14 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Uladzislau Rezki (Sony), linux-mm, Andrew Morton, Baoquan He,
	David Hildenbrand (Arm)

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm/vmalloc: Bail out if in softirq context
  2026-10-27 10:25 ` sashiko-bot
  2026-10-27 13:14   ` Uladzislau Rezki
@ 2026-10-27 17:43   ` Uladzislau Rezki
  1 sibling, 0 replies; 7+ messages in thread
From: Uladzislau Rezki @ 2026-10-27 17:43 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Uladzislau Rezki (Sony), linux-mm, Andrew Morton, Baoquan He,
	David Hildenbrand (Arm)

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?
> 
CPU_0           CPU_1
 vmalloc()       lock(B)
  lock(A)         vmalloc()
   soft-irq        lock(A)
    lock(B)

Such callers have to be fixed at their call sites. __get_vm_area_node()
cannot detect ABBA deadlock which involves a lock held by a task-context
caller.

--
Uladzislau Rezki


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-27 17:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-26 10:25 [PATCH] mm/vmalloc: Bail out if in softirq context Uladzislau Rezki (Sony)
2026-10-26 16:04 ` Andrew Morton
2026-10-26 17:39   ` Uladzislau Rezki
2026-10-27  0:57 ` Baoquan He
2026-10-27 10:25 ` sashiko-bot
2026-10-27 13:14   ` Uladzislau Rezki
2026-10-27 17:43   ` Uladzislau Rezki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox