]> www.infradead.org Git - nvme.git/commitdiff
net: hold instance lock during NETDEV_REGISTER/UP
authorStanislav Fomichev <sdf@fomichev.me>
Tue, 1 Apr 2025 16:34:43 +0000 (09:34 -0700)
committerJakub Kicinski <kuba@kernel.org>
Thu, 3 Apr 2025 22:32:08 +0000 (15:32 -0700)
Callers of inetdev_init can come from several places with inconsistent
expectation about netdev instance lock. Grab instance lock during
REGISTER (plus UP). Also solve the inconsistency with UNREGISTER
where it was locked only during move netns path.

WARNING: CPU: 10 PID: 1479 at ./include/net/netdev_lock.h:54
__netdev_update_features+0x65f/0xca0
__warn+0x81/0x180
__netdev_update_features+0x65f/0xca0
report_bug+0x156/0x180
handle_bug+0x4f/0x90
exc_invalid_op+0x13/0x60
asm_exc_invalid_op+0x16/0x20
__netdev_update_features+0x65f/0xca0
netif_disable_lro+0x30/0x1d0
inetdev_init+0x12f/0x1f0
inetdev_event+0x48b/0x870
notifier_call_chain+0x38/0xf0
register_netdevice+0x741/0x8b0
register_netdev+0x1f/0x40
mlx5e_probe+0x4e3/0x8e0 [mlx5_core]
auxiliary_bus_probe+0x3f/0x90
really_probe+0xc3/0x3a0
__driver_probe_device+0x80/0x150
driver_probe_device+0x1f/0x90
__device_attach_driver+0x7d/0x100
bus_for_each_drv+0x80/0xd0
__device_attach+0xb4/0x1c0
bus_probe_device+0x91/0xa0
device_add+0x657/0x870

Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Reported-by: Cosmin Ratiu <cratiu@nvidia.com>
Fixes: ad7c7b2172c3 ("net: hold netdev instance lock during sysfs operations")
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20250401163452.622454-3-sdf@fomichev.me
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
include/linux/netdevice.h
net/core/dev.c
net/core/dev_api.c
net/core/rtnetlink.c

index fa79145518d1eb845ada659165c2d95fad53d343..cf3b6445817bb9d3a142da10549ade1c49659313 100644 (file)
@@ -4192,7 +4192,7 @@ int dev_change_flags(struct net_device *dev, unsigned int flags,
 int netif_set_alias(struct net_device *dev, const char *alias, size_t len);
 int dev_set_alias(struct net_device *, const char *, size_t);
 int dev_get_alias(const struct net_device *, char *, size_t);
-int netif_change_net_namespace(struct net_device *dev, struct net *net,
+int __dev_change_net_namespace(struct net_device *dev, struct net *net,
                               const char *pat, int new_ifindex,
                               struct netlink_ext_ack *extack);
 int dev_change_net_namespace(struct net_device *dev, struct net *net,
index 5d20ff226d5e78a4e203bce7ad61db83c79675f7..ce6612106e60695e755b162bea895e33c88cbd1c 100644 (file)
@@ -1858,7 +1858,9 @@ static int call_netdevice_register_net_notifiers(struct notifier_block *nb,
        int err;
 
        for_each_netdev(net, dev) {
+               netdev_lock_ops(dev);
                err = call_netdevice_register_notifiers(nb, dev);
+               netdev_unlock_ops(dev);
                if (err)
                        goto rollback;
        }
@@ -11047,7 +11049,9 @@ int register_netdevice(struct net_device *dev)
                memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
 
        /* Notify protocols, that a new device appeared. */
+       netdev_lock_ops(dev);
        ret = call_netdevice_notifiers(NETDEV_REGISTER, dev);
+       netdev_unlock_ops(dev);
        ret = notifier_to_errno(ret);
        if (ret) {
                /* Expect explicit free_netdev() on failure */
@@ -12059,7 +12063,7 @@ void unregister_netdev(struct net_device *dev)
 }
 EXPORT_SYMBOL(unregister_netdev);
 
-int netif_change_net_namespace(struct net_device *dev, struct net *net,
+int __dev_change_net_namespace(struct net_device *dev, struct net *net,
                               const char *pat, int new_ifindex,
                               struct netlink_ext_ack *extack)
 {
@@ -12144,11 +12148,12 @@ int netif_change_net_namespace(struct net_device *dev, struct net *net,
         * And now a mini version of register_netdevice unregister_netdevice.
         */
 
+       netdev_lock_ops(dev);
        /* If device is running close it first. */
        netif_close(dev);
-
        /* And unlink it from device chain */
        unlist_netdevice(dev);
+       netdev_unlock_ops(dev);
 
        synchronize_net();
 
@@ -12210,11 +12215,12 @@ int netif_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);
        /* Add the device back in the hashes */
        list_netdevice(dev);
-
        /* Notify protocols, that a new device appeared. */
        call_netdevice_notifiers(NETDEV_REGISTER, dev);
+       netdev_unlock_ops(dev);
 
        /*
         *      Prevent userspace races by waiting until the network
index 8dbc6061210079f571713c6fbf456cba0baacf98..90bafb0b1b8c3f593c410e30b401ebd63c2524aa 100644 (file)
@@ -117,13 +117,7 @@ EXPORT_SYMBOL(dev_set_mac_address_user);
 int dev_change_net_namespace(struct net_device *dev, struct net *net,
                             const char *pat)
 {
-       int ret;
-
-       netdev_lock_ops(dev);
-       ret = netif_change_net_namespace(dev, net, pat, 0, NULL);
-       netdev_unlock_ops(dev);
-
-       return ret;
+       return __dev_change_net_namespace(dev, net, pat, 0, NULL);
 }
 EXPORT_SYMBOL_GPL(dev_change_net_namespace);
 
index 334db17be37d1b1ecf135b23d2142fdd1593d782..c238528350506d406b436fae7ed592d331e80e5f 100644 (file)
@@ -3025,8 +3025,6 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
        char ifname[IFNAMSIZ];
        int err;
 
-       netdev_lock_ops(dev);
-
        err = validate_linkmsg(dev, tb, extack);
        if (err < 0)
                goto errout;
@@ -3042,14 +3040,16 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
 
                new_ifindex = nla_get_s32_default(tb[IFLA_NEW_IFINDEX], 0);
 
-               err = netif_change_net_namespace(dev, tgt_net, pat,
+               err = __dev_change_net_namespace(dev, tgt_net, pat,
                                                 new_ifindex, extack);
                if (err)
-                       goto errout;
+                       return err;
 
                status |= DO_SETLINK_MODIFIED;
        }
 
+       netdev_lock_ops(dev);
+
        if (tb[IFLA_MAP]) {
                struct rtnl_link_ifmap *u_map;
                struct ifmap k_map;