]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
netpoll: Fix deadlock in memory allocation under spinlock
authorBreno Leitao <leitao@debian.org>
Mon, 3 Nov 2025 16:38:17 +0000 (08:38 -0800)
committerJakub Kicinski <kuba@kernel.org>
Wed, 5 Nov 2025 03:17:00 +0000 (19:17 -0800)
Fix a AA deadlock in refill_skbs() where memory allocation while holding
skb_pool->lock can trigger a recursive lock acquisition attempt.

The deadlock scenario occurs when the system is under severe memory
pressure:

1. refill_skbs() acquires skb_pool->lock (spinlock)
2. alloc_skb() is called while holding the lock
3. Memory allocator fails and calls slab_out_of_memory()
4. This triggers printk() for the OOM warning
5. The console output path calls netpoll_send_udp()
6. netpoll_send_udp() attempts to acquire the same skb_pool->lock
7. Deadlock: the lock is already held by the same CPU

Call stack:
  refill_skbs()
    spin_lock_irqsave(&skb_pool->lock)    <- lock acquired
    __alloc_skb()
      kmem_cache_alloc_node_noprof()
        slab_out_of_memory()
          printk()
            console_flush_all()
              netpoll_send_udp()
                skb_dequeue()
                  spin_lock_irqsave(&skb_pool->lock)     <- deadlock attempt

This bug was exposed by commit 248f6571fd4c51 ("netpoll: Optimize skb
refilling on critical path") which removed refill_skbs() from the
critical path (where nested printk was being deferred), letting nested
printk being called from inside refill_skbs()

Refactor refill_skbs() to never allocate memory while holding
the spinlock.

Another possible solution to fix this problem is protecting the
refill_skbs() from nested printks, basically calling
printk_deferred_{enter,exit}() in refill_skbs(), then, any nested
pr_warn() would be deferred.

I prefer this approach, given I _think_ it might be a good idea to move
the alloc_skb() from GFP_ATOMIC to GFP_KERNEL in the future, so, having
the alloc_skb() outside of the lock will be necessary step.

There is a possible TOCTOU issue when checking for the pool length, and
queueing the new allocated skb, but, this is not an issue, given that
an extra SKB in the pool is harmless and it will be eventually used.

Signed-off-by: Breno Leitao <leitao@debian.org>
Fixes: 248f6571fd4c51 ("netpoll: Optimize skb refilling on critical path")
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20251103-fix_netpoll_aa-v4-1-4cfecdf6da7c@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
net/core/netpoll.c

index 60a05d3b7c2491096f79ea6cf82eeef222c3eac2..c85f740065fc6ec281762e50db0e242da7fa8d2d 100644 (file)
@@ -228,19 +228,16 @@ static void refill_skbs(struct netpoll *np)
 {
        struct sk_buff_head *skb_pool;
        struct sk_buff *skb;
-       unsigned long flags;
 
        skb_pool = &np->skb_pool;
 
-       spin_lock_irqsave(&skb_pool->lock, flags);
-       while (skb_pool->qlen < MAX_SKBS) {
+       while (READ_ONCE(skb_pool->qlen) < MAX_SKBS) {
                skb = alloc_skb(MAX_SKB_SIZE, GFP_ATOMIC);
                if (!skb)
                        break;
 
-               __skb_queue_tail(skb_pool, skb);
+               skb_queue_tail(skb_pool, skb);
        }
-       spin_unlock_irqrestore(&skb_pool->lock, flags);
 }
 
 static void zap_completion_queue(void)