* [PATCH net] vlan: Fix VLAN 0 memory leak
@ 2023-07-28 16:31 Vlad Buslov
2023-07-30 15:30 ` Ido Schimmel
0 siblings, 1 reply; 5+ messages in thread
From: Vlad Buslov @ 2023-07-28 16:31 UTC (permalink / raw)
To: davem, kuba, edumazet, pabeni
Cc: netdev, amir.hanania, jeffrey.t.kirsher, john.fastabend, idosch,
Vlad Buslov
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.
[0]:
> modprobe 8021q
> ip l set dev eth2 up
> ethtool -k eth2 | grep rx-vlan-filter
rx-vlan-filter: on
> ethtool -K eth2 rx-vlan-filter off
> ip l set dev eth2 down
> ip l set dev eth2 up
> modprobe -r mlx5_ib
> modprobe -r mlx5_core
> 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>
---
net/8021q/vlan.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index e40aa3e3641c..b3662119ddbc 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -384,8 +384,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
dev->name);
vlan_vid_add(dev, htons(ETH_P_8021Q), 0);
}
- if (event == NETDEV_DOWN &&
- (dev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
+ if (event == NETDEV_DOWN)
vlan_vid_del(dev, htons(ETH_P_8021Q), 0);
vlan_info = rtnl_dereference(dev->vlan_info);
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net] vlan: Fix VLAN 0 memory leak
2023-07-28 16:31 [PATCH net] vlan: Fix VLAN 0 memory leak Vlad Buslov
@ 2023-07-30 15:30 ` Ido Schimmel
2023-07-31 9:52 ` Simon Horman
0 siblings, 1 reply; 5+ messages in thread
From: Ido Schimmel @ 2023-07-30 15:30 UTC (permalink / raw)
To: Vlad Buslov
Cc: davem, kuba, edumazet, pabeni, netdev, amir.hanania,
jeffrey.t.kirsher, john.fastabend
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>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] vlan: Fix VLAN 0 memory leak
2023-07-30 15:30 ` Ido Schimmel
@ 2023-07-31 9:52 ` Simon Horman
2023-07-31 15:45 ` Ido Schimmel
0 siblings, 1 reply; 5+ messages in thread
From: Simon Horman @ 2023-07-31 9:52 UTC (permalink / raw)
To: Ido Schimmel
Cc: Vlad Buslov, davem, kuba, edumazet, pabeni, netdev, amir.hanania,
jeffrey.t.kirsher, john.fastabend
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] vlan: Fix VLAN 0 memory leak
2023-07-31 9:52 ` Simon Horman
@ 2023-07-31 15:45 ` Ido Schimmel
2023-07-31 19:11 ` Vlad Buslov
0 siblings, 1 reply; 5+ messages in thread
From: Ido Schimmel @ 2023-07-31 15:45 UTC (permalink / raw)
To: Simon Horman
Cc: Vlad Buslov, davem, kuba, edumazet, pabeni, netdev, amir.hanania,
jeffrey.t.kirsher, john.fastabend
On Mon, Jul 31, 2023 at 11:52:19AM +0200, Simon Horman wrote:
> 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.
I agree (assuming my analysis is correct).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] vlan: Fix VLAN 0 memory leak
2023-07-31 15:45 ` Ido Schimmel
@ 2023-07-31 19:11 ` Vlad Buslov
0 siblings, 0 replies; 5+ messages in thread
From: Vlad Buslov @ 2023-07-31 19:11 UTC (permalink / raw)
To: Ido Schimmel
Cc: Simon Horman, davem, kuba, edumazet, pabeni, netdev, amir.hanania,
jeffrey.t.kirsher, john.fastabend
On Mon 31 Jul 2023 at 18:45, Ido Schimmel <idosch@idosch.org> wrote:
> On Mon, Jul 31, 2023 at 11:52:19AM +0200, Simon Horman wrote:
>> 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.
>
> I agree (assuming my analysis is correct).
Sorry for making it more convoluted than necessary. Just disabling HW
vlan feature on the device and removing the module is enough for repro:
# modprobe 8021q
# ip l set dev eth2 up
# ethtool -K eth2 rx-vlan-filter off
# modprobe -r mlx5_ib
# modprobe -r mlx5_core
# cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff888103dcd900 (size 256):
comm "ip", pid 1490, jiffies 4294907305 (age 325.364s)
hex dump (first 32 bytes):
00 80 5d 03 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:
[<00000000899f3bb9>] kmalloc_trace+0x25/0x80
[<000000002889a7a2>] vlan_vid_add+0xa0/0x210
[<000000007177800e>] vlan_device_event+0x374/0x760 [8021q]
[<000000009a0716b1>] notifier_call_chain+0x35/0xb0
[<00000000bbf3d162>] __dev_notify_flags+0x58/0xf0
[<0000000053d2b05d>] dev_change_flags+0x4d/0x60
[<00000000982807e9>] do_setlink+0x28d/0x10a0
[<0000000058c1be00>] __rtnl_newlink+0x545/0x980
[<00000000e66c3bd9>] rtnl_newlink+0x44/0x70
[<00000000a2cc5970>] rtnetlink_rcv_msg+0x29c/0x390
[<00000000d307d1e4>] netlink_rcv_skb+0x54/0x100
[<00000000259d16f9>] netlink_unicast+0x1f6/0x2c0
[<000000007ce2afa1>] netlink_sendmsg+0x232/0x4a0
[<00000000f3f4bb39>] sock_sendmsg+0x38/0x60
[<000000002f9c0624>] ____sys_sendmsg+0x1e3/0x200
[<00000000d6ff5520>] ___sys_sendmsg+0x80/0xc0
unreferenced object 0xffff88813354fde0 (size 32):
comm "ip", pid 1490, jiffies 4294907305 (age 325.364s)
hex dump (first 32 bytes):
a0 d9 dc 03 81 88 ff ff a0 d9 dc 03 81 88 ff ff ................
81 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000899f3bb9>] kmalloc_trace+0x25/0x80
[<000000002da64724>] vlan_vid_add+0xdf/0x210
[<000000007177800e>] vlan_device_event+0x374/0x760 [8021q]
[<000000009a0716b1>] notifier_call_chain+0x35/0xb0
[<00000000bbf3d162>] __dev_notify_flags+0x58/0xf0
[<0000000053d2b05d>] dev_change_flags+0x4d/0x60
[<00000000982807e9>] do_setlink+0x28d/0x10a0
[<0000000058c1be00>] __rtnl_newlink+0x545/0x980
[<00000000e66c3bd9>] rtnl_newlink+0x44/0x70
[<00000000a2cc5970>] rtnetlink_rcv_msg+0x29c/0x390
[<00000000d307d1e4>] netlink_rcv_skb+0x54/0x100
[<00000000259d16f9>] netlink_unicast+0x1f6/0x2c0
[<000000007ce2afa1>] netlink_sendmsg+0x232/0x4a0
[<00000000f3f4bb39>] sock_sendmsg+0x38/0x60
[<000000002f9c0624>] ____sys_sendmsg+0x1e3/0x200
[<00000000d6ff5520>] ___sys_sendmsg+0x80/0xc0
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-31 19:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-28 16:31 [PATCH net] vlan: Fix VLAN 0 memory leak Vlad Buslov
2023-07-30 15:30 ` Ido Schimmel
2023-07-31 9:52 ` Simon Horman
2023-07-31 15:45 ` Ido Schimmel
2023-07-31 19:11 ` Vlad Buslov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).