From 6dd132516f8e467f144f7871ff2708ce827417a1 Mon Sep 17 00:00:00 2001 From: Stanislav Fomichev Date: Wed, 12 Mar 2025 12:05:13 -0700 Subject: [PATCH] net: reorder dev_addr_sem lock Lockdep complains about circular lock in 1 -> 2 -> 3 (see below). Change the lock ordering to be: - rtnl_lock - dev_addr_sem - netdev_ops (only for lower devices!) - team_lock (or other per-upper device lock) 1. rtnl_lock -> netdev_ops -> dev_addr_sem rtnl_setlink rtnl_lock do_setlink IFLA_ADDRESS on lower netdev_ops dev_addr_sem 2. rtnl_lock -> team_lock -> netdev_ops rtnl_newlink rtnl_lock do_setlink IFLA_MASTER on lower do_set_master team_add_slave team_lock team_port_add dev_set_mtu netdev_ops 3. rtnl_lock -> dev_addr_sem -> team_lock rtnl_newlink rtnl_lock do_setlink IFLA_ADDRESS on upper dev_addr_sem netif_set_mac_address team_set_mac_address team_lock 4. rtnl_lock -> netdev_ops -> dev_addr_sem rtnl_lock dev_ifsioc dev_set_mac_address_user __tun_chr_ioctl rtnl_lock dev_set_mac_address_user tap_ioctl rtnl_lock dev_set_mac_address_user dev_set_mac_address_user netdev_lock_ops netif_set_mac_address_user dev_addr_sem v2: - move lock reorder to happen after kmalloc (Kuniyuki) Cc: Kohei Enju Fixes: df43d8bf1031 ("net: replace dev_addr_sem with netdev instance lock") Signed-off-by: Stanislav Fomichev Reviewed-by: Kuniyuki Iwashima Link: https://patch.msgid.link/20250312190513.1252045-3-sdf@fomichev.me Tested-by: Lei Yang Signed-off-by: Paolo Abeni --- include/linux/netdevice.h | 2 -- net/core/dev.c | 11 ----------- net/core/dev_api.c | 4 +++- net/core/rtnetlink.c | 15 +++++++++++++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 0db9fc0afe36..0c5b1f7f8f3a 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -4196,8 +4196,6 @@ int netif_set_mac_address(struct net_device *dev, struct sockaddr *sa, struct netlink_ext_ack *extack); int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa, struct netlink_ext_ack *extack); -int netif_set_mac_address_user(struct net_device *dev, struct sockaddr *sa, - struct netlink_ext_ack *extack); int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa, struct netlink_ext_ack *extack); int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name); diff --git a/net/core/dev.c b/net/core/dev.c index 977a9946d39c..235560341765 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -9577,17 +9577,6 @@ int netif_set_mac_address(struct net_device *dev, struct sockaddr *sa, DECLARE_RWSEM(dev_addr_sem); -int netif_set_mac_address_user(struct net_device *dev, struct sockaddr *sa, - struct netlink_ext_ack *extack) -{ - int ret; - - down_write(&dev_addr_sem); - ret = netif_set_mac_address(dev, sa, extack); - up_write(&dev_addr_sem); - return ret; -} - int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name) { size_t size = sizeof(sa->sa_data_min); diff --git a/net/core/dev_api.c b/net/core/dev_api.c index 2e17548af685..8dbc60612100 100644 --- a/net/core/dev_api.c +++ b/net/core/dev_api.c @@ -89,9 +89,11 @@ int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa, { int ret; + down_write(&dev_addr_sem); netdev_lock_ops(dev); - ret = netif_set_mac_address_user(dev, sa, extack); + ret = netif_set_mac_address(dev, sa, extack); netdev_unlock_ops(dev); + up_write(&dev_addr_sem); return ret; } diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 9355058bf996..5a24a30dfc2d 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3088,13 +3088,24 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev, goto errout; } sa->sa_family = dev->type; + + netdev_unlock_ops(dev); + + /* dev_addr_sem is an outer lock, enforce proper ordering */ + down_write(&dev_addr_sem); + netdev_lock_ops(dev); + memcpy(sa->sa_data, nla_data(tb[IFLA_ADDRESS]), dev->addr_len); - err = netif_set_mac_address_user(dev, sa, extack); + err = netif_set_mac_address(dev, sa, extack); kfree(sa); - if (err) + if (err) { + up_write(&dev_addr_sem); goto errout; + } status |= DO_SETLINK_MODIFIED; + + up_write(&dev_addr_sem); } if (tb[IFLA_MTU]) {