netdev - Re: [PATCH net-next v2 08/11] net: make NETDEV_UNREGISTER and instance lock more consistent

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0be167ba394a9485c78c67d187ec546131a5cbe1.camel@nvidia.com>
Date: Tue, 25 Mar 2025 12:17:15 +0000
From: Cosmin Ratiu <cratiu@...dia.com>
To: "kuba@...nel.org" <kuba@...nel.org>, "davem@...emloft.net"
	<davem@...emloft.net>, "sdf@...ichev.me" <sdf@...ichev.me>
CC: "horms@...nel.org" <horms@...nel.org>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "pabeni@...hat.com"
	<pabeni@...hat.com>
Subject: Re: [PATCH net-next v2 08/11] net: make NETDEV_UNREGISTER and
 instance lock more consistent

On Mon, 2025-03-24 at 15:45 -0700, Jakub Kicinski wrote:
> The NETDEV_UNREGISTER notifier gets called under the ops lock
> when device changes namespace but not during real unregistration.
> Take it consistently, XSK tries to poke at netdev queue state
> from this notifier.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  net/core/dev.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 652f2c6f5674..7bd8bd82f66f 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1848,7 +1848,9 @@ static void
> call_netdevice_unregister_notifiers(struct notifier_block *nb,
>  					dev);
>  		call_netdevice_notifier(nb, NETDEV_DOWN, dev);
>  	}
> +	netdev_lock_ops(dev);
>  	call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev);
> +	netdev_unlock_ops(dev);
>  }

This introduces a potential deadlock when changing a device namespace:
do_setlink already locks the instance lock and
call_netdevice_unregister_notifiers will then deadlock.

============================================
WARNING: possible recursive locking detected
6.14.0-rc6+ #143 Not tainted
--------------------------------------------
ip/2459 is trying to acquire lock:
ffff888113ca8c80 (&dev->lock){+.+.}-{4:4}, at:
call_netdevice_unregister_notifiers+0x73/0x110

but task is already holding lock:
ffff888155a10c80 (&dev->lock){+.+.}-{4:4}, at:
do_setlink.isra.0+0x5b/0x1220

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

       CPU0
       ----
  lock(&dev->lock);
  lock(&dev->lock);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

2 locks held by ip/2459:
 #0: ffffffff82df18c8 (rtnl_mutex){+.+.}-{4:4}, at:
rtnl_newlink+0x35d/0xb50
 #1: ffff888155a10c80 (&dev->lock){+.+.}-{4:4}, at:
do_setlink.isra.0+0x5b/0x1220

Call Trace:
 <TASK>
 dump_stack_lvl+0x62/0x90
 print_deadlock_bug+0x274/0x3b0
 __lock_acquire+0x1229/0x2470
 lock_acquire+0xb7/0x2b0
 __mutex_lock+0xa6/0xd20
 call_netdevice_unregister_notifiers+0x73/0x110
 netif_change_net_namespace+0x4b7/0xa90
 do_setlink.isra.0+0xd5/0x1220
 rtnl_newlink+0x7ea/0xb50
 rtnetlink_rcv_msg+0x459/0x5e0
 netlink_rcv_skb+0x54/0x100
 netlink_unicast+0x193/0x270
 netlink_sendmsg+0x204/0x450
 <snip>