netdev - Re: [PATCH net-next] net: let lockdep compare instance locks

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20250516101441.5ad5b722@kernel.org>
Date: Fri, 16 May 2025 10:14:41 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: <andrew+netdev@...n.ch>, <davem@...emloft.net>, <edumazet@...gle.com>,
 <horms@...nel.org>, <netdev@...r.kernel.org>, <pabeni@...hat.com>,
 <sdf@...ichev.me>
Subject: Re: [PATCH net-next] net: let lockdep compare instance locks

On Fri, 16 May 2025 08:22:43 -0700 Jakub Kicinski wrote:
> On Thu, 15 May 2025 19:59:41 -0700 Kuniyuki Iwashima wrote:
> > > Is the thinking that once the big rtnl lock disappears in cleanup_net
> > > the devices are safe to destroy without any locking because there can't
> > > be any live users trying to access them?    
> > 
> > I hope yes, but removing VF via sysfs and removing netns might
> > race and need some locking ?  
> 
> I think we should take the small lock around default_device_exit_net()
> and then we'd be safe? Either a given VF gets moved to init_net first
> or the sysfs gets to it and unregisters it safely in the old netns.

Thinking about it some more, we'll have to revisit this problem before
removing the big lock, anyway. I'm leaning towards doing this for now:

diff --git a/include/net/netdev_lock.h b/include/net/netdev_lock.h
index 2a753813f849..c345afecd4c5 100644
--- a/include/net/netdev_lock.h
+++ b/include/net/netdev_lock.h
@@ -99,16 +99,15 @@ static inline void netdev_unlock_ops_compat(struct net_device *dev)
 static inline int netdev_lock_cmp_fn(const struct lockdep_map *a,
 				     const struct lockdep_map *b)
 {
-	/* Only lower devices currently grab the instance lock, so no
-	 * real ordering issues can occur. In the near future, only
-	 * hardware devices will grab instance lock which also does not
-	 * involve any ordering. Suppress lockdep ordering warnings
-	 * until (if) we start grabbing instance lock on pure SW
-	 * devices (bond/team/veth/etc).
-	 */
 	if (a == b)
 		return 0;
-	return -1;
+
+	/* Allow locking multiple devices only under rtnl_lock,
+	 * the exact order doesn't matter.
+	 * Note that upper devices don't lock their ops, so nesting
+	 * mostly happens during batched device removal for now.
+	 */
+	return lockdep_rtnl_is_held() ? -1 : 1;
 }
 
 #define netdev_lockdep_set_classes(dev)				\