net: make netdev_lock() protect netdev->reg_state
authorJakub Kicinski <kuba@kernel.org>
Wed, 15 Jan 2025 03:53:10 +0000 (19:53 -0800)
committerJakub Kicinski <kuba@kernel.org>
Thu, 16 Jan 2025 03:13:33 +0000 (19:13 -0800)
Protect writes to netdev->reg_state with netdev_lock().
From now on holding netdev_lock() is sufficient to prevent
the net_device from getting unregistered, so code which
wants to hold just a single netdev around no longer needs
to hold rtnl_lock.

We do not protect the NETREG_UNREGISTERED -> NETREG_RELEASED
transition. We'd need to move mutex_destroy(netdev->lock)
to .release, but the real reason is that trying to stop
the unregistration process mid-way would be unsafe / crazy.
Taking references on such devices is not safe, either.
So the intended semantics are to lock REGISTERED devices.

Reviewed-by: Joe Damato <jdamato@fastly.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250115035319.559603-3-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
include/linux/netdevice.h
net/core/dev.c

index 6d440db35d5f3b47a5af4028a635a6f9e36f696b..007bcfa383c9ba1fd8bc31cca032ed4a4ea0df0b 100644 (file)
@@ -2448,7 +2448,7 @@ struct net_device {
         * Should always be taken using netdev_lock() / netdev_unlock() helpers.
         * Drivers are free to use it for other protection.
         *
-        * Protects: @net_shaper_hierarchy.
+        * Protects: @reg_state, @net_shaper_hierarchy.
         * Ordering: take after rtnl_lock.
         */
        struct mutex            lock;
index 47e6b0f73cfc790b55252c554e6f194eb5d1fdfc..bbe6fb9e32cdf1aa808a201ba7d309c25d131cd1 100644 (file)
@@ -10695,7 +10695,9 @@ int register_netdevice(struct net_device *dev)
 
        ret = netdev_register_kobject(dev);
 
+       netdev_lock(dev);
        WRITE_ONCE(dev->reg_state, ret ? NETREG_UNREGISTERED : NETREG_REGISTERED);
+       netdev_unlock(dev);
 
        if (ret)
                goto err_uninit_notify;
@@ -10969,7 +10971,9 @@ void netdev_run_todo(void)
                        continue;
                }
 
+               netdev_lock(dev);
                WRITE_ONCE(dev->reg_state, NETREG_UNREGISTERED);
+               netdev_unlock(dev);
                linkwatch_sync_dev(dev);
        }
 
@@ -11575,7 +11579,9 @@ void unregister_netdevice_many_notify(struct list_head *head,
        list_for_each_entry(dev, head, unreg_list) {
                /* And unlink it from device chain. */
                unlist_netdevice(dev);
+               netdev_lock(dev);
                WRITE_ONCE(dev->reg_state, NETREG_UNREGISTERING);
+               netdev_unlock(dev);
        }
        flush_all_backlogs();