[PATCH net] net: gro_cells: fix lock imbalance in gro_cells_receive()

[PATCH net] net: gro_cells: fix lock imbalance in gro_cells_receive()

[Eric Dumazet]

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: 25718fdcbdd2 ("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>
---
 net/core/gro_cells.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/core/gro_cells.c b/net/core/gro_cells.c
index b43911562f4d10aa3d05c60f343ff89c5d9ed58d..fd57b845de333ff0e397eeb95aa67926d4e4a730 100644
--- a/net/core/gro_cells.c
+++ b/net/core/gro_cells.c
@@ -43,12 +43,11 @@ int gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)
 	if (skb_queue_len(&cell->napi_skbs) == 1)
 		napi_schedule(&cell->napi);
 
-	if (have_bh_lock)
-		local_unlock_nested_bh(&gcells->cells->bh_lock);
-
 	res = NET_RX_SUCCESS;
 
 unlock:
+	if (have_bh_lock)
+		local_unlock_nested_bh(&gcells->cells->bh_lock);
 	rcu_read_unlock();
 	return res;
 }
-- 
2.51.0.858.gf9c4a03a3a-goog

Re: [PATCH net] net: gro_cells: fix lock imbalance in gro_cells_receive()

[David Ahern]

On 10/20/25 10:11 AM, Eric Dumazet wrote:
> 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: 25718fdcbdd2 ("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>
> ---
>  net/core/gro_cells.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net] net: gro_cells: fix lock imbalance in gro_cells_receive()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 20 Oct 2025 16:11:14 +0000 you wrote:
> 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
> 
> [...]

Here is the summary with links:
  - [net] net: gro_cells: fix lock imbalance in gro_cells_receive()
    https://git.kernel.org/netdev/net/c/c5394b8b7a92

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net] net: gro_cells: fix lock imbalance in gro_cells_receive()

[Sebastian Andrzej Siewior]

On 2025-10-20 16:11:14 [+0000], Eric Dumazet wrote:
> syzbot found that the local_unlock_nested_bh() call was
> missing in some cases.

Thank you Eric.

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

Indeed that was the intention. It is even noted in the diffstat that
this the reason the variable.

Sebastian