Skip to content

Commit

Permalink
net: hold instance lock during NETDEV_REGISTER/UP
Browse files Browse the repository at this point in the history 
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: ad7c7b2 ("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>
  • Loading branch information
Stanislav Fomichev authored and Jakub Kicinski committed Apr 3, 2025
1 parent d2ccd05 commit 4c975fd
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 15 deletions.
2 changes: 1 addition & 1 deletion include/linux/netdevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 9 additions & 3 deletions net/core/dev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand Down
8 changes: 1 addition & 7 deletions net/core/dev_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
8 changes: 4 additions & 4 deletions net/core/rtnetlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit 4c975fd

Please sign in to comment.