From e361560a7912958ba3059f51e7dd21612d119169 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Wed, 15 Jan 2025 18:55:43 +0900 Subject: [PATCH 1/3] dev: Acquire netdev_rename_lock before restoring dev->name in dev_change_name(). The cited commit forgot to add netdev_rename_lock in one of the error paths in dev_change_name(). Let's hold netdev_rename_lock before restoring the old dev->name. Fixes: 0840556e5a3a ("net: Protect dev->name by seqlock.") Signed-off-by: Kuniyuki Iwashima Reviewed-by: Eric Dumazet Link: https://patch.msgid.link/20250115095545.52709-2-kuniyu@amazon.com Signed-off-by: Jakub Kicinski --- net/core/dev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/core/dev.c b/net/core/dev.c index 782ae3ff3f8d..cb6da0cac211 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1389,7 +1389,9 @@ int dev_change_name(struct net_device *dev, const char *newname) rollback: ret = device_rename(&dev->dev, dev->name); if (ret) { + write_seqlock_bh(&netdev_rename_lock); memcpy(dev->name, oldname, IFNAMSIZ); + write_sequnlock_bh(&netdev_rename_lock); WRITE_ONCE(dev->name_assign_type, old_assign_type); up_write(&devnet_rename_sem); return ret; From 2f1bb1e2cc0068b6f5fe992ed3108d89b9b29cf9 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Wed, 15 Jan 2025 18:55:44 +0900 Subject: [PATCH 2/3] dev: Remove devnet_rename_sem. devnet_rename_sem is no longer used since commit 0840556e5a3a ("net: Protect dev->name by seqlock."). Also, RTNL serialises dev_change_name(). Let's remove devnet_rename_sem. Signed-off-by: Kuniyuki Iwashima Reviewed-by: Eric Dumazet Link: https://patch.msgid.link/20250115095545.52709-3-kuniyu@amazon.com Signed-off-by: Jakub Kicinski --- net/core/dev.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index cb6da0cac211..32fcf54b779d 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -181,8 +181,6 @@ static DEFINE_SPINLOCK(napi_hash_lock); static unsigned int napi_gen_id = NR_CPUS; static DEFINE_READ_MOSTLY_HASHTABLE(napi_hash, 8); -static DECLARE_RWSEM(devnet_rename_sem); - static inline void dev_base_seq_inc(struct net *net) { unsigned int val = net->dev_base_seq + 1; @@ -1361,12 +1359,8 @@ int dev_change_name(struct net_device *dev, const char *newname) net = dev_net(dev); - down_write(&devnet_rename_sem); - - if (strncmp(newname, dev->name, IFNAMSIZ) == 0) { - up_write(&devnet_rename_sem); + if (!strncmp(newname, dev->name, IFNAMSIZ)) return 0; - } memcpy(oldname, dev->name, IFNAMSIZ); @@ -1374,10 +1368,8 @@ int dev_change_name(struct net_device *dev, const char *newname) err = dev_get_valid_name(net, dev, newname); write_sequnlock_bh(&netdev_rename_lock); - if (err < 0) { - up_write(&devnet_rename_sem); + if (err < 0) return err; - } if (oldname[0] && !strchr(oldname, '%')) netdev_info(dev, "renamed from %s%s\n", oldname, @@ -1393,12 +1385,9 @@ int dev_change_name(struct net_device *dev, const char *newname) memcpy(dev->name, oldname, IFNAMSIZ); write_sequnlock_bh(&netdev_rename_lock); WRITE_ONCE(dev->name_assign_type, old_assign_type); - up_write(&devnet_rename_sem); return ret; } - up_write(&devnet_rename_sem); - netdev_adjacent_rename_links(dev, oldname); netdev_name_node_del(dev->name_node); @@ -1414,7 +1403,6 @@ int dev_change_name(struct net_device *dev, const char *newname) /* err >= 0 after dev_alloc_name() or stores the first errno */ if (err >= 0) { err = ret; - down_write(&devnet_rename_sem); write_seqlock_bh(&netdev_rename_lock); memcpy(dev->name, oldname, IFNAMSIZ); write_sequnlock_bh(&netdev_rename_lock); From be94cfdb993ff091ce3b98a16b45795198957f95 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Wed, 15 Jan 2025 18:55:45 +0900 Subject: [PATCH 3/3] dev: Hold rtnl_net_lock() for dev_ifsioc(). Basically, dev_ifsioc() operates on the passed single netns (except for netdev notifier chains with lower/upper devices for which we will need more changes). Let's hold rtnl_net_lock() for dev_ifsioc(). Now that NETDEV_CHANGENAME is always triggered under rtnl_net_lock() of the device's netns. (do_setlink() and dev_ifsioc()) Signed-off-by: Kuniyuki Iwashima Reviewed-by: Eric Dumazet Link: https://patch.msgid.link/20250115095545.52709-4-kuniyu@amazon.com Signed-off-by: Jakub Kicinski --- net/core/dev.c | 7 ++----- net/core/dev_ioctl.c | 26 +++++++++++++++++--------- net/core/rtnl_net_debug.c | 15 +++------------ 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 32fcf54b779d..fe5f5855593d 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1348,16 +1348,13 @@ static int dev_get_valid_name(struct net *net, struct net_device *dev, */ int dev_change_name(struct net_device *dev, const char *newname) { + struct net *net = dev_net(dev); unsigned char old_assign_type; char oldname[IFNAMSIZ]; int err = 0; int ret; - struct net *net; - ASSERT_RTNL(); - BUG_ON(!dev_net(dev)); - - net = dev_net(dev); + ASSERT_RTNL_NET(net); if (!strncmp(newname, dev->name, IFNAMSIZ)) return 0; diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c index 087a57b7e4fa..4c2098ac9d72 100644 --- a/net/core/dev_ioctl.c +++ b/net/core/dev_ioctl.c @@ -543,7 +543,7 @@ static int dev_siocwandev(struct net_device *dev, struct if_settings *ifs) } /* - * Perform the SIOCxIFxxx calls, inside rtnl_lock() + * Perform the SIOCxIFxxx calls, inside rtnl_net_lock() */ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data, unsigned int cmd) @@ -620,11 +620,14 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data, return -ENODEV; if (!netif_is_bridge_master(dev)) return -EOPNOTSUPP; + netdev_hold(dev, &dev_tracker, GFP_KERNEL); - rtnl_unlock(); + rtnl_net_unlock(net); + err = br_ioctl_call(net, netdev_priv(dev), cmd, ifr, NULL); + netdev_put(dev, &dev_tracker); - rtnl_lock(); + rtnl_net_lock(net); return err; case SIOCDEVPRIVATE ... SIOCDEVPRIVATE + 15: @@ -770,9 +773,11 @@ int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr, dev_load(net, ifr->ifr_name); if (!ns_capable(net->user_ns, CAP_NET_ADMIN)) return -EPERM; - rtnl_lock(); + + rtnl_net_lock(net); ret = dev_ifsioc(net, ifr, data, cmd); - rtnl_unlock(); + rtnl_net_unlock(net); + if (colon) *colon = ':'; return ret; @@ -816,9 +821,11 @@ int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr, case SIOCBONDSLAVEINFOQUERY: case SIOCBONDINFOQUERY: dev_load(net, ifr->ifr_name); - rtnl_lock(); + + rtnl_net_lock(net); ret = dev_ifsioc(net, ifr, data, cmd); - rtnl_unlock(); + rtnl_net_unlock(net); + if (need_copyout) *need_copyout = false; return ret; @@ -841,9 +848,10 @@ int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr, (cmd >= SIOCDEVPRIVATE && cmd <= SIOCDEVPRIVATE + 15)) { dev_load(net, ifr->ifr_name); - rtnl_lock(); + + rtnl_net_lock(net); ret = dev_ifsioc(net, ifr, data, cmd); - rtnl_unlock(); + rtnl_net_unlock(net); return ret; } return -ENOTTY; diff --git a/net/core/rtnl_net_debug.c b/net/core/rtnl_net_debug.c index f406045cbd0e..7ecd28cc1c22 100644 --- a/net/core/rtnl_net_debug.c +++ b/net/core/rtnl_net_debug.c @@ -27,7 +27,6 @@ static int rtnl_net_debug_event(struct notifier_block *nb, case NETDEV_CHANGEADDR: case NETDEV_PRE_CHANGEADDR: case NETDEV_GOING_DOWN: - case NETDEV_CHANGENAME: case NETDEV_FEAT_CHANGE: case NETDEV_BONDING_FAILOVER: case NETDEV_PRE_UP: @@ -60,18 +59,10 @@ static int rtnl_net_debug_event(struct notifier_block *nb, ASSERT_RTNL(); break; - /* Once an event fully supports RTNL_NET, move it here - * and remove "if (0)" below. - * - * case NETDEV_XXX: - * ASSERT_RTNL_NET(net); - * break; - */ - } - - /* Just to avoid unused-variable error for dev and net. */ - if (0) + case NETDEV_CHANGENAME: ASSERT_RTNL_NET(net); + break; + } return NOTIFY_DONE; }