* [PATCH net v2] net: bridge: fix memleak in br_add_if()
@ 2021-08-09 9:02 Yang Yingliang
2021-08-09 9:09 ` Nikolay Aleksandrov
0 siblings, 1 reply; 3+ messages in thread
From: Yang Yingliang @ 2021-08-09 9:02 UTC (permalink / raw)
To: linux-kernel, netdev, bridge; +Cc: roopa, nikolay, davem, kuba
I got a memleak report:
BUG: memory leak
unreferenced object 0x607ee521a658 (size 240):
comm "syz-executor.0", pid 955, jiffies 4294780569 (age 16.449s)
hex dump (first 32 bytes, cpu 1):
00 00 00 00 00 00 00 00 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:
[<00000000d830ea5a>] br_multicast_add_port+0x1c2/0x300 net/bridge/br_multicast.c:1693
[<00000000274d9a71>] new_nbp net/bridge/br_if.c:435 [inline]
[<00000000274d9a71>] br_add_if+0x670/0x1740 net/bridge/br_if.c:611
[<0000000012ce888e>] do_set_master net/core/rtnetlink.c:2513 [inline]
[<0000000012ce888e>] do_set_master+0x1aa/0x210 net/core/rtnetlink.c:2487
[<0000000099d1cafc>] __rtnl_newlink+0x1095/0x13e0 net/core/rtnetlink.c:3457
[<00000000a01facc0>] rtnl_newlink+0x64/0xa0 net/core/rtnetlink.c:3488
[<00000000acc9186c>] rtnetlink_rcv_msg+0x369/0xa10 net/core/rtnetlink.c:5550
[<00000000d4aabb9c>] netlink_rcv_skb+0x134/0x3d0 net/netlink/af_netlink.c:2504
[<00000000bc2e12a3>] netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline]
[<00000000bc2e12a3>] netlink_unicast+0x4a0/0x6a0 net/netlink/af_netlink.c:1340
[<00000000e4dc2d0e>] netlink_sendmsg+0x789/0xc70 net/netlink/af_netlink.c:1929
[<000000000d22c8b3>] sock_sendmsg_nosec net/socket.c:654 [inline]
[<000000000d22c8b3>] sock_sendmsg+0x139/0x170 net/socket.c:674
[<00000000e281417a>] ____sys_sendmsg+0x658/0x7d0 net/socket.c:2350
[<00000000237aa2ab>] ___sys_sendmsg+0xf8/0x170 net/socket.c:2404
[<000000004f2dc381>] __sys_sendmsg+0xd3/0x190 net/socket.c:2433
[<0000000005feca6c>] do_syscall_64+0x37/0x90 arch/x86/entry/common.c:47
[<000000007304477d>] entry_SYSCALL_64_after_hwframe+0x44/0xae
On error path of br_add_if(), p->mcast_stats allocated in
new_nbp() need be freed, or it will be leaked.
Fixes: 1080ab95e3c7 ("net: bridge: add support for IGMP/MLD stats and export them via netlink")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
v2:
move free_percpu(p->mcast_stats) in release_nbp() and
fix the compile error when CONFIG_BRIDGE_IGMP_SNOOPING
is disabled.
---
net/bridge/br_if.c | 5 ++++-
net/bridge/br_multicast.c | 1 -
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 6e4a32354a13..c5547772614a 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -250,6 +250,9 @@ static void release_nbp(struct kobject *kobj)
{
struct net_bridge_port *p
= container_of(kobj, struct net_bridge_port, kobj);
+#if IS_ENABLED(CONFIG_BRIDGE_IGMP_SNOOPING)
+ free_percpu(p->mcast_stats);
+#endif
kfree(p);
}
@@ -616,7 +619,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev,
err = dev_set_allmulti(dev, 1);
if (err) {
- kfree(p); /* kobject not yet init'd, manually free */
+ release_nbp(&p->kobj); /* kobject not yet init'd, manually free */
goto err1;
}
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index d0434dc8c03b..1d3436279d2f 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1715,7 +1715,6 @@ void br_multicast_del_port(struct net_bridge_port *port)
#if IS_ENABLED(CONFIG_IPV6)
del_timer_sync(&port->ip6_mc_router_timer);
#endif
- free_percpu(port->mcast_stats);
}
static void br_multicast_enable(struct bridge_mcast_own_query *query)
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net v2] net: bridge: fix memleak in br_add_if()
2021-08-09 9:02 [PATCH net v2] net: bridge: fix memleak in br_add_if() Yang Yingliang
@ 2021-08-09 9:09 ` Nikolay Aleksandrov
2021-08-09 9:30 ` Yang Yingliang
0 siblings, 1 reply; 3+ messages in thread
From: Nikolay Aleksandrov @ 2021-08-09 9:09 UTC (permalink / raw)
To: Yang Yingliang, linux-kernel, netdev, bridge; +Cc: roopa, davem, kuba
On 09/08/2021 12:02, Yang Yingliang wrote:
> I got a memleak report:
>
> BUG: memory leak
> unreferenced object 0x607ee521a658 (size 240):
> comm "syz-executor.0", pid 955, jiffies 4294780569 (age 16.449s)
> hex dump (first 32 bytes, cpu 1):
> 00 00 00 00 00 00 00 00 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:
> [<00000000d830ea5a>] br_multicast_add_port+0x1c2/0x300 net/bridge/br_multicast.c:1693
> [<00000000274d9a71>] new_nbp net/bridge/br_if.c:435 [inline]
> [<00000000274d9a71>] br_add_if+0x670/0x1740 net/bridge/br_if.c:611
> [<0000000012ce888e>] do_set_master net/core/rtnetlink.c:2513 [inline]
> [<0000000012ce888e>] do_set_master+0x1aa/0x210 net/core/rtnetlink.c:2487
> [<0000000099d1cafc>] __rtnl_newlink+0x1095/0x13e0 net/core/rtnetlink.c:3457
> [<00000000a01facc0>] rtnl_newlink+0x64/0xa0 net/core/rtnetlink.c:3488
> [<00000000acc9186c>] rtnetlink_rcv_msg+0x369/0xa10 net/core/rtnetlink.c:5550
> [<00000000d4aabb9c>] netlink_rcv_skb+0x134/0x3d0 net/netlink/af_netlink.c:2504
> [<00000000bc2e12a3>] netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline]
> [<00000000bc2e12a3>] netlink_unicast+0x4a0/0x6a0 net/netlink/af_netlink.c:1340
> [<00000000e4dc2d0e>] netlink_sendmsg+0x789/0xc70 net/netlink/af_netlink.c:1929
> [<000000000d22c8b3>] sock_sendmsg_nosec net/socket.c:654 [inline]
> [<000000000d22c8b3>] sock_sendmsg+0x139/0x170 net/socket.c:674
> [<00000000e281417a>] ____sys_sendmsg+0x658/0x7d0 net/socket.c:2350
> [<00000000237aa2ab>] ___sys_sendmsg+0xf8/0x170 net/socket.c:2404
> [<000000004f2dc381>] __sys_sendmsg+0xd3/0x190 net/socket.c:2433
> [<0000000005feca6c>] do_syscall_64+0x37/0x90 arch/x86/entry/common.c:47
> [<000000007304477d>] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> On error path of br_add_if(), p->mcast_stats allocated in
> new_nbp() need be freed, or it will be leaked.
>
> Fixes: 1080ab95e3c7 ("net: bridge: add support for IGMP/MLD stats and export them via netlink")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
> v2:
> move free_percpu(p->mcast_stats) in release_nbp() and
> fix the compile error when CONFIG_BRIDGE_IGMP_SNOOPING
> is disabled.
> ---
> net/bridge/br_if.c | 5 ++++-
> net/bridge/br_multicast.c | 1 -
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
You can add a helper to free mcast stats and use it in the error path.
I'd like to keep the port dismantling code as it's currently done, moving
multicast stats freeing away from br_multicast_del_port is wrong.
In fact I think you can directly use br_multicast_del_port() as it's a noop w.r.t the
other actions (stopping timers that were never started and walking empty lists).
You'll have to test it, of course.
For this patch:
Nacked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
Thanks,
Nik
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net v2] net: bridge: fix memleak in br_add_if()
2021-08-09 9:09 ` Nikolay Aleksandrov
@ 2021-08-09 9:30 ` Yang Yingliang
0 siblings, 0 replies; 3+ messages in thread
From: Yang Yingliang @ 2021-08-09 9:30 UTC (permalink / raw)
To: Nikolay Aleksandrov, linux-kernel, netdev, bridge; +Cc: roopa, davem, kuba
On 2021/8/9 17:09, Nikolay Aleksandrov wrote:
> On 09/08/2021 12:02, Yang Yingliang wrote:
>> I got a memleak report:
>>
>> BUG: memory leak
>> unreferenced object 0x607ee521a658 (size 240):
>> comm "syz-executor.0", pid 955, jiffies 4294780569 (age 16.449s)
>> hex dump (first 32 bytes, cpu 1):
>> 00 00 00 00 00 00 00 00 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:
>> [<00000000d830ea5a>] br_multicast_add_port+0x1c2/0x300 net/bridge/br_multicast.c:1693
>> [<00000000274d9a71>] new_nbp net/bridge/br_if.c:435 [inline]
>> [<00000000274d9a71>] br_add_if+0x670/0x1740 net/bridge/br_if.c:611
>> [<0000000012ce888e>] do_set_master net/core/rtnetlink.c:2513 [inline]
>> [<0000000012ce888e>] do_set_master+0x1aa/0x210 net/core/rtnetlink.c:2487
>> [<0000000099d1cafc>] __rtnl_newlink+0x1095/0x13e0 net/core/rtnetlink.c:3457
>> [<00000000a01facc0>] rtnl_newlink+0x64/0xa0 net/core/rtnetlink.c:3488
>> [<00000000acc9186c>] rtnetlink_rcv_msg+0x369/0xa10 net/core/rtnetlink.c:5550
>> [<00000000d4aabb9c>] netlink_rcv_skb+0x134/0x3d0 net/netlink/af_netlink.c:2504
>> [<00000000bc2e12a3>] netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline]
>> [<00000000bc2e12a3>] netlink_unicast+0x4a0/0x6a0 net/netlink/af_netlink.c:1340
>> [<00000000e4dc2d0e>] netlink_sendmsg+0x789/0xc70 net/netlink/af_netlink.c:1929
>> [<000000000d22c8b3>] sock_sendmsg_nosec net/socket.c:654 [inline]
>> [<000000000d22c8b3>] sock_sendmsg+0x139/0x170 net/socket.c:674
>> [<00000000e281417a>] ____sys_sendmsg+0x658/0x7d0 net/socket.c:2350
>> [<00000000237aa2ab>] ___sys_sendmsg+0xf8/0x170 net/socket.c:2404
>> [<000000004f2dc381>] __sys_sendmsg+0xd3/0x190 net/socket.c:2433
>> [<0000000005feca6c>] do_syscall_64+0x37/0x90 arch/x86/entry/common.c:47
>> [<000000007304477d>] entry_SYSCALL_64_after_hwframe+0x44/0xae
>>
>> On error path of br_add_if(), p->mcast_stats allocated in
>> new_nbp() need be freed, or it will be leaked.
>>
>> Fixes: 1080ab95e3c7 ("net: bridge: add support for IGMP/MLD stats and export them via netlink")
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>> v2:
>> move free_percpu(p->mcast_stats) in release_nbp() and
>> fix the compile error when CONFIG_BRIDGE_IGMP_SNOOPING
>> is disabled.
>> ---
>> net/bridge/br_if.c | 5 ++++-
>> net/bridge/br_multicast.c | 1 -
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
> You can add a helper to free mcast stats and use it in the error path.
> I'd like to keep the port dismantling code as it's currently done, moving
> multicast stats freeing away from br_multicast_del_port is wrong.
>
> In fact I think you can directly use br_multicast_del_port() as it's a noop w.r.t the
> other actions (stopping timers that were never started and walking empty lists).
> You'll have to test it, of course.
OK, I will send a v3 after my testing.
Thanks,
Yang
>
> For this patch:
> Nacked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
>
> Thanks,
> Nik
>
> .
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-09 9:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-09 9:02 [PATCH net v2] net: bridge: fix memleak in br_add_if() Yang Yingliang
2021-08-09 9:09 ` Nikolay Aleksandrov
2021-08-09 9:30 ` Yang Yingliang
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).