]> Git Repo - linux.git/commitdiff
net: change addr_list_lock back to static key
authorCong Wang <[email protected]>
Mon, 8 Jun 2020 21:53:01 +0000 (14:53 -0700)
committerDavid S. Miller <[email protected]>
Tue, 9 Jun 2020 19:59:45 +0000 (12:59 -0700)
The dynamic key update for addr_list_lock still causes troubles,
for example the following race condition still exists:

CPU 0: CPU 1:
(RCU read lock) (RTNL lock)
dev_mc_seq_show() netdev_update_lockdep_key()
  -> lockdep_unregister_key()
 -> netif_addr_lock_bh()

because lockdep doesn't provide an API to update it atomically.
Therefore, we have to move it back to static keys and use subclass
for nest locking like before.

In commit 1a33e10e4a95 ("net: partially revert dynamic lockdep key
changes"), I already reverted most parts of commit ab92d68fc22f
("net: core: add generic lockdep keys").

This patch reverts the rest and also part of commit f3b0a18bb6cb
("net: remove unnecessary variables and callback"). After this
patch, addr_list_lock changes back to using static keys and
subclasses to satisfy lockdep. Thanks to dev->lower_level, we do
not have to change back to ->ndo_get_lock_subclass().

And hopefully this reduces some syzbot lockdep noises too.

Reported-by: [email protected]
Cc: Taehee Yoo <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Signed-off-by: Cong Wang <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
17 files changed:
drivers/net/bonding/bond_main.c
drivers/net/bonding/bond_options.c
drivers/net/hamradio/bpqether.c
drivers/net/macsec.c
drivers/net/macvlan.c
drivers/net/vxlan.c
drivers/net/wireless/intersil/hostap/hostap_hw.c
include/linux/netdevice.h
net/8021q/vlan_dev.c
net/batman-adv/soft-interface.c
net/bridge/br_device.c
net/core/dev.c
net/core/dev_addr_lists.c
net/core/rtnetlink.c
net/dsa/master.c
net/netrom/af_netrom.c
net/rose/af_rose.c

index a25c65d4af71649a6b81441ed11903128395b9c7..004919aea5fbf8e5fd44734dd7b89d5a7f8e9d5e 100644 (file)
@@ -3687,8 +3687,6 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
        case BOND_RELEASE_OLD:
        case SIOCBONDRELEASE:
                res = bond_release(bond_dev, slave_dev);
-               if (!res)
-                       netdev_update_lockdep_key(slave_dev);
                break;
        case BOND_SETHWADDR_OLD:
        case SIOCBONDSETHWADDR:
index 215c1092328937a01531840ea9222fc5fe0e13a2..ddb3916d3506bec2fe317973054a32ea5c4255ad 100644 (file)
@@ -1398,8 +1398,6 @@ static int bond_option_slaves_set(struct bonding *bond,
        case '-':
                slave_dbg(bond->dev, dev, "Releasing interface\n");
                ret = bond_release(bond->dev, dev);
-               if (!ret)
-                       netdev_update_lockdep_key(dev);
                break;
 
        default:
index 60dcaf2a04a913305bc0d87b497827dbc2c17032..1ad6085994b1cd74a851c2dca47375d896768553 100644 (file)
@@ -113,6 +113,7 @@ static LIST_HEAD(bpq_devices);
  * off into a separate class since they always nest.
  */
 static struct lock_class_key bpq_netdev_xmit_lock_key;
+static struct lock_class_key bpq_netdev_addr_lock_key;
 
 static void bpq_set_lockdep_class_one(struct net_device *dev,
                                      struct netdev_queue *txq,
@@ -123,6 +124,7 @@ static void bpq_set_lockdep_class_one(struct net_device *dev,
 
 static void bpq_set_lockdep_class(struct net_device *dev)
 {
+       lockdep_set_class(&dev->addr_list_lock, &bpq_netdev_addr_lock_key);
        netdev_for_each_tx_queue(dev, bpq_set_lockdep_class_one, NULL);
 }
 
index 20b53e255f68ab11de32b6c15629b03c820984aa..e56547bfdac9a909a159bfdb5aa67014d159476b 100644 (file)
@@ -3999,6 +3999,8 @@ static int macsec_add_dev(struct net_device *dev, sci_t sci, u8 icv_len)
        return 0;
 }
 
+static struct lock_class_key macsec_netdev_addr_lock_key;
+
 static int macsec_newlink(struct net *net, struct net_device *dev,
                          struct nlattr *tb[], struct nlattr *data[],
                          struct netlink_ext_ack *extack)
@@ -4050,6 +4052,9 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
                return err;
 
        netdev_lockdep_set_classes(dev);
+       lockdep_set_class_and_subclass(&dev->addr_list_lock,
+                                      &macsec_netdev_addr_lock_key,
+                                      dev->lower_level);
 
        err = netdev_upper_dev_link(real_dev, dev, extack);
        if (err < 0)
index 563aed5b3d9feb9a2fb0c8b48ebfd0ff90e55a94..6a6cc9f75307566ec108e26aad9599633d7b6577 100644 (file)
@@ -860,6 +860,8 @@ static int macvlan_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
  * "super class" of normal network devices; split their locks off into a
  * separate class since they always nest.
  */
+static struct lock_class_key macvlan_netdev_addr_lock_key;
+
 #define ALWAYS_ON_OFFLOADS \
        (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \
         NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL)
@@ -875,6 +877,14 @@ static int macvlan_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 #define MACVLAN_STATE_MASK \
        ((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT))
 
+static void macvlan_set_lockdep_class(struct net_device *dev)
+{
+       netdev_lockdep_set_classes(dev);
+       lockdep_set_class_and_subclass(&dev->addr_list_lock,
+                                      &macvlan_netdev_addr_lock_key,
+                                      dev->lower_level);
+}
+
 static int macvlan_init(struct net_device *dev)
 {
        struct macvlan_dev *vlan = netdev_priv(dev);
@@ -892,8 +902,7 @@ static int macvlan_init(struct net_device *dev)
        dev->gso_max_size       = lowerdev->gso_max_size;
        dev->gso_max_segs       = lowerdev->gso_max_segs;
        dev->hard_header_len    = lowerdev->hard_header_len;
-
-       netdev_lockdep_set_classes(dev);
+       macvlan_set_lockdep_class(dev);
 
        vlan->pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
        if (!vlan->pcpu_stats)
index 5bb448ae6c9c456988d4f30d5bc02ca5e9754621..47424b2da64376d02f300bfa53184b27e429cf2a 100644 (file)
@@ -4245,10 +4245,8 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
                mod_timer(&vxlan->age_timer, jiffies);
 
        netdev_adjacent_change_commit(dst->remote_dev, lowerdev, dev);
-       if (lowerdev && lowerdev != dst->remote_dev) {
+       if (lowerdev && lowerdev != dst->remote_dev)
                dst->remote_dev = lowerdev;
-               netdev_update_lockdep_key(lowerdev);
-       }
        vxlan_config_apply(dev, &conf, lowerdev, vxlan->net, true);
        return 0;
 }
index aadf3dec5bf327ac7a52645a85246eeca5272ddd..2ab34cf74ecc3f9d9b4b72a1b3070dde30260477 100644 (file)
@@ -3048,6 +3048,7 @@ static void prism2_clear_set_tim_queue(local_info_t *local)
  * This is a natural nesting, which needs a split lock type.
  */
 static struct lock_class_key hostap_netdev_xmit_lock_key;
+static struct lock_class_key hostap_netdev_addr_lock_key;
 
 static void prism2_set_lockdep_class_one(struct net_device *dev,
                                         struct netdev_queue *txq,
@@ -3059,6 +3060,8 @@ static void prism2_set_lockdep_class_one(struct net_device *dev,
 
 static void prism2_set_lockdep_class(struct net_device *dev)
 {
+       lockdep_set_class(&dev->addr_list_lock,
+                         &hostap_netdev_addr_lock_key);
        netdev_for_each_tx_queue(dev, prism2_set_lockdep_class_one, NULL);
 }
 
index 1a96e9c4ec36fce56d2b167298411277710eb84b..e2825e27ef89d845255b48a6f1c206c1a7c707b5 100644 (file)
@@ -1821,8 +1821,6 @@ enum netdev_priv_flags {
  *                     for hardware timestamping
  *     @sfp_bus:       attached &struct sfp_bus structure.
  *
- *     @addr_list_lock_key:    lockdep class annotating
- *                             net_device->addr_list_lock spinlock
  *     @qdisc_tx_busylock: lockdep class annotating Qdisc->busylock spinlock
  *     @qdisc_running_key: lockdep class annotating Qdisc->running seqcount
  *
@@ -2125,7 +2123,6 @@ struct net_device {
 #endif
        struct phy_device       *phydev;
        struct sfp_bus          *sfp_bus;
-       struct lock_class_key   addr_list_lock_key;
        struct lock_class_key   *qdisc_tx_busylock;
        struct lock_class_key   *qdisc_running_key;
        bool                    proto_down;
@@ -2217,10 +2214,13 @@ static inline void netdev_for_each_tx_queue(struct net_device *dev,
        static struct lock_class_key qdisc_tx_busylock_key;     \
        static struct lock_class_key qdisc_running_key;         \
        static struct lock_class_key qdisc_xmit_lock_key;       \
+       static struct lock_class_key dev_addr_list_lock_key;    \
        unsigned int i;                                         \
                                                                \
        (dev)->qdisc_tx_busylock = &qdisc_tx_busylock_key;      \
        (dev)->qdisc_running_key = &qdisc_running_key;          \
+       lockdep_set_class(&(dev)->addr_list_lock,               \
+                         &dev_addr_list_lock_key);             \
        for (i = 0; i < (dev)->num_tx_queues; i++)              \
                lockdep_set_class(&(dev)->_tx[i]._xmit_lock,    \
                                  &qdisc_xmit_lock_key);        \
@@ -3253,7 +3253,6 @@ static inline void netif_stop_queue(struct net_device *dev)
 }
 
 void netif_tx_stop_all_queues(struct net_device *dev);
-void netdev_update_lockdep_key(struct net_device *dev);
 
 static inline bool netif_tx_queue_stopped(const struct netdev_queue *dev_queue)
 {
@@ -4239,6 +4238,11 @@ static inline void netif_addr_lock(struct net_device *dev)
        spin_lock(&dev->addr_list_lock);
 }
 
+static inline void netif_addr_lock_nested(struct net_device *dev)
+{
+       spin_lock_nested(&dev->addr_list_lock, dev->lower_level);
+}
+
 static inline void netif_addr_lock_bh(struct net_device *dev)
 {
        spin_lock_bh(&dev->addr_list_lock);
index f00bb57f0f600b2366eb8ac06da0f1f6d8590099..c8d6a07e23c579c5b097a62e229cee0145efd8c3 100644 (file)
@@ -494,6 +494,7 @@ static void vlan_dev_set_rx_mode(struct net_device *vlan_dev)
  * separate class since they always nest.
  */
 static struct lock_class_key vlan_netdev_xmit_lock_key;
+static struct lock_class_key vlan_netdev_addr_lock_key;
 
 static void vlan_dev_set_lockdep_one(struct net_device *dev,
                                     struct netdev_queue *txq,
@@ -502,8 +503,11 @@ static void vlan_dev_set_lockdep_one(struct net_device *dev,
        lockdep_set_class(&txq->_xmit_lock, &vlan_netdev_xmit_lock_key);
 }
 
-static void vlan_dev_set_lockdep_class(struct net_device *dev)
+static void vlan_dev_set_lockdep_class(struct net_device *dev, int subclass)
 {
+       lockdep_set_class_and_subclass(&dev->addr_list_lock,
+                                      &vlan_netdev_addr_lock_key,
+                                      subclass);
        netdev_for_each_tx_queue(dev, vlan_dev_set_lockdep_one, NULL);
 }
 
@@ -597,7 +601,7 @@ static int vlan_dev_init(struct net_device *dev)
 
        SET_NETDEV_DEVTYPE(dev, &vlan_type);
 
-       vlan_dev_set_lockdep_class(dev);
+       vlan_dev_set_lockdep_class(dev, dev->lower_level);
 
        vlan->vlan_pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
        if (!vlan->vlan_pcpu_stats)
index 0ddd80130ea36bb6a812db2332092316c5a930c2..f1f1c86f34193fc2f1ce005d988bb43b55c00ed6 100644 (file)
@@ -745,6 +745,7 @@ static int batadv_interface_kill_vid(struct net_device *dev, __be16 proto,
  * separate class since they always nest.
  */
 static struct lock_class_key batadv_netdev_xmit_lock_key;
+static struct lock_class_key batadv_netdev_addr_lock_key;
 
 /**
  * batadv_set_lockdep_class_one() - Set lockdep class for a single tx queue
@@ -765,6 +766,7 @@ static void batadv_set_lockdep_class_one(struct net_device *dev,
  */
 static void batadv_set_lockdep_class(struct net_device *dev)
 {
+       lockdep_set_class(&dev->addr_list_lock, &batadv_netdev_addr_lock_key);
        netdev_for_each_tx_queue(dev, batadv_set_lockdep_class_one, NULL);
 }
 
index 8ec1362588af168ed366ee322089c29923b2fd04..8c7b78f8bc230f5811803f83ab72ee42b71be5e2 100644 (file)
@@ -105,6 +105,13 @@ out:
        return NETDEV_TX_OK;
 }
 
+static struct lock_class_key bridge_netdev_addr_lock_key;
+
+static void br_set_lockdep_class(struct net_device *dev)
+{
+       lockdep_set_class(&dev->addr_list_lock, &bridge_netdev_addr_lock_key);
+}
+
 static int br_dev_init(struct net_device *dev)
 {
        struct net_bridge *br = netdev_priv(dev);
@@ -143,6 +150,7 @@ static int br_dev_init(struct net_device *dev)
                br_fdb_hash_fini(br);
        }
 
+       br_set_lockdep_class(dev);
        return err;
 }
 
index 061496a1f640fe7d0d97373cf0a11b352c3bebb9..6bc2388141f6fd7c66c0e8349514a326e5106db2 100644 (file)
@@ -439,6 +439,7 @@ static const char *const netdev_lock_name[] = {
        "_xmit_IEEE802154", "_xmit_VOID", "_xmit_NONE"};
 
 static struct lock_class_key netdev_xmit_lock_key[ARRAY_SIZE(netdev_lock_type)];
+static struct lock_class_key netdev_addr_lock_key[ARRAY_SIZE(netdev_lock_type)];
 
 static inline unsigned short netdev_lock_pos(unsigned short dev_type)
 {
@@ -460,11 +461,25 @@ static inline void netdev_set_xmit_lockdep_class(spinlock_t *lock,
        lockdep_set_class_and_name(lock, &netdev_xmit_lock_key[i],
                                   netdev_lock_name[i]);
 }
+
+static inline void netdev_set_addr_lockdep_class(struct net_device *dev)
+{
+       int i;
+
+       i = netdev_lock_pos(dev->type);
+       lockdep_set_class_and_name(&dev->addr_list_lock,
+                                  &netdev_addr_lock_key[i],
+                                  netdev_lock_name[i]);
+}
 #else
 static inline void netdev_set_xmit_lockdep_class(spinlock_t *lock,
                                                 unsigned short dev_type)
 {
 }
+
+static inline void netdev_set_addr_lockdep_class(struct net_device *dev)
+{
+}
 #endif
 
 /*******************************************************************************
@@ -9373,15 +9388,6 @@ void netif_tx_stop_all_queues(struct net_device *dev)
 }
 EXPORT_SYMBOL(netif_tx_stop_all_queues);
 
-void netdev_update_lockdep_key(struct net_device *dev)
-{
-       lockdep_unregister_key(&dev->addr_list_lock_key);
-       lockdep_register_key(&dev->addr_list_lock_key);
-
-       lockdep_set_class(&dev->addr_list_lock, &dev->addr_list_lock_key);
-}
-EXPORT_SYMBOL(netdev_update_lockdep_key);
-
 /**
  *     register_netdevice      - register a network device
  *     @dev: device to register
@@ -9420,7 +9426,7 @@ int register_netdevice(struct net_device *dev)
                return ret;
 
        spin_lock_init(&dev->addr_list_lock);
-       lockdep_set_class(&dev->addr_list_lock, &dev->addr_list_lock_key);
+       netdev_set_addr_lockdep_class(dev);
 
        ret = dev_get_valid_name(net, dev, dev->name);
        if (ret < 0)
@@ -9939,8 +9945,6 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 
        dev_net_set(dev, &init_net);
 
-       lockdep_register_key(&dev->addr_list_lock_key);
-
        dev->gso_max_size = GSO_MAX_SIZE;
        dev->gso_max_segs = GSO_MAX_SEGS;
        dev->upper_level = 1;
@@ -10028,8 +10032,6 @@ void free_netdev(struct net_device *dev)
        free_percpu(dev->xdp_bulkq);
        dev->xdp_bulkq = NULL;
 
-       lockdep_unregister_key(&dev->addr_list_lock_key);
-
        /*  Compatibility with error handling in drivers */
        if (dev->reg_state == NETREG_UNINITIALIZED) {
                netdev_freemem(dev);
index 2f949b5a1eb9cd9a65e0c0a6d089032a92e100e2..6393ba930097b49b26a70cf01b657da08514f552 100644 (file)
@@ -637,7 +637,7 @@ int dev_uc_sync(struct net_device *to, struct net_device *from)
        if (to->addr_len != from->addr_len)
                return -EINVAL;
 
-       netif_addr_lock(to);
+       netif_addr_lock_nested(to);
        err = __hw_addr_sync(&to->uc, &from->uc, to->addr_len);
        if (!err)
                __dev_set_rx_mode(to);
@@ -667,7 +667,7 @@ int dev_uc_sync_multiple(struct net_device *to, struct net_device *from)
        if (to->addr_len != from->addr_len)
                return -EINVAL;
 
-       netif_addr_lock(to);
+       netif_addr_lock_nested(to);
        err = __hw_addr_sync_multiple(&to->uc, &from->uc, to->addr_len);
        if (!err)
                __dev_set_rx_mode(to);
@@ -691,7 +691,7 @@ void dev_uc_unsync(struct net_device *to, struct net_device *from)
                return;
 
        netif_addr_lock_bh(from);
-       netif_addr_lock(to);
+       netif_addr_lock_nested(to);
        __hw_addr_unsync(&to->uc, &from->uc, to->addr_len);
        __dev_set_rx_mode(to);
        netif_addr_unlock(to);
@@ -858,7 +858,7 @@ int dev_mc_sync(struct net_device *to, struct net_device *from)
        if (to->addr_len != from->addr_len)
                return -EINVAL;
 
-       netif_addr_lock(to);
+       netif_addr_lock_nested(to);
        err = __hw_addr_sync(&to->mc, &from->mc, to->addr_len);
        if (!err)
                __dev_set_rx_mode(to);
@@ -888,7 +888,7 @@ int dev_mc_sync_multiple(struct net_device *to, struct net_device *from)
        if (to->addr_len != from->addr_len)
                return -EINVAL;
 
-       netif_addr_lock(to);
+       netif_addr_lock_nested(to);
        err = __hw_addr_sync_multiple(&to->mc, &from->mc, to->addr_len);
        if (!err)
                __dev_set_rx_mode(to);
@@ -912,7 +912,7 @@ void dev_mc_unsync(struct net_device *to, struct net_device *from)
                return;
 
        netif_addr_lock_bh(from);
-       netif_addr_lock(to);
+       netif_addr_lock_nested(to);
        __hw_addr_unsync(&to->mc, &from->mc, to->addr_len);
        __dev_set_rx_mode(to);
        netif_addr_unlock(to);
index 2269199c589107659bc82fc0e13804b11d7de8ba..9aedc15736adfb87758f509d53fdfc0f2d1a0889 100644 (file)
@@ -2462,7 +2462,6 @@ static int do_set_master(struct net_device *dev, int ifindex,
                        err = ops->ndo_del_slave(upper_dev, dev);
                        if (err)
                                return err;
-                       netdev_update_lockdep_key(dev);
                } else {
                        return -EOPNOTSUPP;
                }
index a621367c6e8c292997984534614b0caea1fa991a..480a61460c239a11b5cadd3b65f0b8b43a55f2cf 100644 (file)
@@ -327,6 +327,8 @@ static void dsa_master_reset_mtu(struct net_device *dev)
        rtnl_unlock();
 }
 
+static struct lock_class_key dsa_master_addr_list_lock_key;
+
 int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
 {
        int ret;
@@ -345,6 +347,8 @@ int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
        wmb();
 
        dev->dsa_ptr = cpu_dp;
+       lockdep_set_class(&dev->addr_list_lock,
+                         &dsa_master_addr_list_lock_key);
        ret = dsa_master_ethtool_setup(dev);
        if (ret)
                return ret;
index eccc7d366e17f6eb52129908ee02f521da8bcb96..f90ef6934b8f4d668221f6dfcb1f5c31b346788b 100644 (file)
@@ -70,6 +70,7 @@ static const struct proto_ops nr_proto_ops;
  * separate class since they always nest.
  */
 static struct lock_class_key nr_netdev_xmit_lock_key;
+static struct lock_class_key nr_netdev_addr_lock_key;
 
 static void nr_set_lockdep_one(struct net_device *dev,
                               struct netdev_queue *txq,
@@ -80,6 +81,7 @@ static void nr_set_lockdep_one(struct net_device *dev,
 
 static void nr_set_lockdep_key(struct net_device *dev)
 {
+       lockdep_set_class(&dev->addr_list_lock, &nr_netdev_addr_lock_key);
        netdev_for_each_tx_queue(dev, nr_set_lockdep_one, NULL);
 }
 
index e7a872207b464e53fbc95c0496c0f17309a52870..ce85656ac9c159b6bb82bd5367981e06d2cfd6e6 100644 (file)
@@ -71,6 +71,7 @@ ax25_address rose_callsign;
  * separate class since they always nest.
  */
 static struct lock_class_key rose_netdev_xmit_lock_key;
+static struct lock_class_key rose_netdev_addr_lock_key;
 
 static void rose_set_lockdep_one(struct net_device *dev,
                                 struct netdev_queue *txq,
@@ -81,6 +82,7 @@ static void rose_set_lockdep_one(struct net_device *dev,
 
 static void rose_set_lockdep_key(struct net_device *dev)
 {
+       lockdep_set_class(&dev->addr_list_lock, &rose_netdev_addr_lock_key);
        netdev_for_each_tx_queue(dev, rose_set_lockdep_one, NULL);
 }
 
This page took 0.139102 seconds and 4 git commands to generate.