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]

Message-ID: <ZMeEU/Aqq0ljY8NE@kernel.org>
Date: Mon, 31 Jul 2023 11:52:19 +0200
From: Simon Horman <horms@...nel.org>
To: Ido Schimmel <idosch@...sch.org>
Cc: Vlad Buslov <vladbu@...dia.com>, 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 Sun, Jul 30, 2023 at 06:30:15PM +0300, Ido Schimmel wrote:
> 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.

Thanks Ido and Vlad,

perhaps it would be worth including the information added
by Ido above in the patch description. Not a hard requirement
from my side, just an idea.