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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 30 Jul 2023 18:30:15 +0300
From: Ido Schimmel <idosch@...sch.org>
To: Vlad Buslov <vladbu@...dia.com>
Cc: davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com,
	pabeni@...hat.com, netdev@...r.kernel.org, amir.hanania@...el.com,
	jeffrey.t.kirsher@...el.com, john.fastabend@...il.com
Subject: Re: [PATCH net] vlan: Fix VLAN 0 memory leak

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@...dia.com>

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