* [PATCH net] bonding: Fix memory leak when changing bond type to Ethernet
@ 2023-04-17 6:12 Ido Schimmel
2023-04-19 0:00 ` Jay Vosburgh
2023-04-19 8:00 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Ido Schimmel @ 2023-04-17 6:12 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, j.vosburgh, andy, monis, razor,
mirsad.todorovac, Ido Schimmel
When a net device is put administratively up, its 'IFF_UP' flag is set
(if not set already) and a 'NETDEV_UP' notification is emitted, which
causes the 8021q driver to add VLAN ID 0 on the device. The reverse
happens when a net device is put administratively down.
When changing the type of a bond to Ethernet, its 'IFF_UP' flag is
incorrectly cleared, resulting in the kernel skipping the above process
and VLAN ID 0 being leaked [1].
Fix by restoring the flag when changing the type to Ethernet, in a
similar fashion to the restoration of the 'IFF_SLAVE' flag.
The issue can be reproduced using the script in [2], with example out
before and after the fix in [3].
[1]
unreferenced object 0xffff888103479900 (size 256):
comm "ip", pid 329, jiffies 4294775225 (age 28.561s)
hex dump (first 32 bytes):
00 a0 0c 15 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:
[<ffffffff81a6051a>] kmalloc_trace+0x2a/0xe0
[<ffffffff8406426c>] vlan_vid_add+0x30c/0x790
[<ffffffff84068e21>] vlan_device_event+0x1491/0x21a0
[<ffffffff81440c8e>] notifier_call_chain+0xbe/0x1f0
[<ffffffff8372383a>] call_netdevice_notifiers_info+0xba/0x150
[<ffffffff837590f2>] __dev_notify_flags+0x132/0x2e0
[<ffffffff8375ad9f>] dev_change_flags+0x11f/0x180
[<ffffffff8379af36>] do_setlink+0xb96/0x4060
[<ffffffff837adf6a>] __rtnl_newlink+0xc0a/0x18a0
[<ffffffff837aec6c>] rtnl_newlink+0x6c/0xa0
[<ffffffff837ac64e>] rtnetlink_rcv_msg+0x43e/0xe00
[<ffffffff839a99e0>] netlink_rcv_skb+0x170/0x440
[<ffffffff839a738f>] netlink_unicast+0x53f/0x810
[<ffffffff839a7fcb>] netlink_sendmsg+0x96b/0xe90
[<ffffffff8369d12f>] ____sys_sendmsg+0x30f/0xa70
[<ffffffff836a6d7a>] ___sys_sendmsg+0x13a/0x1e0
unreferenced object 0xffff88810f6a83e0 (size 32):
comm "ip", pid 329, jiffies 4294775225 (age 28.561s)
hex dump (first 32 bytes):
a0 99 47 03 81 88 ff ff a0 99 47 03 81 88 ff ff ..G.......G.....
81 00 00 00 01 00 00 00 cc cc cc cc cc cc cc cc ................
backtrace:
[<ffffffff81a6051a>] kmalloc_trace+0x2a/0xe0
[<ffffffff84064369>] vlan_vid_add+0x409/0x790
[<ffffffff84068e21>] vlan_device_event+0x1491/0x21a0
[<ffffffff81440c8e>] notifier_call_chain+0xbe/0x1f0
[<ffffffff8372383a>] call_netdevice_notifiers_info+0xba/0x150
[<ffffffff837590f2>] __dev_notify_flags+0x132/0x2e0
[<ffffffff8375ad9f>] dev_change_flags+0x11f/0x180
[<ffffffff8379af36>] do_setlink+0xb96/0x4060
[<ffffffff837adf6a>] __rtnl_newlink+0xc0a/0x18a0
[<ffffffff837aec6c>] rtnl_newlink+0x6c/0xa0
[<ffffffff837ac64e>] rtnetlink_rcv_msg+0x43e/0xe00
[<ffffffff839a99e0>] netlink_rcv_skb+0x170/0x440
[<ffffffff839a738f>] netlink_unicast+0x53f/0x810
[<ffffffff839a7fcb>] netlink_sendmsg+0x96b/0xe90
[<ffffffff8369d12f>] ____sys_sendmsg+0x30f/0xa70
[<ffffffff836a6d7a>] ___sys_sendmsg+0x13a/0x1e0
[2]
ip link add name t-nlmon type nlmon
ip link add name t-dummy type dummy
ip link add name t-bond type bond mode active-backup
ip link set dev t-bond up
ip link set dev t-nlmon master t-bond
ip link set dev t-nlmon nomaster
ip link show dev t-bond
ip link set dev t-dummy master t-bond
ip link show dev t-bond
ip link del dev t-bond
ip link del dev t-dummy
ip link del dev t-nlmon
[3]
Before:
12: t-bond: <NO-CARRIER,BROADCAST,MULTICAST,MASTER,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
link/netlink
12: t-bond: <BROADCAST,MULTICAST,MASTER,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
link/ether 46:57:39:a4:46:a2 brd ff:ff:ff:ff:ff:ff
After:
12: t-bond: <NO-CARRIER,BROADCAST,MULTICAST,MASTER,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
link/netlink
12: t-bond: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
link/ether 66:48:7b:74:b6:8a brd ff:ff:ff:ff:ff:ff
Fixes: e36b9d16c6a6 ("bonding: clean muticast addresses when device changes type")
Fixes: 75c78500ddad ("bonding: remap muticast addresses without using dev_close() and dev_open()")
Fixes: 9ec7eb60dcbc ("bonding: restore IFF_MASTER/SLAVE flags on bond enslave ether type change")
Reported-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/netdev/78a8a03b-6070-3e6b-5042-f848dab16fb8@alu.unizg.hr/
Tested-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
drivers/net/bonding/bond_main.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8cc9a74789b7..7a7d584f378a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1777,14 +1777,15 @@ void bond_lower_state_changed(struct slave *slave)
/* The bonding driver uses ether_setup() to convert a master bond device
* to ARPHRD_ETHER, that resets the target netdevice's flags so we always
- * have to restore the IFF_MASTER flag, and only restore IFF_SLAVE if it was set
+ * have to restore the IFF_MASTER flag, and only restore IFF_SLAVE and IFF_UP
+ * if they were set
*/
static void bond_ether_setup(struct net_device *bond_dev)
{
- unsigned int slave_flag = bond_dev->flags & IFF_SLAVE;
+ unsigned int flags = bond_dev->flags & (IFF_SLAVE | IFF_UP);
ether_setup(bond_dev);
- bond_dev->flags |= IFF_MASTER | slave_flag;
+ bond_dev->flags |= IFF_MASTER | flags;
bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
}
--
2.37.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] bonding: Fix memory leak when changing bond type to Ethernet
2023-04-17 6:12 [PATCH net] bonding: Fix memory leak when changing bond type to Ethernet Ido Schimmel
@ 2023-04-19 0:00 ` Jay Vosburgh
2023-04-19 8:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Jay Vosburgh @ 2023-04-19 0:00 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, kuba, pabeni, edumazet, andy, monis, razor,
mirsad.todorovac
Ido Schimmel <idosch@nvidia.com> wrote:
>When a net device is put administratively up, its 'IFF_UP' flag is set
>(if not set already) and a 'NETDEV_UP' notification is emitted, which
>causes the 8021q driver to add VLAN ID 0 on the device. The reverse
>happens when a net device is put administratively down.
>
>When changing the type of a bond to Ethernet, its 'IFF_UP' flag is
>incorrectly cleared, resulting in the kernel skipping the above process
>and VLAN ID 0 being leaked [1].
>
>Fix by restoring the flag when changing the type to Ethernet, in a
>similar fashion to the restoration of the 'IFF_SLAVE' flag.
>
>The issue can be reproduced using the script in [2], with example out
>before and after the fix in [3].
>
>[1]
>unreferenced object 0xffff888103479900 (size 256):
> comm "ip", pid 329, jiffies 4294775225 (age 28.561s)
> hex dump (first 32 bytes):
> 00 a0 0c 15 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:
> [<ffffffff81a6051a>] kmalloc_trace+0x2a/0xe0
> [<ffffffff8406426c>] vlan_vid_add+0x30c/0x790
> [<ffffffff84068e21>] vlan_device_event+0x1491/0x21a0
> [<ffffffff81440c8e>] notifier_call_chain+0xbe/0x1f0
> [<ffffffff8372383a>] call_netdevice_notifiers_info+0xba/0x150
> [<ffffffff837590f2>] __dev_notify_flags+0x132/0x2e0
> [<ffffffff8375ad9f>] dev_change_flags+0x11f/0x180
> [<ffffffff8379af36>] do_setlink+0xb96/0x4060
> [<ffffffff837adf6a>] __rtnl_newlink+0xc0a/0x18a0
> [<ffffffff837aec6c>] rtnl_newlink+0x6c/0xa0
> [<ffffffff837ac64e>] rtnetlink_rcv_msg+0x43e/0xe00
> [<ffffffff839a99e0>] netlink_rcv_skb+0x170/0x440
> [<ffffffff839a738f>] netlink_unicast+0x53f/0x810
> [<ffffffff839a7fcb>] netlink_sendmsg+0x96b/0xe90
> [<ffffffff8369d12f>] ____sys_sendmsg+0x30f/0xa70
> [<ffffffff836a6d7a>] ___sys_sendmsg+0x13a/0x1e0
>unreferenced object 0xffff88810f6a83e0 (size 32):
> comm "ip", pid 329, jiffies 4294775225 (age 28.561s)
> hex dump (first 32 bytes):
> a0 99 47 03 81 88 ff ff a0 99 47 03 81 88 ff ff ..G.......G.....
> 81 00 00 00 01 00 00 00 cc cc cc cc cc cc cc cc ................
> backtrace:
> [<ffffffff81a6051a>] kmalloc_trace+0x2a/0xe0
> [<ffffffff84064369>] vlan_vid_add+0x409/0x790
> [<ffffffff84068e21>] vlan_device_event+0x1491/0x21a0
> [<ffffffff81440c8e>] notifier_call_chain+0xbe/0x1f0
> [<ffffffff8372383a>] call_netdevice_notifiers_info+0xba/0x150
> [<ffffffff837590f2>] __dev_notify_flags+0x132/0x2e0
> [<ffffffff8375ad9f>] dev_change_flags+0x11f/0x180
> [<ffffffff8379af36>] do_setlink+0xb96/0x4060
> [<ffffffff837adf6a>] __rtnl_newlink+0xc0a/0x18a0
> [<ffffffff837aec6c>] rtnl_newlink+0x6c/0xa0
> [<ffffffff837ac64e>] rtnetlink_rcv_msg+0x43e/0xe00
> [<ffffffff839a99e0>] netlink_rcv_skb+0x170/0x440
> [<ffffffff839a738f>] netlink_unicast+0x53f/0x810
> [<ffffffff839a7fcb>] netlink_sendmsg+0x96b/0xe90
> [<ffffffff8369d12f>] ____sys_sendmsg+0x30f/0xa70
> [<ffffffff836a6d7a>] ___sys_sendmsg+0x13a/0x1e0
>
>[2]
>ip link add name t-nlmon type nlmon
>ip link add name t-dummy type dummy
>ip link add name t-bond type bond mode active-backup
>
>ip link set dev t-bond up
>ip link set dev t-nlmon master t-bond
>ip link set dev t-nlmon nomaster
>ip link show dev t-bond
>ip link set dev t-dummy master t-bond
>ip link show dev t-bond
>
>ip link del dev t-bond
>ip link del dev t-dummy
>ip link del dev t-nlmon
>
>[3]
>Before:
>
>12: t-bond: <NO-CARRIER,BROADCAST,MULTICAST,MASTER,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
> link/netlink
>12: t-bond: <BROADCAST,MULTICAST,MASTER,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
> link/ether 46:57:39:a4:46:a2 brd ff:ff:ff:ff:ff:ff
>
>After:
>
>12: t-bond: <NO-CARRIER,BROADCAST,MULTICAST,MASTER,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
> link/netlink
>12: t-bond: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
> link/ether 66:48:7b:74:b6:8a brd ff:ff:ff:ff:ff:ff
>
>Fixes: e36b9d16c6a6 ("bonding: clean muticast addresses when device changes type")
>Fixes: 75c78500ddad ("bonding: remap muticast addresses without using dev_close() and dev_open()")
>Fixes: 9ec7eb60dcbc ("bonding: restore IFF_MASTER/SLAVE flags on bond enslave ether type change")
>Reported-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
>Link: https://lore.kernel.org/netdev/78a8a03b-6070-3e6b-5042-f848dab16fb8@alu.unizg.hr/
>Tested-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
>Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Note that this has nothing to do with nlmon specifically, it's
related to anything that's not ARPHRD_ETHER.
Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
-J
>---
> drivers/net/bonding/bond_main.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 8cc9a74789b7..7a7d584f378a 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1777,14 +1777,15 @@ void bond_lower_state_changed(struct slave *slave)
>
> /* The bonding driver uses ether_setup() to convert a master bond device
> * to ARPHRD_ETHER, that resets the target netdevice's flags so we always
>- * have to restore the IFF_MASTER flag, and only restore IFF_SLAVE if it was set
>+ * have to restore the IFF_MASTER flag, and only restore IFF_SLAVE and IFF_UP
>+ * if they were set
> */
> static void bond_ether_setup(struct net_device *bond_dev)
> {
>- unsigned int slave_flag = bond_dev->flags & IFF_SLAVE;
>+ unsigned int flags = bond_dev->flags & (IFF_SLAVE | IFF_UP);
>
> ether_setup(bond_dev);
>- bond_dev->flags |= IFF_MASTER | slave_flag;
>+ bond_dev->flags |= IFF_MASTER | flags;
> bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
> }
>
>--
>2.37.3
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] bonding: Fix memory leak when changing bond type to Ethernet
2023-04-17 6:12 [PATCH net] bonding: Fix memory leak when changing bond type to Ethernet Ido Schimmel
2023-04-19 0:00 ` Jay Vosburgh
@ 2023-04-19 8:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-04-19 8:00 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, kuba, pabeni, edumazet, j.vosburgh, andy, monis,
razor, mirsad.todorovac
Hello:
This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:
On Mon, 17 Apr 2023 09:12:16 +0300 you wrote:
> When a net device is put administratively up, its 'IFF_UP' flag is set
> (if not set already) and a 'NETDEV_UP' notification is emitted, which
> causes the 8021q driver to add VLAN ID 0 on the device. The reverse
> happens when a net device is put administratively down.
>
> When changing the type of a bond to Ethernet, its 'IFF_UP' flag is
> incorrectly cleared, resulting in the kernel skipping the above process
> and VLAN ID 0 being leaked [1].
>
> [...]
Here is the summary with links:
- [net] bonding: Fix memory leak when changing bond type to Ethernet
https://git.kernel.org/netdev/net/c/c484fcc058ba
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-04-19 8:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-17 6:12 [PATCH net] bonding: Fix memory leak when changing bond type to Ethernet Ido Schimmel
2023-04-19 0:00 ` Jay Vosburgh
2023-04-19 8:00 ` patchwork-bot+netdevbpf
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).