[Devel] [PATCH RH7] net: net_assign_generic(NULL) lead to memory leaks

Vasily Averin vvs at virtuozzo.com
Mon Apr 5 12:13:45 MSK 2021


net_assign_generic(NULL) called during net_init or net_exit hooks
cleans up net_generic() pointer allocated by ops_init()
and does not allow to free it during ops_free()

Let's drop calls from net_exit hooks
and use net_generic_free() call instead in net_init_hooks.
https://jira.sw.ru/browse/PSBM-92950
Signed-off-by: Vasily Averin <vvs at virtuozzo.com>
---
  drivers/net/ppp/ppp_generic.c |  7 ++++---
  drivers/net/ppp/pppoe.c       |  7 ++++---
  include/net/netns/generic.h   | 13 ++++++++++++-
  net/ipv4/ip_gre.c             | 12 ++++++++----
  net/ipv4/ip_vti.c             |  6 ++++--
  net/ipv4/ipip.c               |  8 ++++----
  net/ipv6/ip6_gre.c            |  6 ++++--
  net/ipv6/ip6_tunnel.c         |  6 ++++--
  net/ipv6/sit.c                |  7 ++++---
  9 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index a9480c10081a..5c7e0d5f0aa3 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -884,9 +884,10 @@ static __net_init int ppp_init_net(struct net *net)
  {
  	struct ppp_net *pn = net_generic(net, ppp_net_id);
  
-	if (!(net->owner_ve->features & VE_FEATURE_PPP))
-		return net_assign_generic(net, ppp_net_id, NULL);
-
+	if (!(net->owner_ve->features & VE_FEATURE_PPP)) {
+		net_generic_free(net, ppp_net_id);
+		return 0;
+	}
  	idr_init(&pn->units_idr);
  	mutex_init(&pn->all_ppp_mutex);
  
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 15d3f44775f5..3733453a993a 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -1168,9 +1168,10 @@ static __net_init int pppoe_init_net(struct net *net)
  	struct pppoe_net *pn = pppoe_pernet(net);
  	struct proc_dir_entry *pde;
  
-	if (!(net->owner_ve->features & VE_FEATURE_PPP))
-		return net_assign_generic(net, pppoe_net_id, NULL);
-
+	if (!(net->owner_ve->features & VE_FEATURE_PPP)) {
+		net_generic_free(net, pppoe_net_id);
+		return 0;
+	}
  	rwlock_init(&pn->hash_lock);
  
  	pde = proc_net_create("pppoe", S_IRUGO, net->proc_net, &pppoe_seq_fops);
diff --git a/include/net/netns/generic.h b/include/net/netns/generic.h
index a07ef65870cb..eb07bda77fa9 100644
--- a/include/net/netns/generic.h
+++ b/include/net/netns/generic.h
@@ -49,6 +49,17 @@ static inline void *net_generic(const struct net *net, int id)
  	return ptr;
  }
  
-extern int net_assign_generic(struct net *net, int id, void *data);
+static inline void net_generic_free(const struct net *net, unsigned int id)
+{
+	struct net_generic *ng;
+	void *ptr;
+
+	rcu_read_lock();
+	ng = rcu_dereference(net->gen);
+	ptr = ng->ptr[id];
+	ng->ptr[id] = NULL;
+	rcu_read_unlock();
  
+	kfree(ptr);
+}
  #endif
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index aecb1a8feb35..52234a549bea 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -788,8 +788,10 @@ static const struct gre_protocol ipgre_protocol = {
  static int __net_init ipgre_init_net(struct net *net)
  {
  #ifdef CONFIG_VE
-	if (!(net->owner_ve->features & VE_FEATURE_IPGRE))
-		return net_assign_generic(net, ipgre_net_id, NULL);
+	if (!(net->owner_ve->features & VE_FEATURE_IPGRE)) {
+		net_generic_free(net, ipgre_net_id);
+		return 0;
+	}
  #endif
  	return ip_tunnel_init_net(net, ipgre_net_id, &ipgre_link_ops, NULL);
  }
@@ -1222,8 +1224,10 @@ EXPORT_SYMBOL_GPL(gretap_fb_dev_create);
  static int __net_init ipgre_tap_init_net(struct net *net)
  {
  #ifdef CONFIG_VE
-	if (!(net->owner_ve->features & VE_FEATURE_IPGRE))
-		return net_assign_generic(net, gre_tap_net_id, NULL);
+	if (!(net->owner_ve->features & VE_FEATURE_IPGRE)) {
+		net_generic_free(net, gre_tap_net_id);
+		return 0;
+	}
  #endif
  	return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, "gretap0");
  }
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 3ab6f1aec18d..3608f4e454ed 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -436,8 +436,10 @@ static int __net_init vti_init_net(struct net *net)
  	int err;
  	struct ip_tunnel_net *itn;
  
-	if (!ve_is_super(net->owner_ve))
-		return net_assign_generic(net, vti_net_id, NULL);
+	if (!ve_is_super(net->owner_ve)) {
+		net_generic_free(net, vti_net_id);
+		return 0;
+	}
  
  	err = ip_tunnel_init_net(net, vti_net_id, &vti_link_ops, "ip_vti0");
  	if (err)
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index ea55274488bf..a4c1c205feb2 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -471,9 +471,10 @@ static struct xfrm_tunnel ipip_handler __read_mostly = {
  
  static int __net_init ipip_init_net(struct net *net)
  {
-	if (!(net->owner_ve->features & VE_FEATURE_IPIP))
-		return net_assign_generic(net, ipip_net_id, NULL);
-
+	if (!(net->owner_ve->features & VE_FEATURE_IPIP)) {
+		net_assign_generic(net, ipip_net_id);
+		return 0;
+	}
  	return ip_tunnel_init_net(net, ipip_net_id, &ipip_link_ops, "tunl0");
  }
  
@@ -485,7 +486,6 @@ static void __net_exit ipip_exit_net(struct net *net)
  		return;
  
  	ip_tunnel_delete_net(itn, &ipip_link_ops);
-	net_assign_generic(net, ipip_net_id, NULL);
  }
  
  static struct pernet_operations ipip_net_ops = {
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 818d76efc635..574369249d8e 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1234,8 +1234,10 @@ static int __net_init ip6gre_init_net(struct net *net)
  	int err;
  
  #ifdef CONFIG_VE
-	if (!(net->owner_ve->features & VE_FEATURE_IPGRE))
-		return net_assign_generic(net, ip6gre_net_id, NULL);
+	if (!(net->owner_ve->features & VE_FEATURE_IPGRE)) {
+		net_generic_free(net, ip6gre_net_id);
+		return 0;
+	}
  #endif
  
  	ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), "ip6gre0",
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 15617196bbac..94c892be8820 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -2045,8 +2045,10 @@ static int __net_init ip6_tnl_init_net(struct net *net)
  	int err;
  
  #ifdef CONFIG_VE
-	if (!(net->owner_ve->features & VE_FEATURE_IPIP))
-		return net_assign_generic(net, ip6_tnl_net_id, NULL);
+	if (!(net->owner_ve->features & VE_FEATURE_IPIP)) {
+		net_generic_free(net, ip6_tnl_net_id);
+		return 0;
+	}
  #endif
  
  	ip6n->tnls[0] = ip6n->tnls_wc;
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index b25418369061..832f1d87ff21 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1739,8 +1739,10 @@ static int __net_init sit_init_net(struct net *net)
  	struct ip_tunnel *t;
  	int err;
  
-	if (!(net->owner_ve->features & VE_FEATURE_SIT))
-		return net_assign_generic(net, sit_net_id, NULL);
+	if (!(net->owner_ve->features & VE_FEATURE_SIT)) {
+		net_generic_free(net, sit_net_id);
+		return 0;
+	}
  
  	sitn->tunnels[0] = sitn->tunnels_wc;
  	sitn->tunnels[1] = sitn->tunnels_l;
@@ -1790,7 +1792,6 @@ static void __net_exit sit_exit_net(struct net *net)
  	sit_destroy_tunnels(net, &list);
  	unregister_netdevice_many(&list);
  	rtnl_unlock();
-	net_assign_generic(net, sit_net_id, NULL);
  }
  
  static struct pernet_operations sit_net_ops = {
-- 
2.25.1



More information about the Devel mailing list