net: avoid potential race between netdev_get_by_index_lock() and netns switch

netdev_get_by_index_lock() performs following steps:

  rcu_lock();
  dev = lookup(netns, ifindex);
  dev_get(dev);
  rcu_unlock();
  [... lock & validate the dev ...]
  return dev

Validation right now only checks if the device is registered but since
the lookup is netns-aware we must also protect against the device
switching netns right after we dropped the RCU lock. Otherwise
the caller in netns1 may get a pointer to a device which has just
switched to netns2.

We can't hold the lock for the entire netns change process (because of
the NETDEV_UNREGISTER notifier), and there's no existing marking to
indicate that the netns is unlisted because of netns move, so add one.

AFAIU none of the existing netdev_get_by_index_lock() callers can
suffer from this problem (NAPI code double checks the netns membership
and other callers are either under rtnl_lock or not ns-sensitive),
so this patch does not have to be treated as a fix.

Reviewed-by: Joe Damato <jdamato@fastly.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20250408195956.412733-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cf3b6445817bb9d3a142da10549ade1c49659313..8e9be80bc1670a9ba89bfab74e043dc6afb6c684 100644 (file)
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1952,6 +1952,7 @@ enum netdev_reg_state {
  *     @priv_destructor:       Called from unregister
  *     @npinfo:                XXX: need comments on this one
  *     @nd_net:                Network namespace this network device is inside
+ *                             protected by @lock
  *
  *     @ml_priv:       Mid-layer private
  *     @ml_priv_type:  Mid-layer private type
@@ -2359,6 +2360,9 @@ struct net_device {
 
        bool dismantle;
 
+       /** @moving_ns: device is changing netns, protected by @lock */
+       bool moving_ns;
+
        enum {
                RTNL_LINK_INITIALIZED,
                RTNL_LINK_INITIALIZING,
@@ -2521,7 +2525,7 @@ struct net_device {
         *      @net_shaper_hierarchy, @reg_state, @threaded
         *
         * Double protects:
-        *      @up
+        *      @up, @moving_ns, @nd_net
         *
         * Double ops protects:
         *      @real_num_rx_queues, @real_num_tx_queues

diff --git a/net/core/dev.c b/net/core/dev.c
index 4ccc6dc5303e8e15f5dca65856aad5fdfd8aae17..8c13e5339cc98a638e59ae4fa8a76668433a4585 100644 (file)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -828,7 +828,7 @@ netdev_napi_by_id_lock(struct net *net, unsigned int napi_id)
        dev_hold(dev);
        rcu_read_unlock();
 
-       dev = __netdev_put_lock(dev);
+       dev = __netdev_put_lock(dev, net);
        if (!dev)
                return NULL;
 
@@ -1039,10 +1039,11 @@ struct net_device *dev_get_by_napi_id(unsigned int napi_id)
  * This helper is intended for locking net_device after it has been looked up
  * using a lockless lookup helper. Lock prevents the instance from going away.
  */
-struct net_device *__netdev_put_lock(struct net_device *dev)
+struct net_device *__netdev_put_lock(struct net_device *dev, struct net *net)
 {
        netdev_lock(dev);
-       if (dev->reg_state > NETREG_REGISTERED) {
+       if (dev->reg_state > NETREG_REGISTERED ||
+           dev->moving_ns || !net_eq(dev_net(dev), net)) {
                netdev_unlock(dev);
                dev_put(dev);
                return NULL;
@@ -1070,7 +1071,7 @@ struct net_device *netdev_get_by_index_lock(struct net *net, int ifindex)
        if (!dev)
                return NULL;
 
-       return __netdev_put_lock(dev);
+       return __netdev_put_lock(dev, net);
 }
 
 struct net_device *
@@ -1090,7 +1091,7 @@ netdev_xa_find_lock(struct net *net, struct net_device *dev,
                dev_hold(dev);
                rcu_read_unlock();
 
-               dev = __netdev_put_lock(dev);
+               dev = __netdev_put_lock(dev, net);
                if (dev)
                        return dev;
 
@@ -12157,7 +12158,11 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
        netif_close(dev);
        /* And unlink it from device chain */
        unlist_netdevice(dev);
-       netdev_unlock_ops(dev);
+
+       if (!netdev_need_ops_lock(dev))
+               netdev_lock(dev);
+       dev->moving_ns = true;
+       netdev_unlock(dev);
 
        synchronize_net();
 
@@ -12193,7 +12198,9 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
        move_netdevice_notifiers_dev_net(dev, net);
 
        /* Actually switch the network namespace */
+       netdev_lock(dev);
        dev_net_set(dev, net);
+       netdev_unlock(dev);
        dev->ifindex = new_ifindex;
 
        if (new_name[0]) {
@@ -12219,7 +12226,11 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
        err = netdev_change_owner(dev, net_old, net);
        WARN_ON(err);
 
-       netdev_lock_ops(dev);
+       netdev_lock(dev);
+       dev->moving_ns = false;
+       if (!netdev_need_ops_lock(dev))
+               netdev_unlock(dev);
+
        /* Add the device back in the hashes */
        list_netdevice(dev);
        /* Notify protocols, that a new device appeared. */

diff --git a/net/core/dev.h b/net/core/dev.h
index 710abc05ebdb0da9fface4a60692778ec8ff634e..e7bf21f52fc7639b7c9382a09da484a8ed71c3ec 100644 (file)
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -30,7 +30,7 @@ netdev_napi_by_id_lock(struct net *net, unsigned int napi_id);
 struct net_device *dev_get_by_napi_id(unsigned int napi_id);
 
 struct net_device *netdev_get_by_index_lock(struct net *net, int ifindex);
-struct net_device *__netdev_put_lock(struct net_device *dev);
+struct net_device *__netdev_put_lock(struct net_device *dev, struct net *net);
 struct net_device *
 netdev_xa_find_lock(struct net *net, struct net_device *dev,
                    unsigned long *index);