* [PATCH net 0/2] net: ipmr, ip6mr: fix static leaks on netns destruction
@ 2015-11-20 12:54 Nikolay Aleksandrov
2015-11-20 12:54 ` [PATCH net 1/2] net: ipmr: fix static mfc/dev leaks on table destruction Nikolay Aleksandrov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-20 12:54 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji, davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Hi,
While testing various ipmr scenarios I found that static mfc entries and
static devices get leaked on netns/table destruction because
mroute_clean_tables doesn't delete them. It is fine to leave the static
entries when cleaning up the mrtsock, but when destroying the table they
need to be removed.
Cheers,
Nik
Nikolay Aleksandrov (2):
net: ipmr: fix static mfc/dev leaks on table destruction
net: ip6mr: fix static mfc/dev leaks on table destruction
net/ipv4/ipmr.c | 15 ++++++++-------
net/ipv6/ip6mr.c | 15 ++++++++-------
2 files changed, 16 insertions(+), 14 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 1/2] net: ipmr: fix static mfc/dev leaks on table destruction
2015-11-20 12:54 [PATCH net 0/2] net: ipmr, ip6mr: fix static leaks on netns destruction Nikolay Aleksandrov
@ 2015-11-20 12:54 ` Nikolay Aleksandrov
2015-11-20 18:52 ` Cong Wang
2015-11-20 12:54 ` [PATCH net 2/2] net: ip6mr: " Nikolay Aleksandrov
2015-11-23 17:14 ` [PATCH net 0/2] net: ipmr, ip6mr: fix static leaks on netns destruction David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-20 12:54 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji, davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
When destroying an mrt table the static mfc entries and the static
devices are kept, which leads to devices that can never be destroyed
(because of refcnt taken) and leaked memory, for example:
unreferenced object 0xffff880034c144c0 (size 192):
comm "mfc-broken", pid 4777, jiffies 4320349055 (age 46001.964s)
hex dump (first 32 bytes):
98 53 f0 34 00 88 ff ff 98 53 f0 34 00 88 ff ff .S.4.....S.4....
ef 0a 0a 14 01 02 03 04 00 00 00 00 01 00 00 00 ................
backtrace:
[<ffffffff815c1b9e>] kmemleak_alloc+0x4e/0xb0
[<ffffffff811ea6e0>] kmem_cache_alloc+0x190/0x300
[<ffffffff815931cb>] ip_mroute_setsockopt+0x5cb/0x910
[<ffffffff8153d575>] do_ip_setsockopt.isra.11+0x105/0xff0
[<ffffffff8153e490>] ip_setsockopt+0x30/0xa0
[<ffffffff81564e13>] raw_setsockopt+0x33/0x90
[<ffffffff814d1e14>] sock_common_setsockopt+0x14/0x20
[<ffffffff814d0b51>] SyS_setsockopt+0x71/0xc0
[<ffffffff815cdbf6>] entry_SYSCALL_64_fastpath+0x16/0x7a
[<ffffffffffffffff>] 0xffffffffffffffff
Make sure that everything is cleaned on netns destruction.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
This doesn't fix a specific commit as the behaviour seems to have been like
that since beginning of use of mroute_clean_tables to cleanup on netns
exit, but the fix can be sent back up to
acbb219d5f53 ("net: ipv4: ipmr_expire_timer causes crash when removing net
namespace")
which started cleaning up on netns destruction.
net/ipv4/ipmr.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 92dd4b74d513..292123bc30fa 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -134,7 +134,7 @@ static int __ipmr_fill_mroute(struct mr_table *mrt, struct sk_buff *skb,
struct mfc_cache *c, struct rtmsg *rtm);
static void mroute_netlink_event(struct mr_table *mrt, struct mfc_cache *mfc,
int cmd);
-static void mroute_clean_tables(struct mr_table *mrt);
+static void mroute_clean_tables(struct mr_table *mrt, bool all);
static void ipmr_expire_process(unsigned long arg);
#ifdef CONFIG_IP_MROUTE_MULTIPLE_TABLES
@@ -350,7 +350,7 @@ static struct mr_table *ipmr_new_table(struct net *net, u32 id)
static void ipmr_free_table(struct mr_table *mrt)
{
del_timer_sync(&mrt->ipmr_expire_timer);
- mroute_clean_tables(mrt);
+ mroute_clean_tables(mrt, true);
kfree(mrt);
}
@@ -1208,7 +1208,7 @@ static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
* Close the multicast socket, and clear the vif tables etc
*/
-static void mroute_clean_tables(struct mr_table *mrt)
+static void mroute_clean_tables(struct mr_table *mrt, bool all)
{
int i;
LIST_HEAD(list);
@@ -1217,8 +1217,9 @@ static void mroute_clean_tables(struct mr_table *mrt)
/* Shut down all active vif entries */
for (i = 0; i < mrt->maxvif; i++) {
- if (!(mrt->vif_table[i].flags & VIFF_STATIC))
- vif_delete(mrt, i, 0, &list);
+ if (!all && (mrt->vif_table[i].flags & VIFF_STATIC))
+ continue;
+ vif_delete(mrt, i, 0, &list);
}
unregister_netdevice_many(&list);
@@ -1226,7 +1227,7 @@ static void mroute_clean_tables(struct mr_table *mrt)
for (i = 0; i < MFC_LINES; i++) {
list_for_each_entry_safe(c, next, &mrt->mfc_cache_array[i], list) {
- if (c->mfc_flags & MFC_STATIC)
+ if (!all && (c->mfc_flags & MFC_STATIC))
continue;
list_del_rcu(&c->list);
mroute_netlink_event(mrt, c, RTM_DELROUTE);
@@ -1261,7 +1262,7 @@ static void mrtsock_destruct(struct sock *sk)
NETCONFA_IFINDEX_ALL,
net->ipv4.devconf_all);
RCU_INIT_POINTER(mrt->mroute_sk, NULL);
- mroute_clean_tables(mrt);
+ mroute_clean_tables(mrt, false);
}
}
rtnl_unlock();
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/2] net: ip6mr: fix static mfc/dev leaks on table destruction
2015-11-20 12:54 [PATCH net 0/2] net: ipmr, ip6mr: fix static leaks on netns destruction Nikolay Aleksandrov
2015-11-20 12:54 ` [PATCH net 1/2] net: ipmr: fix static mfc/dev leaks on table destruction Nikolay Aleksandrov
@ 2015-11-20 12:54 ` Nikolay Aleksandrov
2015-11-20 18:52 ` Cong Wang
2015-11-23 17:14 ` [PATCH net 0/2] net: ipmr, ip6mr: fix static leaks on netns destruction David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-20 12:54 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji, davem, Nikolay Aleksandrov, Benjamin Thery
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Similar to ipv4, when destroying an mrt table the static mfc entries and
the static devices are kept, which leads to devices that can never be
destroyed (because of refcnt taken) and leaked memory. Make sure that
everything is cleaned up on netns destruction.
Fixes: 8229efdaef1e ("netns: ip6mr: enable namespace support in ipv6 multicast forwarding code")
CC: Benjamin Thery <benjamin.thery@bull.net>
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/ipv6/ip6mr.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index ad19136086dd..7a4a1b81dbb6 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -118,7 +118,7 @@ static void mr6_netlink_event(struct mr6_table *mrt, struct mfc6_cache *mfc,
int cmd);
static int ip6mr_rtm_dumproute(struct sk_buff *skb,
struct netlink_callback *cb);
-static void mroute_clean_tables(struct mr6_table *mrt);
+static void mroute_clean_tables(struct mr6_table *mrt, bool all);
static void ipmr_expire_process(unsigned long arg);
#ifdef CONFIG_IPV6_MROUTE_MULTIPLE_TABLES
@@ -334,7 +334,7 @@ static struct mr6_table *ip6mr_new_table(struct net *net, u32 id)
static void ip6mr_free_table(struct mr6_table *mrt)
{
del_timer_sync(&mrt->ipmr_expire_timer);
- mroute_clean_tables(mrt);
+ mroute_clean_tables(mrt, true);
kfree(mrt);
}
@@ -1542,7 +1542,7 @@ static int ip6mr_mfc_add(struct net *net, struct mr6_table *mrt,
* Close the multicast socket, and clear the vif tables etc
*/
-static void mroute_clean_tables(struct mr6_table *mrt)
+static void mroute_clean_tables(struct mr6_table *mrt, bool all)
{
int i;
LIST_HEAD(list);
@@ -1552,8 +1552,9 @@ static void mroute_clean_tables(struct mr6_table *mrt)
* Shut down all active vif entries
*/
for (i = 0; i < mrt->maxvif; i++) {
- if (!(mrt->vif6_table[i].flags & VIFF_STATIC))
- mif6_delete(mrt, i, &list);
+ if (!all && (mrt->vif6_table[i].flags & VIFF_STATIC))
+ continue;
+ mif6_delete(mrt, i, &list);
}
unregister_netdevice_many(&list);
@@ -1562,7 +1563,7 @@ static void mroute_clean_tables(struct mr6_table *mrt)
*/
for (i = 0; i < MFC6_LINES; i++) {
list_for_each_entry_safe(c, next, &mrt->mfc6_cache_array[i], list) {
- if (c->mfc_flags & MFC_STATIC)
+ if (!all && (c->mfc_flags & MFC_STATIC))
continue;
write_lock_bh(&mrt_lock);
list_del(&c->list);
@@ -1625,7 +1626,7 @@ int ip6mr_sk_done(struct sock *sk)
net->ipv6.devconf_all);
write_unlock_bh(&mrt_lock);
- mroute_clean_tables(mrt);
+ mroute_clean_tables(mrt, false);
err = 0;
break;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] net: ipmr: fix static mfc/dev leaks on table destruction
2015-11-20 12:54 ` [PATCH net 1/2] net: ipmr: fix static mfc/dev leaks on table destruction Nikolay Aleksandrov
@ 2015-11-20 18:52 ` Cong Wang
0 siblings, 0 replies; 6+ messages in thread
From: Cong Wang @ 2015-11-20 18:52 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, Hideaki YOSHIFUJI, David Miller, Nikolay Aleksandrov
On Fri, Nov 20, 2015 at 4:54 AM, Nikolay Aleksandrov
<razor@blackwall.org> wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>
> When destroying an mrt table the static mfc entries and the static
> devices are kept, which leads to devices that can never be destroyed
> (because of refcnt taken) and leaked memory, for example:
> unreferenced object 0xffff880034c144c0 (size 192):
> comm "mfc-broken", pid 4777, jiffies 4320349055 (age 46001.964s)
> hex dump (first 32 bytes):
> 98 53 f0 34 00 88 ff ff 98 53 f0 34 00 88 ff ff .S.4.....S.4....
> ef 0a 0a 14 01 02 03 04 00 00 00 00 01 00 00 00 ................
> backtrace:
> [<ffffffff815c1b9e>] kmemleak_alloc+0x4e/0xb0
> [<ffffffff811ea6e0>] kmem_cache_alloc+0x190/0x300
> [<ffffffff815931cb>] ip_mroute_setsockopt+0x5cb/0x910
> [<ffffffff8153d575>] do_ip_setsockopt.isra.11+0x105/0xff0
> [<ffffffff8153e490>] ip_setsockopt+0x30/0xa0
> [<ffffffff81564e13>] raw_setsockopt+0x33/0x90
> [<ffffffff814d1e14>] sock_common_setsockopt+0x14/0x20
> [<ffffffff814d0b51>] SyS_setsockopt+0x71/0xc0
> [<ffffffff815cdbf6>] entry_SYSCALL_64_fastpath+0x16/0x7a
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> Make sure that everything is cleaned on netns destruction.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Looks good to me,
Reviewed-by: Cong Wang <cwang@twopensource.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net: ip6mr: fix static mfc/dev leaks on table destruction
2015-11-20 12:54 ` [PATCH net 2/2] net: ip6mr: " Nikolay Aleksandrov
@ 2015-11-20 18:52 ` Cong Wang
0 siblings, 0 replies; 6+ messages in thread
From: Cong Wang @ 2015-11-20 18:52 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, Hideaki YOSHIFUJI, David Miller, Nikolay Aleksandrov,
Benjamin Thery
On Fri, Nov 20, 2015 at 4:54 AM, Nikolay Aleksandrov
<razor@blackwall.org> wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>
> Similar to ipv4, when destroying an mrt table the static mfc entries and
> the static devices are kept, which leads to devices that can never be
> destroyed (because of refcnt taken) and leaked memory. Make sure that
> everything is cleaned up on netns destruction.
>
> Fixes: 8229efdaef1e ("netns: ip6mr: enable namespace support in ipv6 multicast forwarding code")
> CC: Benjamin Thery <benjamin.thery@bull.net>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Reviewed-by: Cong Wang <cwang@twopensource.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/2] net: ipmr, ip6mr: fix static leaks on netns destruction
2015-11-20 12:54 [PATCH net 0/2] net: ipmr, ip6mr: fix static leaks on netns destruction Nikolay Aleksandrov
2015-11-20 12:54 ` [PATCH net 1/2] net: ipmr: fix static mfc/dev leaks on table destruction Nikolay Aleksandrov
2015-11-20 12:54 ` [PATCH net 2/2] net: ip6mr: " Nikolay Aleksandrov
@ 2015-11-23 17:14 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2015-11-23 17:14 UTC (permalink / raw)
To: razor; +Cc: netdev, yoshfuji, nikolay
From: Nikolay Aleksandrov <razor@blackwall.org>
Date: Fri, 20 Nov 2015 13:54:18 +0100
> While testing various ipmr scenarios I found that static mfc entries and
> static devices get leaked on netns/table destruction because
> mroute_clean_tables doesn't delete them. It is fine to leave the static
> entries when cleaning up the mrtsock, but when destroying the table they
> need to be removed.
Series applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-23 17:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-20 12:54 [PATCH net 0/2] net: ipmr, ip6mr: fix static leaks on netns destruction Nikolay Aleksandrov
2015-11-20 12:54 ` [PATCH net 1/2] net: ipmr: fix static mfc/dev leaks on table destruction Nikolay Aleksandrov
2015-11-20 18:52 ` Cong Wang
2015-11-20 12:54 ` [PATCH net 2/2] net: ip6mr: " Nikolay Aleksandrov
2015-11-20 18:52 ` Cong Wang
2015-11-23 17:14 ` [PATCH net 0/2] net: ipmr, ip6mr: fix static leaks on netns destruction David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.