Skip to content

Commit 845e0eb

Browse files
congwangdavem330
congwang
authored and
davem330
committed
net: change addr_list_lock back to static key
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 1a33e10 ("net: partially revert dynamic lockdep key changes"), I already reverted most parts of commit ab92d68 ("net: core: add generic lockdep keys"). This patch reverts the rest and also part of commit f3b0a18 ("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]>
1 parent 8027bc0 commit 845e0eb

File tree

17 files changed

+76
-36
lines changed

17 files changed

+76
-36
lines changed

Diff for: ‎drivers/net/bonding/bond_main.c

-2
Original file line numberDiff line numberDiff line change
@@ -3687,8 +3687,6 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
36873687
case BOND_RELEASE_OLD:
36883688
case SIOCBONDRELEASE:
36893689
res = bond_release(bond_dev, slave_dev);
3690-
if (!res)
3691-
netdev_update_lockdep_key(slave_dev);
36923690
break;
36933691
case BOND_SETHWADDR_OLD:
36943692
case SIOCBONDSETHWADDR:

Diff for: ‎drivers/net/bonding/bond_options.c

-2
Original file line numberDiff line numberDiff line change
@@ -1398,8 +1398,6 @@ static int bond_option_slaves_set(struct bonding *bond,
13981398
case '-':
13991399
slave_dbg(bond->dev, dev, "Releasing interface\n");
14001400
ret = bond_release(bond->dev, dev);
1401-
if (!ret)
1402-
netdev_update_lockdep_key(dev);
14031401
break;
14041402

14051403
default:

Diff for: ‎drivers/net/hamradio/bpqether.c

+2
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ static LIST_HEAD(bpq_devices);
113113
* off into a separate class since they always nest.
114114
*/
115115
static struct lock_class_key bpq_netdev_xmit_lock_key;
116+
static struct lock_class_key bpq_netdev_addr_lock_key;
116117

117118
static void bpq_set_lockdep_class_one(struct net_device *dev,
118119
struct netdev_queue *txq,
@@ -123,6 +124,7 @@ static void bpq_set_lockdep_class_one(struct net_device *dev,
123124

124125
static void bpq_set_lockdep_class(struct net_device *dev)
125126
{
127+
lockdep_set_class(&dev->addr_list_lock, &bpq_netdev_addr_lock_key);
126128
netdev_for_each_tx_queue(dev, bpq_set_lockdep_class_one, NULL);
127129
}
128130

Diff for: ‎drivers/net/macsec.c

+5
Original file line numberDiff line numberDiff line change
@@ -3999,6 +3999,8 @@ static int macsec_add_dev(struct net_device *dev, sci_t sci, u8 icv_len)
39993999
return 0;
40004000
}
40014001

4002+
static struct lock_class_key macsec_netdev_addr_lock_key;
4003+
40024004
static int macsec_newlink(struct net *net, struct net_device *dev,
40034005
struct nlattr *tb[], struct nlattr *data[],
40044006
struct netlink_ext_ack *extack)
@@ -4050,6 +4052,9 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
40504052
return err;
40514053

40524054
netdev_lockdep_set_classes(dev);
4055+
lockdep_set_class_and_subclass(&dev->addr_list_lock,
4056+
&macsec_netdev_addr_lock_key,
4057+
dev->lower_level);
40534058

40544059
err = netdev_upper_dev_link(real_dev, dev, extack);
40554060
if (err < 0)

Diff for: ‎drivers/net/macvlan.c

+11-2
Original file line numberDiff line numberDiff line change
@@ -860,6 +860,8 @@ static int macvlan_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
860860
* "super class" of normal network devices; split their locks off into a
861861
* separate class since they always nest.
862862
*/
863+
static struct lock_class_key macvlan_netdev_addr_lock_key;
864+
863865
#define ALWAYS_ON_OFFLOADS \
864866
(NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \
865867
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)
875877
#define MACVLAN_STATE_MASK \
876878
((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT))
877879

880+
static void macvlan_set_lockdep_class(struct net_device *dev)
881+
{
882+
netdev_lockdep_set_classes(dev);
883+
lockdep_set_class_and_subclass(&dev->addr_list_lock,
884+
&macvlan_netdev_addr_lock_key,
885+
dev->lower_level);
886+
}
887+
878888
static int macvlan_init(struct net_device *dev)
879889
{
880890
struct macvlan_dev *vlan = netdev_priv(dev);
@@ -892,8 +902,7 @@ static int macvlan_init(struct net_device *dev)
892902
dev->gso_max_size = lowerdev->gso_max_size;
893903
dev->gso_max_segs = lowerdev->gso_max_segs;
894904
dev->hard_header_len = lowerdev->hard_header_len;
895-
896-
netdev_lockdep_set_classes(dev);
905+
macvlan_set_lockdep_class(dev);
897906

898907
vlan->pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
899908
if (!vlan->pcpu_stats)

Diff for: ‎drivers/net/vxlan.c

+1-3
Original file line numberDiff line numberDiff line change
@@ -4245,10 +4245,8 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
42454245
mod_timer(&vxlan->age_timer, jiffies);
42464246

42474247
netdev_adjacent_change_commit(dst->remote_dev, lowerdev, dev);
4248-
if (lowerdev && lowerdev != dst->remote_dev) {
4248+
if (lowerdev && lowerdev != dst->remote_dev)
42494249
dst->remote_dev = lowerdev;
4250-
netdev_update_lockdep_key(lowerdev);
4251-
}
42524250
vxlan_config_apply(dev, &conf, lowerdev, vxlan->net, true);
42534251
return 0;
42544252
}

Diff for: ‎drivers/net/wireless/intersil/hostap/hostap_hw.c

+3
Original file line numberDiff line numberDiff line change
@@ -3048,6 +3048,7 @@ static void prism2_clear_set_tim_queue(local_info_t *local)
30483048
* This is a natural nesting, which needs a split lock type.
30493049
*/
30503050
static struct lock_class_key hostap_netdev_xmit_lock_key;
3051+
static struct lock_class_key hostap_netdev_addr_lock_key;
30513052

30523053
static void prism2_set_lockdep_class_one(struct net_device *dev,
30533054
struct netdev_queue *txq,
@@ -3059,6 +3060,8 @@ static void prism2_set_lockdep_class_one(struct net_device *dev,
30593060

30603061
static void prism2_set_lockdep_class(struct net_device *dev)
30613062
{
3063+
lockdep_set_class(&dev->addr_list_lock,
3064+
&hostap_netdev_addr_lock_key);
30623065
netdev_for_each_tx_queue(dev, prism2_set_lockdep_class_one, NULL);
30633066
}
30643067

Diff for: ‎include/linux/netdevice.h

+8-4
Original file line numberDiff line numberDiff line change
@@ -1821,8 +1821,6 @@ enum netdev_priv_flags {
18211821
* for hardware timestamping
18221822
* @sfp_bus: attached &struct sfp_bus structure.
18231823
*
1824-
* @addr_list_lock_key: lockdep class annotating
1825-
* net_device->addr_list_lock spinlock
18261824
* @qdisc_tx_busylock: lockdep class annotating Qdisc->busylock spinlock
18271825
* @qdisc_running_key: lockdep class annotating Qdisc->running seqcount
18281826
*
@@ -2125,7 +2123,6 @@ struct net_device {
21252123
#endif
21262124
struct phy_device *phydev;
21272125
struct sfp_bus *sfp_bus;
2128-
struct lock_class_key addr_list_lock_key;
21292126
struct lock_class_key *qdisc_tx_busylock;
21302127
struct lock_class_key *qdisc_running_key;
21312128
bool proto_down;
@@ -2217,10 +2214,13 @@ static inline void netdev_for_each_tx_queue(struct net_device *dev,
22172214
static struct lock_class_key qdisc_tx_busylock_key; \
22182215
static struct lock_class_key qdisc_running_key; \
22192216
static struct lock_class_key qdisc_xmit_lock_key; \
2217+
static struct lock_class_key dev_addr_list_lock_key; \
22202218
unsigned int i; \
22212219
\
22222220
(dev)->qdisc_tx_busylock = &qdisc_tx_busylock_key; \
22232221
(dev)->qdisc_running_key = &qdisc_running_key; \
2222+
lockdep_set_class(&(dev)->addr_list_lock, \
2223+
&dev_addr_list_lock_key); \
22242224
for (i = 0; i < (dev)->num_tx_queues; i++) \
22252225
lockdep_set_class(&(dev)->_tx[i]._xmit_lock, \
22262226
&qdisc_xmit_lock_key); \
@@ -3253,7 +3253,6 @@ static inline void netif_stop_queue(struct net_device *dev)
32533253
}
32543254

32553255
void netif_tx_stop_all_queues(struct net_device *dev);
3256-
void netdev_update_lockdep_key(struct net_device *dev);
32573256

32583257
static inline bool netif_tx_queue_stopped(const struct netdev_queue *dev_queue)
32593258
{
@@ -4239,6 +4238,11 @@ static inline void netif_addr_lock(struct net_device *dev)
42394238
spin_lock(&dev->addr_list_lock);
42404239
}
42414240

4241+
static inline void netif_addr_lock_nested(struct net_device *dev)
4242+
{
4243+
spin_lock_nested(&dev->addr_list_lock, dev->lower_level);
4244+
}
4245+
42424246
static inline void netif_addr_lock_bh(struct net_device *dev)
42434247
{
42444248
spin_lock_bh(&dev->addr_list_lock);

Diff for: ‎net/8021q/vlan_dev.c

+6-2
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@ static void vlan_dev_set_rx_mode(struct net_device *vlan_dev)
494494
* separate class since they always nest.
495495
*/
496496
static struct lock_class_key vlan_netdev_xmit_lock_key;
497+
static struct lock_class_key vlan_netdev_addr_lock_key;
497498

498499
static void vlan_dev_set_lockdep_one(struct net_device *dev,
499500
struct netdev_queue *txq,
@@ -502,8 +503,11 @@ static void vlan_dev_set_lockdep_one(struct net_device *dev,
502503
lockdep_set_class(&txq->_xmit_lock, &vlan_netdev_xmit_lock_key);
503504
}
504505

505-
static void vlan_dev_set_lockdep_class(struct net_device *dev)
506+
static void vlan_dev_set_lockdep_class(struct net_device *dev, int subclass)
506507
{
508+
lockdep_set_class_and_subclass(&dev->addr_list_lock,
509+
&vlan_netdev_addr_lock_key,
510+
subclass);
507511
netdev_for_each_tx_queue(dev, vlan_dev_set_lockdep_one, NULL);
508512
}
509513

@@ -597,7 +601,7 @@ static int vlan_dev_init(struct net_device *dev)
597601

598602
SET_NETDEV_DEVTYPE(dev, &vlan_type);
599603

600-
vlan_dev_set_lockdep_class(dev);
604+
vlan_dev_set_lockdep_class(dev, dev->lower_level);
601605

602606
vlan->vlan_pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
603607
if (!vlan->vlan_pcpu_stats)

Diff for: ‎net/batman-adv/soft-interface.c

+2
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,7 @@ static int batadv_interface_kill_vid(struct net_device *dev, __be16 proto,
745745
* separate class since they always nest.
746746
*/
747747
static struct lock_class_key batadv_netdev_xmit_lock_key;
748+
static struct lock_class_key batadv_netdev_addr_lock_key;
748749

749750
/**
750751
* 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,
765766
*/
766767
static void batadv_set_lockdep_class(struct net_device *dev)
767768
{
769+
lockdep_set_class(&dev->addr_list_lock, &batadv_netdev_addr_lock_key);
768770
netdev_for_each_tx_queue(dev, batadv_set_lockdep_class_one, NULL);
769771
}
770772

Diff for: ‎net/bridge/br_device.c

+8
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
105105
return NETDEV_TX_OK;
106106
}
107107

108+
static struct lock_class_key bridge_netdev_addr_lock_key;
109+
110+
static void br_set_lockdep_class(struct net_device *dev)
111+
{
112+
lockdep_set_class(&dev->addr_list_lock, &bridge_netdev_addr_lock_key);
113+
}
114+
108115
static int br_dev_init(struct net_device *dev)
109116
{
110117
struct net_bridge *br = netdev_priv(dev);
@@ -143,6 +150,7 @@ static int br_dev_init(struct net_device *dev)
143150
br_fdb_hash_fini(br);
144151
}
145152

153+
br_set_lockdep_class(dev);
146154
return err;
147155
}
148156

Diff for: ‎net/core/dev.c

+16-14
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,7 @@ static const char *const netdev_lock_name[] = {
439439
"_xmit_IEEE802154", "_xmit_VOID", "_xmit_NONE"};
440440

441441
static struct lock_class_key netdev_xmit_lock_key[ARRAY_SIZE(netdev_lock_type)];
442+
static struct lock_class_key netdev_addr_lock_key[ARRAY_SIZE(netdev_lock_type)];
442443

443444
static inline unsigned short netdev_lock_pos(unsigned short dev_type)
444445
{
@@ -460,11 +461,25 @@ static inline void netdev_set_xmit_lockdep_class(spinlock_t *lock,
460461
lockdep_set_class_and_name(lock, &netdev_xmit_lock_key[i],
461462
netdev_lock_name[i]);
462463
}
464+
465+
static inline void netdev_set_addr_lockdep_class(struct net_device *dev)
466+
{
467+
int i;
468+
469+
i = netdev_lock_pos(dev->type);
470+
lockdep_set_class_and_name(&dev->addr_list_lock,
471+
&netdev_addr_lock_key[i],
472+
netdev_lock_name[i]);
473+
}
463474
#else
464475
static inline void netdev_set_xmit_lockdep_class(spinlock_t *lock,
465476
unsigned short dev_type)
466477
{
467478
}
479+
480+
static inline void netdev_set_addr_lockdep_class(struct net_device *dev)
481+
{
482+
}
468483
#endif
469484

470485
/*******************************************************************************
@@ -9373,15 +9388,6 @@ void netif_tx_stop_all_queues(struct net_device *dev)
93739388
}
93749389
EXPORT_SYMBOL(netif_tx_stop_all_queues);
93759390

9376-
void netdev_update_lockdep_key(struct net_device *dev)
9377-
{
9378-
lockdep_unregister_key(&dev->addr_list_lock_key);
9379-
lockdep_register_key(&dev->addr_list_lock_key);
9380-
9381-
lockdep_set_class(&dev->addr_list_lock, &dev->addr_list_lock_key);
9382-
}
9383-
EXPORT_SYMBOL(netdev_update_lockdep_key);
9384-
93859391
/**
93869392
* register_netdevice - register a network device
93879393
* @dev: device to register
@@ -9420,7 +9426,7 @@ int register_netdevice(struct net_device *dev)
94209426
return ret;
94219427

94229428
spin_lock_init(&dev->addr_list_lock);
9423-
lockdep_set_class(&dev->addr_list_lock, &dev->addr_list_lock_key);
9429+
netdev_set_addr_lockdep_class(dev);
94249430

94259431
ret = dev_get_valid_name(net, dev, dev->name);
94269432
if (ret < 0)
@@ -9939,8 +9945,6 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
99399945

99409946
dev_net_set(dev, &init_net);
99419947

9942-
lockdep_register_key(&dev->addr_list_lock_key);
9943-
99449948
dev->gso_max_size = GSO_MAX_SIZE;
99459949
dev->gso_max_segs = GSO_MAX_SEGS;
99469950
dev->upper_level = 1;
@@ -10028,8 +10032,6 @@ void free_netdev(struct net_device *dev)
1002810032
free_percpu(dev->xdp_bulkq);
1002910033
dev->xdp_bulkq = NULL;
1003010034

10031-
lockdep_unregister_key(&dev->addr_list_lock_key);
10032-
1003310035
/* Compatibility with error handling in drivers */
1003410036
if (dev->reg_state == NETREG_UNINITIALIZED) {
1003510037
netdev_freemem(dev);

Diff for: ‎net/core/dev_addr_lists.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,7 @@ int dev_uc_sync(struct net_device *to, struct net_device *from)
637637
if (to->addr_len != from->addr_len)
638638
return -EINVAL;
639639

640-
netif_addr_lock(to);
640+
netif_addr_lock_nested(to);
641641
err = __hw_addr_sync(&to->uc, &from->uc, to->addr_len);
642642
if (!err)
643643
__dev_set_rx_mode(to);
@@ -667,7 +667,7 @@ int dev_uc_sync_multiple(struct net_device *to, struct net_device *from)
667667
if (to->addr_len != from->addr_len)
668668
return -EINVAL;
669669

670-
netif_addr_lock(to);
670+
netif_addr_lock_nested(to);
671671
err = __hw_addr_sync_multiple(&to->uc, &from->uc, to->addr_len);
672672
if (!err)
673673
__dev_set_rx_mode(to);
@@ -691,7 +691,7 @@ void dev_uc_unsync(struct net_device *to, struct net_device *from)
691691
return;
692692

693693
netif_addr_lock_bh(from);
694-
netif_addr_lock(to);
694+
netif_addr_lock_nested(to);
695695
__hw_addr_unsync(&to->uc, &from->uc, to->addr_len);
696696
__dev_set_rx_mode(to);
697697
netif_addr_unlock(to);
@@ -858,7 +858,7 @@ int dev_mc_sync(struct net_device *to, struct net_device *from)
858858
if (to->addr_len != from->addr_len)
859859
return -EINVAL;
860860

861-
netif_addr_lock(to);
861+
netif_addr_lock_nested(to);
862862
err = __hw_addr_sync(&to->mc, &from->mc, to->addr_len);
863863
if (!err)
864864
__dev_set_rx_mode(to);
@@ -888,7 +888,7 @@ int dev_mc_sync_multiple(struct net_device *to, struct net_device *from)
888888
if (to->addr_len != from->addr_len)
889889
return -EINVAL;
890890

891-
netif_addr_lock(to);
891+
netif_addr_lock_nested(to);
892892
err = __hw_addr_sync_multiple(&to->mc, &from->mc, to->addr_len);
893893
if (!err)
894894
__dev_set_rx_mode(to);
@@ -912,7 +912,7 @@ void dev_mc_unsync(struct net_device *to, struct net_device *from)
912912
return;
913913

914914
netif_addr_lock_bh(from);
915-
netif_addr_lock(to);
915+
netif_addr_lock_nested(to);
916916
__hw_addr_unsync(&to->mc, &from->mc, to->addr_len);
917917
__dev_set_rx_mode(to);
918918
netif_addr_unlock(to);

Diff for: ‎net/core/rtnetlink.c

-1
Original file line numberDiff line numberDiff line change
@@ -2462,7 +2462,6 @@ static int do_set_master(struct net_device *dev, int ifindex,
24622462
err = ops->ndo_del_slave(upper_dev, dev);
24632463
if (err)
24642464
return err;
2465-
netdev_update_lockdep_key(dev);
24662465
} else {
24672466
return -EOPNOTSUPP;
24682467
}

Diff for: ‎net/dsa/master.c

+4
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,8 @@ static void dsa_master_reset_mtu(struct net_device *dev)
327327
rtnl_unlock();
328328
}
329329

330+
static struct lock_class_key dsa_master_addr_list_lock_key;
331+
330332
int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
331333
{
332334
int ret;
@@ -345,6 +347,8 @@ int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
345347
wmb();
346348

347349
dev->dsa_ptr = cpu_dp;
350+
lockdep_set_class(&dev->addr_list_lock,
351+
&dsa_master_addr_list_lock_key);
348352
ret = dsa_master_ethtool_setup(dev);
349353
if (ret)
350354
return ret;

0 commit comments

Comments
 (0)