Message ID | 20240502132006.3430840-1-cascardo@igalia.com (mailing list archive) |
---|---|
State | Accepted |
Commit | a26ff37e624d12e28077e5b24d2b264f62764ad6 |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net,v3] net: fix out-of-bounds access in ops_init | expand |
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!
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) {