[PATCH net] net: pin protocol module before socket allocation

[Chengfeng Ye <nicoyip.dev@gmail.com>]

sk_prot_alloc() reads prot->slab and starts allocating the socket
before it gets a reference to prot->owner. A protocol module can begin
unloading after its protocol was selected but before the reference is
taken, allowing this interleaving:

  CPU 0                                CPU 1
  slab = prot->slab
                                       proto_unregister(prot)
                                         kmem_cache_destroy(prot->slab)
  kmem_cache_alloc(slab, ...)

kmem_cache_alloc() then dereferences a freed struct kmem_cache and can
crash or corrupt memory. The kernel reported:

  Oops: general protection fault, probably for non-canonical address
  KASAN: maybe wild-memory-access in range
  RIP: kmem_cache_alloc_noprof+0x63/0x370
  Call Trace:
   sk_prot_alloc+0x74/0x2c0
   sk_alloc+0x2b/0x6c0
   inet_create+0x2cd/0xd40
   __sock_create+0x1c3/0x430
   __sys_socket+0x116/0x1d0

Take the module reference before reading prot->slab so module removal
cannot destroy the cache during allocation. Drop that reference after
freeing the allocation on either failure path; on success sk_prot_free()
continues to release it as before.

Fixes: a79af59efd20 ("[NET]: Fix module reference counts for loadable protocol modules")
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/core/sock.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 1ad41904db25..59f4b15fd594 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2240,33 +2240,34 @@ static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority,
 	struct sock *sk;
 	struct kmem_cache *slab;
 
+	if (!try_module_get(prot->owner))
+		return NULL;
+
 	slab = prot->slab;
 	if (slab != NULL) {
 		sk = kmem_cache_alloc(slab, priority & ~__GFP_ZERO);
 		if (!sk)
-			return sk;
+			goto out_module_put;
 		if (want_init_on_alloc(priority))
 			sk_prot_clear_nulls(sk, prot->obj_size);
 	} else
 		sk = kmalloc(prot->obj_size, priority);
 
-	if (sk != NULL) {
-		if (security_sk_alloc(sk, family, priority))
-			goto out_free;
-
-		if (!try_module_get(prot->owner))
-			goto out_free_sec;
-	}
+	if (!sk)
+		goto out_module_put;
+
+	if (security_sk_alloc(sk, family, priority))
+		goto out_free;
 
 	return sk;
 
-out_free_sec:
-	security_sk_free(sk);
 out_free:
 	if (slab != NULL)
 		kmem_cache_free(slab, sk);
 	else
 		kfree(sk);
+out_module_put:
+	module_put(prot->owner);
 	return NULL;
 }
 
-- 
2.43.0