hsr: switch ->dellink() to ->ndo_uninit()
Switching from ->priv_destructor to dellink() has an unexpected consequence: existing RCU readers, that is, hsr_port_get_hsr() callers, may still be able to read the port list. Instead of checking the return value of each hsr_port_get_hsr(), we can just move it to ->ndo_uninit() which is called after device unregister and synchronize_net(), and we still have RTNL lock there. Fixes:b9a1e62740
("hsr: implement dellink to clean up resources") Fixes:edf070a0fb
("hsr: fix a NULL pointer deref in hsr_dev_xmit()") Reported-by: syzbot+097ef84cdc95843fbaa8@syzkaller.appspotmail.com Cc: Arvid Brodin <arvid.brodin@alten.se> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
parent
aa4c0c9091
commit
311633b604
@ -227,13 +227,8 @@ static int hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
|
||||
struct hsr_port *master;
|
||||
|
||||
master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
|
||||
if (master) {
|
||||
skb->dev = master->dev;
|
||||
hsr_forward_skb(skb, master);
|
||||
} else {
|
||||
atomic_long_inc(&dev->tx_dropped);
|
||||
dev_kfree_skb_any(skb);
|
||||
}
|
||||
skb->dev = master->dev;
|
||||
hsr_forward_skb(skb, master);
|
||||
return NETDEV_TX_OK;
|
||||
}
|
||||
|
||||
@ -348,7 +343,11 @@ static void hsr_announce(struct timer_list *t)
|
||||
rcu_read_unlock();
|
||||
}
|
||||
|
||||
void hsr_dev_destroy(struct net_device *hsr_dev)
|
||||
/* This has to be called after all the readers are gone.
|
||||
* Otherwise we would have to check the return value of
|
||||
* hsr_port_get_hsr().
|
||||
*/
|
||||
static void hsr_dev_destroy(struct net_device *hsr_dev)
|
||||
{
|
||||
struct hsr_priv *hsr;
|
||||
struct hsr_port *port;
|
||||
@ -364,8 +363,6 @@ void hsr_dev_destroy(struct net_device *hsr_dev)
|
||||
del_timer_sync(&hsr->prune_timer);
|
||||
del_timer_sync(&hsr->announce_timer);
|
||||
|
||||
synchronize_rcu();
|
||||
|
||||
hsr_del_self_node(&hsr->self_node_db);
|
||||
hsr_del_nodes(&hsr->node_db);
|
||||
}
|
||||
@ -376,6 +373,7 @@ static const struct net_device_ops hsr_device_ops = {
|
||||
.ndo_stop = hsr_dev_close,
|
||||
.ndo_start_xmit = hsr_dev_xmit,
|
||||
.ndo_fix_features = hsr_fix_features,
|
||||
.ndo_uninit = hsr_dev_destroy,
|
||||
};
|
||||
|
||||
static struct device_type hsr_type = {
|
||||
|
@ -14,7 +14,6 @@
|
||||
void hsr_dev_setup(struct net_device *dev);
|
||||
int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
|
||||
unsigned char multicast_spec, u8 protocol_version);
|
||||
void hsr_dev_destroy(struct net_device *hsr_dev);
|
||||
void hsr_check_carrier_and_operstate(struct hsr_priv *hsr);
|
||||
bool is_hsr_master(struct net_device *dev);
|
||||
int hsr_get_max_mtu(struct hsr_priv *hsr);
|
||||
|
@ -69,12 +69,6 @@ static int hsr_newlink(struct net *src_net, struct net_device *dev,
|
||||
return hsr_dev_finalize(dev, link, multicast_spec, hsr_version);
|
||||
}
|
||||
|
||||
static void hsr_dellink(struct net_device *hsr_dev, struct list_head *head)
|
||||
{
|
||||
hsr_dev_destroy(hsr_dev);
|
||||
unregister_netdevice_queue(hsr_dev, head);
|
||||
}
|
||||
|
||||
static int hsr_fill_info(struct sk_buff *skb, const struct net_device *dev)
|
||||
{
|
||||
struct hsr_priv *hsr;
|
||||
@ -119,7 +113,6 @@ static struct rtnl_link_ops hsr_link_ops __read_mostly = {
|
||||
.priv_size = sizeof(struct hsr_priv),
|
||||
.setup = hsr_dev_setup,
|
||||
.newlink = hsr_newlink,
|
||||
.dellink = hsr_dellink,
|
||||
.fill_info = hsr_fill_info,
|
||||
};
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user