Files
linux/net/core/gro_cells.c
Eric Dumazet c5394b8b7a net: gro_cells: fix lock imbalance in gro_cells_receive()
syzbot found that the local_unlock_nested_bh() call was
missing in some cases.

WARNING: possible recursive locking detected
syzkaller #0 Not tainted
--------------------------------------------
syz.2.329/7421 is trying to acquire lock:
 ffffe8ffffd48888 ((&cell->bh_lock)){+...}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:44 [inline]
 ffffe8ffffd48888 ((&cell->bh_lock)){+...}-{3:3}, at: gro_cells_receive+0x404/0x790 net/core/gro_cells.c:30

but task is already holding lock:
 ffffe8ffffd48888 ((&cell->bh_lock)){+...}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:44 [inline]
 ffffe8ffffd48888 ((&cell->bh_lock)){+...}-{3:3}, at: gro_cells_receive+0x404/0x790 net/core/gro_cells.c:30

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock((&cell->bh_lock));
  lock((&cell->bh_lock));

 *** DEADLOCK ***

Given the introduction of @have_bh_lock variable, it seems the author
intent was to have the local_unlock_nested_bh() after the @unlock label.

Fixes: 25718fdcbd ("net: gro_cells: Use nested-BH locking for gro_cell")
Reported-by: syzbot+f9651b9a8212e1c8906f@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/68f65eb9.a70a0220.205af.0034.GAE@google.com/T/#u
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: David Ahern <dsahern@kernel.org>
Link: https://patch.msgid.link/20251020161114.1891141-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2025-10-21 17:41:09 -07:00

149 lines
3.5 KiB
C

// SPDX-License-Identifier: GPL-2.0
#include <linux/skbuff.h>
#include <linux/slab.h>
#include <linux/netdevice.h>
#include <net/gro_cells.h>
#include <net/hotdata.h>
struct gro_cell {
struct sk_buff_head napi_skbs;
struct napi_struct napi;
local_lock_t bh_lock;
};
int gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)
{
struct net_device *dev = skb->dev;
bool have_bh_lock = false;
struct gro_cell *cell;
int res;
rcu_read_lock();
if (unlikely(!(dev->flags & IFF_UP)))
goto drop;
if (!gcells->cells || skb_cloned(skb) || netif_elide_gro(dev)) {
res = netif_rx(skb);
goto unlock;
}
local_lock_nested_bh(&gcells->cells->bh_lock);
have_bh_lock = true;
cell = this_cpu_ptr(gcells->cells);
if (skb_queue_len(&cell->napi_skbs) > READ_ONCE(net_hotdata.max_backlog)) {
drop:
dev_core_stats_rx_dropped_inc(dev);
kfree_skb(skb);
res = NET_RX_DROP;
goto unlock;
}
__skb_queue_tail(&cell->napi_skbs, skb);
if (skb_queue_len(&cell->napi_skbs) == 1)
napi_schedule(&cell->napi);
res = NET_RX_SUCCESS;
unlock:
if (have_bh_lock)
local_unlock_nested_bh(&gcells->cells->bh_lock);
rcu_read_unlock();
return res;
}
EXPORT_SYMBOL(gro_cells_receive);
/* called under BH context */
static int gro_cell_poll(struct napi_struct *napi, int budget)
{
struct gro_cell *cell = container_of(napi, struct gro_cell, napi);
struct sk_buff *skb;
int work_done = 0;
__local_lock_nested_bh(&cell->bh_lock);
while (work_done < budget) {
skb = __skb_dequeue(&cell->napi_skbs);
if (!skb)
break;
napi_gro_receive(napi, skb);
work_done++;
}
if (work_done < budget)
napi_complete_done(napi, work_done);
__local_unlock_nested_bh(&cell->bh_lock);
return work_done;
}
int gro_cells_init(struct gro_cells *gcells, struct net_device *dev)
{
int i;
gcells->cells = alloc_percpu(struct gro_cell);
if (!gcells->cells)
return -ENOMEM;
for_each_possible_cpu(i) {
struct gro_cell *cell = per_cpu_ptr(gcells->cells, i);
__skb_queue_head_init(&cell->napi_skbs);
local_lock_init(&cell->bh_lock);
set_bit(NAPI_STATE_NO_BUSY_POLL, &cell->napi.state);
netif_napi_add(dev, &cell->napi, gro_cell_poll);
napi_enable(&cell->napi);
}
return 0;
}
EXPORT_SYMBOL(gro_cells_init);
struct percpu_free_defer {
struct rcu_head rcu;
void __percpu *ptr;
};
static void percpu_free_defer_callback(struct rcu_head *head)
{
struct percpu_free_defer *defer;
defer = container_of(head, struct percpu_free_defer, rcu);
free_percpu(defer->ptr);
kfree(defer);
}
void gro_cells_destroy(struct gro_cells *gcells)
{
struct percpu_free_defer *defer;
int i;
if (!gcells->cells)
return;
for_each_possible_cpu(i) {
struct gro_cell *cell = per_cpu_ptr(gcells->cells, i);
napi_disable(&cell->napi);
__netif_napi_del(&cell->napi);
__skb_queue_purge(&cell->napi_skbs);
}
/* We need to observe an rcu grace period before freeing ->cells,
* because netpoll could access dev->napi_list under rcu protection.
* Try hard using call_rcu() instead of synchronize_rcu(),
* because we might be called from cleanup_net(), and we
* definitely do not want to block this critical task.
*/
defer = kmalloc(sizeof(*defer), GFP_KERNEL | __GFP_NOWARN);
if (likely(defer)) {
defer->ptr = gcells->cells;
call_rcu(&defer->rcu, percpu_free_defer_callback);
} else {
/* We do not hold RTNL at this point, synchronize_net()
* would not be able to expedite this sync.
*/
synchronize_rcu_expedited();
free_percpu(gcells->cells);
}
gcells->cells = NULL;
}
EXPORT_SYMBOL(gro_cells_destroy);