Re: [PATCH net] vlan: Fix VLAN 0 memory leak

[Ido Schimmel <idosch@idosch.org>]

On Fri, Jul 28, 2023 at 06:31:52PM +0200, Vlad Buslov wrote:
> The referenced commit intended to fix memleak of VLAN 0 that is implicitly
> created on devices with NETIF_F_HW_VLAN_CTAG_FILTER feature. However, it
> doesn't take into account that the feature can be re-set during the
> netdevice lifetime which will cause memory leak if feature is disabled
> during the device deletion as illustrated by [0]. Fix the leak by
> unconditionally deleting VLAN 0 on NETDEV_DOWN event.

Specifically, what happens is:

> 
> [0]:
> > modprobe 8021q
> > ip l set dev eth2 up

VID 0 is created with reference count of 1

> > ethtool -k eth2 | grep rx-vlan-filter
> rx-vlan-filter: on
> > ethtool -K eth2 rx-vlan-filter off
> > ip l set dev eth2 down

Reference count is not dropped because the feature is off

> > ip l set dev eth2 up

Reference count is not increased because the feature is off. It could
have been increased if this line was preceded by:

ethtool -K eth2 rx-vlan-filter on

> > modprobe -r mlx5_ib
> > modprobe -r mlx5_core

Reference count is not dropped during NETDEV_DOWN because the feature is
off and NETDEV_UNREGISTER only dismantles upper VLAN devices, resulting
in VID 0 being leaked.

> > echo scan > /sys/kernel/debug/kmemleak
> > cat /sys/kernel/debug/kmemleak
> unreferenced object 0xffff888165af1c00 (size 256):
>   comm "ip", pid 1847, jiffies 4294908816 (age 155.892s)
>   hex dump (first 32 bytes):
>     00 80 12 0c 81 88 ff ff 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<0000000081646e58>] kmalloc_trace+0x27/0xc0
>     [<0000000096c47f74>] vlan_vid_add+0x444/0x750
>     [<00000000a7304a26>] vlan_device_event+0x1f1/0x1f20 [8021q]
>     [<00000000a888adcb>] notifier_call_chain+0x97/0x240
>     [<000000005a6ebbb6>] __dev_notify_flags+0xe2/0x250
>     [<00000000d423db72>] dev_change_flags+0xfa/0x170
>     [<0000000048bc9621>] do_setlink+0x84b/0x3140
>     [<0000000087d26a73>] __rtnl_newlink+0x954/0x1550
>     [<00000000f767fdc2>] rtnl_newlink+0x5f/0x90
>     [<0000000093aed008>] rtnetlink_rcv_msg+0x336/0xa40
>     [<000000008d83ca71>] netlink_rcv_skb+0x12c/0x360
>     [<000000006227c8de>] netlink_unicast+0x438/0x710
>     [<00000000957f18cf>] netlink_sendmsg+0x7a0/0xc70
>     [<00000000768833ad>] sock_sendmsg+0xc5/0x190
>     [<0000000048d43666>] ____sys_sendmsg+0x534/0x6b0
>     [<00000000bd83c8d6>] ___sys_sendmsg+0xeb/0x170
> unreferenced object 0xffff888122bb9080 (size 32):
>   comm "ip", pid 1847, jiffies 4294908816 (age 155.892s)
>   hex dump (first 32 bytes):
>     a0 1c af 65 81 88 ff ff a0 1c af 65 81 88 ff ff  ...e.......e....
>     81 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<0000000081646e58>] kmalloc_trace+0x27/0xc0
>     [<00000000174174bb>] vlan_vid_add+0x4fd/0x750
>     [<00000000a7304a26>] vlan_device_event+0x1f1/0x1f20 [8021q]
>     [<00000000a888adcb>] notifier_call_chain+0x97/0x240
>     [<000000005a6ebbb6>] __dev_notify_flags+0xe2/0x250
>     [<00000000d423db72>] dev_change_flags+0xfa/0x170
>     [<0000000048bc9621>] do_setlink+0x84b/0x3140
>     [<0000000087d26a73>] __rtnl_newlink+0x954/0x1550
>     [<00000000f767fdc2>] rtnl_newlink+0x5f/0x90
>     [<0000000093aed008>] rtnetlink_rcv_msg+0x336/0xa40
>     [<000000008d83ca71>] netlink_rcv_skb+0x12c/0x360
>     [<000000006227c8de>] netlink_unicast+0x438/0x710
>     [<00000000957f18cf>] netlink_sendmsg+0x7a0/0xc70
>     [<00000000768833ad>] sock_sendmsg+0xc5/0x190
>     [<0000000048d43666>] ____sys_sendmsg+0x534/0x6b0
>     [<00000000bd83c8d6>] ___sys_sendmsg+0xeb/0x170
> 
> Fixes: efc73f4bbc23 ("net: Fix memory leak - vlan_info struct")
> Signed-off-by: Vlad Buslov <vladbu@nvidia.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>