netdev - [RFC PATCH net-next 7/9] net: delay the removal of the name nodes until run

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]

Date:   Tue, 28 Sep 2021 14:54:58 +0200
From:   Antoine Tenart <atenart@...nel.org>
To:     davem@...emloft.net, kuba@...nel.org
Cc:     Antoine Tenart <atenart@...nel.org>, pabeni@...hat.com,
        gregkh@...uxfoundation.org, ebiederm@...ssion.com,
        stephen@...workplumber.org, herbert@...dor.apana.org.au,
        juri.lelli@...hat.com, netdev@...r.kernel.org
Subject: [RFC PATCH net-next 7/9] net: delay the removal of the name nodes until run_todo

Keep the node name collision detection working until the last
registration stage, by delaying the removal of the name nodes in
run_todo. This allows to perform unregistration operations being
sensitive to name collisions, in run_todo. As run_todo has sections of
code running without the rtnl lock taken, this will allow to perform
some of those operations not under this lock (when possible).

While we move the removal of the name node until a late unregistration
stage, we still want to avoid returning a net device reference when it's
being unregistered (calling __dev_get_by_name for example). We keep this
logic by setting the node name dev reference to NULL. This follows the
logic of __dev_get_by_name. Altnames are in the same list, they are not
special here.

>From now on we have to be strict on the use of __dev_get_by_name vs
netdev_name_node_lookup. One is designed to get the device, the other
one to lookup in the list of currently reserved names. Current users
should have been fixed by previous patches.

One side effect is there is now a window between unregistering the
netdevice and running the todo where names are still reserved and can't
be used for new device creation.

Signed-off-by: Antoine Tenart <atenart@...nel.org>
---
 net/core/dev.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 02f9d505dbe2..a1eab120bb50 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10611,10 +10611,15 @@ void netdev_run_todo(void)
 		if (dev->needs_free_netdev)
 			free_netdev(dev);

-		/* Report a network device has been unregistered */
 		rtnl_lock();
+		unlist_netdevice_name(dev);
+		synchronize_net();
+		netdev_name_node_free(dev->name_node);
+
 		dev_net(dev)->dev_unreg_count--;
 		__rtnl_unlock();
+
+		/* Report a network device has been unregistered */
 		wake_up(&netdev_unregistering_wq);

 		/* Free network device */
@@ -11039,7 +11044,12 @@ void unregister_netdevice_many(struct list_head *head)
 	list_for_each_entry(dev, head, unreg_list) {
 		/* And unlink it from device chain. */
 		unlist_netdevice(dev);
-		unlist_netdevice_name(dev);
+
+		/* Unreference the net device from the node name. From this
+		 * point on the node name is only used for naming collision
+		 * detection.
+		 */
+		dev->name_node->dev = NULL;

 		dev->reg_state = NETREG_UNREGISTERING;
 	}
@@ -11072,7 +11082,6 @@ void unregister_netdevice_many(struct list_head *head)
 		dev_mc_flush(dev);

 		netdev_name_node_alt_flush(dev);
-		netdev_name_node_free(dev->name_node);

 		if (dev->netdev_ops->ndo_uninit)
 			dev->netdev_ops->ndo_uninit(dev);
-- 
2.31.1