Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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: b9a1e627405d ("hsr: implement dellink to clean up resources")
Fixes: edf070a0fb45 ("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>

authored by

Cong Wang and committed by
David S. Miller
311633b6 aa4c0c90

+8 -18
+8 -10
net/hsr/hsr_device.c
··· 227 227 struct hsr_port *master; 228 228 229 229 master = hsr_port_get_hsr(hsr, HSR_PT_MASTER); 230 - if (master) { 231 - skb->dev = master->dev; 232 - hsr_forward_skb(skb, master); 233 - } else { 234 - atomic_long_inc(&dev->tx_dropped); 235 - dev_kfree_skb_any(skb); 236 - } 230 + skb->dev = master->dev; 231 + hsr_forward_skb(skb, master); 237 232 return NETDEV_TX_OK; 238 233 } 239 234 ··· 343 348 rcu_read_unlock(); 344 349 } 345 350 346 - void hsr_dev_destroy(struct net_device *hsr_dev) 351 + /* This has to be called after all the readers are gone. 352 + * Otherwise we would have to check the return value of 353 + * hsr_port_get_hsr(). 354 + */ 355 + static void hsr_dev_destroy(struct net_device *hsr_dev) 347 356 { 348 357 struct hsr_priv *hsr; 349 358 struct hsr_port *port; ··· 363 364 del_timer_sync(&hsr->prune_timer); 364 365 del_timer_sync(&hsr->announce_timer); 365 366 366 - synchronize_rcu(); 367 - 368 367 hsr_del_self_node(&hsr->self_node_db); 369 368 hsr_del_nodes(&hsr->node_db); 370 369 } ··· 373 376 .ndo_stop = hsr_dev_close, 374 377 .ndo_start_xmit = hsr_dev_xmit, 375 378 .ndo_fix_features = hsr_fix_features, 379 + .ndo_uninit = hsr_dev_destroy, 376 380 }; 377 381 378 382 static struct device_type hsr_type = {
-1
net/hsr/hsr_device.h
··· 14 14 void hsr_dev_setup(struct net_device *dev); 15 15 int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2], 16 16 unsigned char multicast_spec, u8 protocol_version); 17 - void hsr_dev_destroy(struct net_device *hsr_dev); 18 17 void hsr_check_carrier_and_operstate(struct hsr_priv *hsr); 19 18 bool is_hsr_master(struct net_device *dev); 20 19 int hsr_get_max_mtu(struct hsr_priv *hsr);
-7
net/hsr/hsr_netlink.c
··· 69 69 return hsr_dev_finalize(dev, link, multicast_spec, hsr_version); 70 70 } 71 71 72 - static void hsr_dellink(struct net_device *hsr_dev, struct list_head *head) 73 - { 74 - hsr_dev_destroy(hsr_dev); 75 - unregister_netdevice_queue(hsr_dev, head); 76 - } 77 - 78 72 static int hsr_fill_info(struct sk_buff *skb, const struct net_device *dev) 79 73 { 80 74 struct hsr_priv *hsr; ··· 113 119 .priv_size = sizeof(struct hsr_priv), 114 120 .setup = hsr_dev_setup, 115 121 .newlink = hsr_newlink, 116 - .dellink = hsr_dellink, 117 122 .fill_info = hsr_fill_info, 118 123 }; 119 124