Netdev Archive mirror
 help / color / mirror / Atom feed
* [PATCH net v3] net: fix out-of-bounds access in ops_init
@ 2024-05-02 13:20 Thadeu Lima de Souza Cascardo
  2024-05-06 13:40 ` patchwork-bot+netdevbpf
  0 siblings, 1 reply; 2+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2024-05-02 13:20 UTC (permalink / raw
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Kuniyuki Iwashima, kernel-dev, stable,
	Thadeu Lima de Souza Cascardo

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 reading max_gen_ptrs only once in net_alloc_generic. If
max_gen_ptrs is later incremented, it will be caught in net_assign_generic.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
Fixes: 073862ba5d24 ("netns: fix net_alloc_generic()")
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Cc: stable@vger.kernel.org
---
v3:
  - Use reverse xmas order in local variable declaration.
  - Use netdev multi-line comment style.
  - Target to net tree.
  - Cc stable.
v2:
  - Instead of delaying struct net_generic allocation to setup_net,
    read max_gen_ptrs only once.
v1: https://lore.kernel.org/netdev/20240430084253.3272177-1-cascardo@igalia.com/
---
 net/core/net_namespace.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index f0540c557515..9d690d32da33 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -69,12 +69,15 @@ DEFINE_COOKIE(net_cookie);
 
 static struct net_generic *net_alloc_generic(void)
 {
+	unsigned int gen_ptrs = READ_ONCE(max_gen_ptrs);
+	unsigned int generic_size;
 	struct net_generic *ng;
-	unsigned int generic_size = offsetof(struct net_generic, ptr[max_gen_ptrs]);
+
+	generic_size = offsetof(struct net_generic, ptr[gen_ptrs]);
 
 	ng = kzalloc(generic_size, GFP_KERNEL);
 	if (ng)
-		ng->s.len = max_gen_ptrs;
+		ng->s.len = gen_ptrs;
 
 	return ng;
 }
@@ -1307,7 +1310,11 @@ static int register_pernet_operations(struct list_head *list,
 		if (error < 0)
 			return error;
 		*ops->id = error;
-		max_gen_ptrs = max(max_gen_ptrs, *ops->id + 1);
+		/* This does not require READ_ONCE as writers already hold
+		 * pernet_ops_rwsem. But WRITE_ONCE is needed to protect
+		 * net_alloc_generic.
+		 */
+		WRITE_ONCE(max_gen_ptrs, max(max_gen_ptrs, *ops->id + 1));
 	}
 	error = __register_pernet_operations(list, ops);
 	if (error) {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net v3] net: fix out-of-bounds access in ops_init
  2024-05-02 13:20 [PATCH net v3] net: fix out-of-bounds access in ops_init Thadeu Lima de Souza Cascardo
@ 2024-05-06 13:40 ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 2+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-05-06 13:40 UTC (permalink / raw
  To: Thadeu Lima de Souza Cascardo
  Cc: netdev, davem, edumazet, kuba, pabeni, kuniyu, kernel-dev, stable

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Thu,  2 May 2024 10:20:06 -0300 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [net,v3] net: fix out-of-bounds access in ops_init
    https://git.kernel.org/netdev/net/c/a26ff37e624d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-05-06 13:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-02 13:20 [PATCH net v3] net: fix out-of-bounds access in ops_init Thadeu Lima de Souza Cascardo
2024-05-06 13:40 ` patchwork-bot+netdevbpf

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 read-only IMAP folder(s) and NNTP newsgroup(s).