Skip to content

Commit b9a1260

Browse files
committed
Merge branch 'Close-race-between-un-register_netdevice_notifier-and-pernet_operations'
Kirill Tkhai says: ==================== Close race between {un, }register_netdevice_notifier and pernet_operations the problem is {,un}register_netdevice_notifier() do not take pernet_ops_rwsem, and they don't see network namespaces, being initialized in setup_net() and cleanup_net(), since at this time net is not hashed to net_namespace_list. This may lead to imbalance, when a notifier is called at time of setup_net()/net is alive, but it's not called at time of cleanup_net(), for the devices, hashed to the net, and vise versa. See (3/3) for the scheme of imbalance. This patchset fixes the problem by acquiring pernet_ops_rwsem at the time of {,un}register_netdevice_notifier() (3/3). (1-2/3) are preparations in xfrm and netfilter subsystems. The problem was introduced a long ago, but backporting won't be easy, since every previous kernel version may have changes in netdevice notifiers, and they all need review and testing. Otherwise, there may be more pernet_operations, which register or unregister netdevice notifiers, and that leads to deadlock (which is was fixed in 1-2/3). This patchset is for net-next. ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents caeeeda + 328fbe7 commit b9a1260

File tree

5 files changed

+55
-31
lines changed

5 files changed

+55
-31
lines changed

include/net/xfrm.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1894,7 +1894,7 @@ static inline struct xfrm_offload *xfrm_offload(struct sk_buff *skb)
18941894
#endif
18951895
}
18961896

1897-
void __net_init xfrm_dev_init(void);
1897+
void __init xfrm_dev_init(void);
18981898

18991899
#ifdef CONFIG_XFRM_OFFLOAD
19001900
void xfrm_dev_resume(struct sk_buff *skb);

net/core/dev.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,6 +1625,8 @@ int register_netdevice_notifier(struct notifier_block *nb)
16251625
struct net *net;
16261626
int err;
16271627

1628+
/* Close race with setup_net() and cleanup_net() */
1629+
down_write(&pernet_ops_rwsem);
16281630
rtnl_lock();
16291631
err = raw_notifier_chain_register(&netdev_chain, nb);
16301632
if (err)
@@ -1649,6 +1651,7 @@ int register_netdevice_notifier(struct notifier_block *nb)
16491651

16501652
unlock:
16511653
rtnl_unlock();
1654+
up_write(&pernet_ops_rwsem);
16521655
return err;
16531656

16541657
rollback:
@@ -1694,6 +1697,8 @@ int unregister_netdevice_notifier(struct notifier_block *nb)
16941697
struct net *net;
16951698
int err;
16961699

1700+
/* Close race with setup_net() and cleanup_net() */
1701+
down_write(&pernet_ops_rwsem);
16971702
rtnl_lock();
16981703
err = raw_notifier_chain_unregister(&netdev_chain, nb);
16991704
if (err)
@@ -1713,6 +1718,7 @@ int unregister_netdevice_notifier(struct notifier_block *nb)
17131718
up_read(&net_rwsem);
17141719
unlock:
17151720
rtnl_unlock();
1721+
up_write(&pernet_ops_rwsem);
17161722
return err;
17171723
}
17181724
EXPORT_SYMBOL(unregister_netdevice_notifier);

net/netfilter/xt_TEE.c

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
#include <linux/netfilter/xt_TEE.h>
2121

2222
struct xt_tee_priv {
23-
struct notifier_block notifier;
23+
struct list_head list;
2424
struct xt_tee_tginfo *tginfo;
2525
int oif;
2626
};
@@ -51,29 +51,35 @@ tee_tg6(struct sk_buff *skb, const struct xt_action_param *par)
5151
}
5252
#endif
5353

54+
static DEFINE_MUTEX(priv_list_mutex);
55+
static LIST_HEAD(priv_list);
56+
5457
static int tee_netdev_event(struct notifier_block *this, unsigned long event,
5558
void *ptr)
5659
{
5760
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
5861
struct xt_tee_priv *priv;
5962

60-
priv = container_of(this, struct xt_tee_priv, notifier);
61-
switch (event) {
62-
case NETDEV_REGISTER:
63-
if (!strcmp(dev->name, priv->tginfo->oif))
64-
priv->oif = dev->ifindex;
65-
break;
66-
case NETDEV_UNREGISTER:
67-
if (dev->ifindex == priv->oif)
68-
priv->oif = -1;
69-
break;
70-
case NETDEV_CHANGENAME:
71-
if (!strcmp(dev->name, priv->tginfo->oif))
72-
priv->oif = dev->ifindex;
73-
else if (dev->ifindex == priv->oif)
74-
priv->oif = -1;
75-
break;
63+
mutex_lock(&priv_list_mutex);
64+
list_for_each_entry(priv, &priv_list, list) {
65+
switch (event) {
66+
case NETDEV_REGISTER:
67+
if (!strcmp(dev->name, priv->tginfo->oif))
68+
priv->oif = dev->ifindex;
69+
break;
70+
case NETDEV_UNREGISTER:
71+
if (dev->ifindex == priv->oif)
72+
priv->oif = -1;
73+
break;
74+
case NETDEV_CHANGENAME:
75+
if (!strcmp(dev->name, priv->tginfo->oif))
76+
priv->oif = dev->ifindex;
77+
else if (dev->ifindex == priv->oif)
78+
priv->oif = -1;
79+
break;
80+
}
7681
}
82+
mutex_unlock(&priv_list_mutex);
7783

7884
return NOTIFY_DONE;
7985
}
@@ -89,8 +95,6 @@ static int tee_tg_check(const struct xt_tgchk_param *par)
8995
return -EINVAL;
9096

9197
if (info->oif[0]) {
92-
int ret;
93-
9498
if (info->oif[sizeof(info->oif)-1] != '\0')
9599
return -EINVAL;
96100

@@ -100,14 +104,11 @@ static int tee_tg_check(const struct xt_tgchk_param *par)
100104

101105
priv->tginfo = info;
102106
priv->oif = -1;
103-
priv->notifier.notifier_call = tee_netdev_event;
104107
info->priv = priv;
105108

106-
ret = register_netdevice_notifier(&priv->notifier);
107-
if (ret) {
108-
kfree(priv);
109-
return ret;
110-
}
109+
mutex_lock(&priv_list_mutex);
110+
list_add(&priv->list, &priv_list);
111+
mutex_unlock(&priv_list_mutex);
111112
} else
112113
info->priv = NULL;
113114

@@ -120,7 +121,9 @@ static void tee_tg_destroy(const struct xt_tgdtor_param *par)
120121
struct xt_tee_tginfo *info = par->targinfo;
121122

122123
if (info->priv) {
123-
unregister_netdevice_notifier(&info->priv->notifier);
124+
mutex_lock(&priv_list_mutex);
125+
list_del(&info->priv->list);
126+
mutex_unlock(&priv_list_mutex);
124127
kfree(info->priv);
125128
}
126129
static_key_slow_dec(&xt_tee_enabled);
@@ -153,13 +156,29 @@ static struct xt_target tee_tg_reg[] __read_mostly = {
153156
#endif
154157
};
155158

159+
static struct notifier_block tee_netdev_notifier = {
160+
.notifier_call = tee_netdev_event,
161+
};
162+
156163
static int __init tee_tg_init(void)
157164
{
158-
return xt_register_targets(tee_tg_reg, ARRAY_SIZE(tee_tg_reg));
165+
int ret;
166+
167+
ret = xt_register_targets(tee_tg_reg, ARRAY_SIZE(tee_tg_reg));
168+
if (ret)
169+
return ret;
170+
ret = register_netdevice_notifier(&tee_netdev_notifier);
171+
if (ret) {
172+
xt_unregister_targets(tee_tg_reg, ARRAY_SIZE(tee_tg_reg));
173+
return ret;
174+
}
175+
176+
return 0;
159177
}
160178

161179
static void __exit tee_tg_exit(void)
162180
{
181+
unregister_netdevice_notifier(&tee_netdev_notifier);
163182
xt_unregister_targets(tee_tg_reg, ARRAY_SIZE(tee_tg_reg));
164183
}
165184

net/xfrm/xfrm_device.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ static struct notifier_block xfrm_dev_notifier = {
350350
.notifier_call = xfrm_dev_event,
351351
};
352352

353-
void __net_init xfrm_dev_init(void)
353+
void __init xfrm_dev_init(void)
354354
{
355355
register_netdevice_notifier(&xfrm_dev_notifier);
356356
}

net/xfrm/xfrm_policy.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2895,8 +2895,6 @@ static int __net_init xfrm_policy_init(struct net *net)
28952895
INIT_LIST_HEAD(&net->xfrm.policy_all);
28962896
INIT_WORK(&net->xfrm.policy_hash_work, xfrm_hash_resize);
28972897
INIT_WORK(&net->xfrm.policy_hthresh.work, xfrm_hash_rebuild);
2898-
if (net_eq(net, &init_net))
2899-
xfrm_dev_init();
29002898
return 0;
29012899

29022900
out_bydst:
@@ -2999,6 +2997,7 @@ void __init xfrm_init(void)
29992997
INIT_WORK(&xfrm_pcpu_work[i], xfrm_pcpu_work_fn);
30002998

30012999
register_pernet_subsys(&xfrm_net_ops);
3000+
xfrm_dev_init();
30023001
seqcount_init(&xfrm_policy_hash_generation);
30033002
xfrm_input_init();
30043003
}

0 commit comments

Comments
 (0)