Skip to content

Commit 052d41c

Browse files
congwangdavem330
authored andcommitted
vlan: fix a use-after-free in vlan_device_event()
After refcnt reaches zero, vlan_vid_del() could free dev->vlan_info via RCU: RCU_INIT_POINTER(dev->vlan_info, NULL); call_rcu(&vlan_info->rcu, vlan_info_rcu_free); However, the pointer 'grp' still points to that memory since it is set before vlan_vid_del(): vlan_info = rtnl_dereference(dev->vlan_info); if (!vlan_info) goto out; grp = &vlan_info->grp; Depends on when that RCU callback is scheduled, we could trigger a use-after-free in vlan_group_for_each_dev() right following this vlan_vid_del(). Fix it by moving vlan_vid_del() before setting grp. This is also symmetric to the vlan_vid_add() we call in vlan_device_event(). Reported-by: Fengguang Wu <[email protected]> Fixes: efc73f4 ("net: Fix memory leak - vlan_info struct") Cc: Alexander Duyck <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Girish Moodalbail <[email protected]> Signed-off-by: Cong Wang <[email protected]> Reviewed-by: Girish Moodalbail <[email protected]> Tested-by: Fengguang Wu <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 2118df9 commit 052d41c

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

net/8021q/vlan.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,9 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
376376
dev->name);
377377
vlan_vid_add(dev, htons(ETH_P_8021Q), 0);
378378
}
379+
if (event == NETDEV_DOWN &&
380+
(dev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
381+
vlan_vid_del(dev, htons(ETH_P_8021Q), 0);
379382

380383
vlan_info = rtnl_dereference(dev->vlan_info);
381384
if (!vlan_info)
@@ -423,9 +426,6 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
423426
struct net_device *tmp;
424427
LIST_HEAD(close_list);
425428

426-
if (dev->features & NETIF_F_HW_VLAN_CTAG_FILTER)
427-
vlan_vid_del(dev, htons(ETH_P_8021Q), 0);
428-
429429
/* Put all VLANs for this dev in the down state too. */
430430
vlan_group_for_each_dev(grp, i, vlandev) {
431431
flgs = vlandev->flags;

0 commit comments

Comments
 (0)