Skip to content

Commit

Permalink
net: fix out-of-bounds access in ops_init
Browse files Browse the repository at this point in the history
commit a26ff37e624d12e28077e5b24d2b264f62764ad6 upstream.

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: 073862b ("netns: fix net_alloc_generic()")
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20240502132006.3430840-1-cascardo@igalia.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 3cdc34d76c4f777579e28ad373979d36c030cfd3)
[Vegard: drop *ops->id assignment due to missing commit 6e77cc4
 ("Convert net_namespace to new IDA API").]
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
  • Loading branch information
Thadeu Lima de Souza Cascardo authored and vegard committed Jun 3, 2024
1 parent 9804a24 commit cc156d9
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions net/core/net_namespace.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,15 @@ static unsigned int max_gen_ptrs = INITIAL_NET_GEN_PTRS;

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;
}
Expand Down Expand Up @@ -954,7 +957,11 @@ static int register_pernet_operations(struct list_head *list,
}
return 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) {
Expand Down

0 comments on commit cc156d9

Please sign in to comment.