Skip to content

Commit 579d4f9

Browse files
Dong Chenchenkuba-moo
authored andcommitted
net: vlan: fix VLAN 0 refcount imbalance of toggling filtering during runtime
Assuming the "rx-vlan-filter" feature is enabled on a net device, the 8021q module will automatically add or remove VLAN 0 when the net device is put administratively up or down, respectively. There are a couple of problems with the above scheme. The first problem is a memory leak that can happen if the "rx-vlan-filter" feature is disabled while the device is running: # ip link add bond1 up type bond mode 0 # ethtool -K bond1 rx-vlan-filter off # ip link del dev bond1 When the device is put administratively down the "rx-vlan-filter" feature is disabled, so the 8021q module will not remove VLAN 0 and the memory will be leaked [1]. Another problem that can happen is that the kernel can automatically delete VLAN 0 when the device is put administratively down despite not adding it when the device was put administratively up since during that time the "rx-vlan-filter" feature was disabled. null-ptr-unref or bug_on[2] will be triggered by unregister_vlan_dev() for refcount imbalance if toggling filtering during runtime: $ ip link add bond0 type bond mode 0 $ ip link add link bond0 name vlan0 type vlan id 0 protocol 802.1q $ ethtool -K bond0 rx-vlan-filter off $ ifconfig bond0 up $ ethtool -K bond0 rx-vlan-filter on $ ifconfig bond0 down $ ip link del vlan0 Root cause is as below: step1: add vlan0 for real_dev, such as bond, team. register_vlan_dev vlan_vid_add(real_dev,htons(ETH_P_8021Q),0) //refcnt=1 step2: disable vlan filter feature and enable real_dev step3: change filter from 0 to 1 vlan_device_event vlan_filter_push_vids ndo_vlan_rx_add_vid //No refcnt added to real_dev vlan0 step4: real_dev down vlan_device_event vlan_vid_del(dev, htons(ETH_P_8021Q), 0); //refcnt=0 vlan_info_rcu_free //free vlan0 step5: delete vlan0 unregister_vlan_dev BUG_ON(!vlan_info); //vlan_info is null Fix both problems by noting in the VLAN info whether VLAN 0 was automatically added upon NETDEV_UP and based on that decide whether it should be deleted upon NETDEV_DOWN, regardless of the state of the "rx-vlan-filter" feature. [1] unreferenced object 0xffff8880068e3100 (size 256): comm "ip", pid 384, jiffies 4296130254 hex dump (first 32 bytes): 00 20 30 0d 80 88 ff ff 00 00 00 00 00 00 00 00 . 0............. 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace (crc 81ce31fa): __kmalloc_cache_noprof+0x2b5/0x340 vlan_vid_add+0x434/0x940 vlan_device_event.cold+0x75/0xa8 notifier_call_chain+0xca/0x150 __dev_notify_flags+0xe3/0x250 rtnl_configure_link+0x193/0x260 rtnl_newlink_create+0x383/0x8e0 __rtnl_newlink+0x22c/0xa40 rtnl_newlink+0x627/0xb00 rtnetlink_rcv_msg+0x6fb/0xb70 netlink_rcv_skb+0x11f/0x350 netlink_unicast+0x426/0x710 netlink_sendmsg+0x75a/0xc20 __sock_sendmsg+0xc1/0x150 ____sys_sendmsg+0x5aa/0x7b0 ___sys_sendmsg+0xfc/0x180 [2] kernel BUG at net/8021q/vlan.c:99! Oops: invalid opcode: 0000 [#1] SMP KASAN PTI CPU: 0 UID: 0 PID: 382 Comm: ip Not tainted 6.16.0-rc3 #61 PREEMPT(voluntary) Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014 RIP: 0010:unregister_vlan_dev (net/8021q/vlan.c:99 (discriminator 1)) RSP: 0018:ffff88810badf310 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff88810da84000 RCX: ffffffffb47ceb9a RDX: dffffc0000000000 RSI: 0000000000000008 RDI: ffff88810e8b43c8 RBP: 0000000000000000 R08: 0000000000000000 R09: fffffbfff6cefe80 R10: ffffffffb677f407 R11: ffff88810badf3c0 R12: ffff88810e8b4000 R13: 0000000000000000 R14: ffff88810642a5c0 R15: 000000000000017e FS: 00007f1ff68c20c0(0000) GS:ffff888163a24000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f1ff5dad240 CR3: 0000000107e56000 CR4: 00000000000006f0 Call Trace: <TASK> rtnl_dellink (net/core/rtnetlink.c:3511 net/core/rtnetlink.c:3553) rtnetlink_rcv_msg (net/core/rtnetlink.c:6945) netlink_rcv_skb (net/netlink/af_netlink.c:2535) netlink_unicast (net/netlink/af_netlink.c:1314 net/netlink/af_netlink.c:1339) netlink_sendmsg (net/netlink/af_netlink.c:1883) ____sys_sendmsg (net/socket.c:712 net/socket.c:727 net/socket.c:2566) ___sys_sendmsg (net/socket.c:2622) __sys_sendmsg (net/socket.c:2652) do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94) Fixes: ad1afb0 ("vlan_dev: VLAN 0 should be treated as "no vlan tag" (802.1p packet)") Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=a8b046e462915c65b10b Suggested-by: Ido Schimmel <[email protected]> Signed-off-by: Dong Chenchen <[email protected]> Reviewed-by: Ido Schimmel <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent afb5bef commit 579d4f9

File tree

2 files changed

+34
-9
lines changed

2 files changed

+34
-9
lines changed

net/8021q/vlan.c

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,35 @@ static int __vlan_device_event(struct net_device *dev, unsigned long event)
357357
return err;
358358
}
359359

360+
static void vlan_vid0_add(struct net_device *dev)
361+
{
362+
struct vlan_info *vlan_info;
363+
int err;
364+
365+
if (!(dev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
366+
return;
367+
368+
pr_info("adding VLAN 0 to HW filter on device %s\n", dev->name);
369+
370+
err = vlan_vid_add(dev, htons(ETH_P_8021Q), 0);
371+
if (err)
372+
return;
373+
374+
vlan_info = rtnl_dereference(dev->vlan_info);
375+
vlan_info->auto_vid0 = true;
376+
}
377+
378+
static void vlan_vid0_del(struct net_device *dev)
379+
{
380+
struct vlan_info *vlan_info = rtnl_dereference(dev->vlan_info);
381+
382+
if (!vlan_info || !vlan_info->auto_vid0)
383+
return;
384+
385+
vlan_info->auto_vid0 = false;
386+
vlan_vid_del(dev, htons(ETH_P_8021Q), 0);
387+
}
388+
360389
static int vlan_device_event(struct notifier_block *unused, unsigned long event,
361390
void *ptr)
362391
{
@@ -378,15 +407,10 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
378407
return notifier_from_errno(err);
379408
}
380409

381-
if ((event == NETDEV_UP) &&
382-
(dev->features & NETIF_F_HW_VLAN_CTAG_FILTER)) {
383-
pr_info("adding VLAN 0 to HW filter on device %s\n",
384-
dev->name);
385-
vlan_vid_add(dev, htons(ETH_P_8021Q), 0);
386-
}
387-
if (event == NETDEV_DOWN &&
388-
(dev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
389-
vlan_vid_del(dev, htons(ETH_P_8021Q), 0);
410+
if (event == NETDEV_UP)
411+
vlan_vid0_add(dev);
412+
else if (event == NETDEV_DOWN)
413+
vlan_vid0_del(dev);
390414

391415
vlan_info = rtnl_dereference(dev->vlan_info);
392416
if (!vlan_info)

net/8021q/vlan.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ struct vlan_info {
3333
struct vlan_group grp;
3434
struct list_head vid_list;
3535
unsigned int nr_vids;
36+
bool auto_vid0;
3637
struct rcu_head rcu;
3738
};
3839

0 commit comments

Comments
 (0)