netdev - [PATCH] net: fix out-of-bounds access in ops

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240430084253.3272177-1-cascardo@igalia.com>
Date: Tue, 30 Apr 2024 05:42:53 -0300
From: Thadeu Lima de Souza Cascardo <cascardo@...lia.com>
To: netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	kernel-dev@...lia.com,
	Thadeu Lima de Souza Cascardo <cascardo@...lia.com>
Subject: [PATCH] net: fix out-of-bounds access in ops_init

net_alloc_generic is called by net_alloc, which is called without any
locking. It reads max_gen_ptrs, which is changed under pernet_ops_rwsem. It
is read twice, first to allocate an array, then to set s.len, which is
later used to limit the bounds of the array access.

It is possible that the array is allocated and another thread is
registering a new pernet ops, increments max_gen_ptrs, which is then used
to set s.len with a larger than allocated length for the variable array.

Fix it by delaying the allocation to setup_net, which is always called
under pernet_ops_rwsem, and is called right after net_alloc.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@...lia.com>
---
 net/core/net_namespace.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index f0540c557515..879e49b10cca 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -87,7 +87,7 @@ static int net_assign_generic(struct net *net, unsigned int id, void *data)
 
 	old_ng = rcu_dereference_protected(net->gen,
 					   lockdep_is_held(&pernet_ops_rwsem));
-	if (old_ng->s.len > id) {
+	if (old_ng && old_ng->s.len > id) {
 		old_ng->ptr[id] = data;
 		return 0;
 	}
@@ -107,8 +107,9 @@ static int net_assign_generic(struct net *net, unsigned int id, void *data)
 	 * the old copy for kfree after a grace period.
 	 */
 
-	memcpy(&ng->ptr[MIN_PERNET_OPS_ID], &old_ng->ptr[MIN_PERNET_OPS_ID],
-	       (old_ng->s.len - MIN_PERNET_OPS_ID) * sizeof(void *));
+	if (old_ng)
+		memcpy(&ng->ptr[MIN_PERNET_OPS_ID], &old_ng->ptr[MIN_PERNET_OPS_ID],
+		       (old_ng->s.len - MIN_PERNET_OPS_ID) * sizeof(void *));
 	ng->ptr[id] = data;
 
 	rcu_assign_pointer(net->gen, ng);
@@ -422,15 +423,10 @@ static struct workqueue_struct *netns_wq;
 static struct net *net_alloc(void)
 {
 	struct net *net = NULL;
-	struct net_generic *ng;
-
-	ng = net_alloc_generic();
-	if (!ng)
-		goto out;
 
 	net = kmem_cache_zalloc(net_cachep, GFP_KERNEL);
 	if (!net)
-		goto out_free;
+		goto out;
 
 #ifdef CONFIG_KEYS
 	net->key_domain = kzalloc(sizeof(struct key_tag), GFP_KERNEL);
@@ -439,7 +435,7 @@ static struct net *net_alloc(void)
 	refcount_set(&net->key_domain->usage, 1);
 #endif
 
-	rcu_assign_pointer(net->gen, ng);
+	rcu_assign_pointer(net->gen, NULL);
 out:
 	return net;
 
@@ -448,8 +444,6 @@ static struct net *net_alloc(void)
 	kmem_cache_free(net_cachep, net);
 	net = NULL;
 #endif
-out_free:
-	kfree(ng);
 	goto out;
 }
 
-- 
2.34.1