[PATCH net] net: fix a resource leak in copy_net_ns() error handling path

Tetsuo Handa <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Currently, preinit_net() does two things:

  (1) call ns_common_init() which might fail
  (2) initialize resources which does not fail

However, preinit_net() is returning early when (1) fails, and copy_net_ns()
is jumping to the dec_ucounts: label. As a result, resources allocated by
net_alloc() are leaking. We need to call key_remove_domain() and
net_passive_dec() in order to release resources allocated by net_alloc().

We cannot simply jump to the put_userns: label when preinit_net() failed,
for (2) is not yet done. But we can reorder (1) and (2), for there is no
dependency between (1) and (2). Therefore, this patch decouples (1) from
preinit_net() and changes preinit_net() back to a void function, and calls
ns_common_init() after preinit_net() succeeded. Then, we can jump to
immediately after ns_common_free() of the put_userns: label.

Reported-by: sashiko (no mail address)
Closes: https://sashiko.dev/#/patchset/af7dabf3-d0d7-46dc-a878-e1715b3c9ac6%40I-love.SAKURA.ne.jp
Fixes: 08027f6b790b ("net: use ns_common_init()")
Signed-off-by: Tetsuo Handa <[email protected]>
---
 net/core/net_namespace.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 9166f467293e..da5f881fbd3b 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -400,14 +400,8 @@ static __net_init void preinit_net_sysctl(struct net *net)
 }
 
 /* init code that must occur even if setup_net() is not called. */
-static __net_init int preinit_net(struct net *net, struct user_namespace *user_ns)
+static __net_init void preinit_net(struct net *net, struct user_namespace *user_ns)
 {
-	int ret;
-
-	ret = ns_common_init(net);
-	if (ret)
-		return ret;
-
 	refcount_set(&net->passive, 1);
 	ref_tracker_dir_init(&net->refcnt_tracker, 128, "net_refcnt");
 	ref_tracker_dir_init(&net->notrefcnt_tracker, 128, "net_notrefcnt");
@@ -431,7 +425,6 @@ static __net_init int preinit_net(struct net *net, struct user_namespace *user_n
 	INIT_LIST_HEAD(&net->ptype_all);
 	INIT_LIST_HEAD(&net->ptype_specific);
 	preinit_net_sysctl(net);
-	return 0;
 }
 
 /*
@@ -574,12 +567,14 @@ struct net *copy_net_ns(u64 flags,
 		goto dec_ucounts;
 	}
 
-	rv = preinit_net(net, user_ns);
-	if (rv < 0)
-		goto dec_ucounts;
+	preinit_net(net, user_ns);
 	net->ucounts = ucounts;
 	get_user_ns(user_ns);
 
+	rv = ns_common_init(net);
+	if (rv)
+		goto put_userns_no_common;
+
 	rv = down_read_killable(&pernet_ops_rwsem);
 	if (rv < 0)
 		goto put_userns;
@@ -591,6 +586,7 @@ struct net *copy_net_ns(u64 flags,
 	if (rv < 0) {
 put_userns:
 		ns_common_free(net);
+put_userns_no_common:
 #ifdef CONFIG_KEYS
 		key_remove_domain(net->key_domain);
 #endif
@@ -1293,7 +1289,8 @@ void __init net_ns_init(void)
 	 * This currently cannot fail as the initial network namespace
 	 * has a static inode number.
 	 */
-	if (preinit_net(&init_net, &init_user_ns))
+	preinit_net(&init_net, &init_user_ns);
+	if (ns_common_init(&init_net))
 		panic("Could not preinitialize the initial network namespace");
 
 	down_write(&pernet_ops_rwsem);
-- 
2.52.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.