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

[Chengfeng Ye]

inet_create() and inet6_create() look up the protocol under
rcu_read_lock(), then drop RCU before using the resulting proto.
sk_alloc() uses GFP_KERNEL, so RCU cannot be held across it.

A loadable protocol can be unregistered in that window.
inet_unregister_protosw() waits with synchronize_net() only for
readers still in the RCU section. After rcu_read_unlock(), module
exit can run proto_unregister(), destroy prot->slab, and free the
module. inet_create() then uses a dangling proto pointer:

  CPU 0 inet_create                     CPU 1 l2tp_ip_exit
  rcu_read_lock()
  answer_prot = answer->prot
  rcu_read_unlock()                     inet_unregister_protosw()
                                          synchronize_net()
                                        proto_unregister()
                                          kmem_cache_destroy(slab)
  WARN_ON(!answer_prot->slab)
  sk_alloc() -> kmem_cache_alloc(stale)

This was reproduced with socket(AF_INET, SOCK_DGRAM, IPPROTO_L2TP)
racing delete_module("l2tp_ip"):

  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

__sock_create() already converts family lookup into a module
reference before dropping RCU, but that pins inet, not the protocol
module. Pin answer_prot->owner while still under RCU, then allocate.
sk_prot_alloc() still takes the socket-lifetime reference; drop the
temporary one on every path after sk_alloc(). inet6_create() has the
same hole.

Fixes: a79af59efd20 ("[NET]: Fix module reference counts for loadable protocol modules")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Chengfeng Ye <nicoyip.dev@xxxxxxxxx>
---
v2: Pin answer_prot->owner in inet_create()/inet6_create() under
    RCU instead of reordering try_module_get() in sk_prot_alloc().
    sk_alloc() can sleep, so the lookup must be converted into a
    module reference before rcu_read_unlock(). Suggested by
    Kuniyuki Iwashima.

 net/ipv4/af_inet.c  | 10 ++++++++--
 net/ipv6/af_inet6.c | 10 ++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 32d006c1a8ee..88a18ac0e6e4 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -325,6 +325,10 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
 	sock->ops = answer->ops;
 	answer_prot = answer->prot;
 	answer_flags = answer->flags;
+	if (!try_module_get(answer_prot->owner)) {
+		err = -EPROTONOSUPPORT;
+		goto out_rcu_unlock;
+	}
 	rcu_read_unlock();
 
 	WARN_ON(!answer_prot->slab);
@@ -332,7 +336,7 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
 	err = -ENOMEM;
 	sk = sk_alloc(net, PF_INET, GFP_KERNEL, answer_prot, kern);
 	if (!sk)
-		goto out;
+		goto out_module_put;
 
 	err = 0;
 	if (INET_PROTOSW_REUSE & answer_flags)
@@ -398,6 +402,8 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
 		if (err)
 			goto out_sk_release;
 	}
+out_module_put:
+	module_put(answer_prot->owner);
 out:
 	return err;
 out_rcu_unlock:
@@ -406,7 +412,7 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
 out_sk_release:
 	sk_common_release(sk);
 	sock->sk = NULL;
-	goto out;
+	goto out_module_put;
 }
 
 
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 282912a11999..82b390ce3e0c 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -170,6 +170,10 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
 	sock->ops = answer->ops;
 	answer_prot = answer->prot;
 	answer_flags = answer->flags;
+	if (!try_module_get(answer_prot->owner)) {
+		err = -EPROTONOSUPPORT;
+		goto out_rcu_unlock;
+	}
 	rcu_read_unlock();
 
 	WARN_ON(!answer_prot->slab);
@@ -177,7 +181,7 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
 	err = -ENOBUFS;
 	sk = sk_alloc(net, PF_INET6, GFP_KERNEL, answer_prot, kern);
 	if (!sk)
-		goto out;
+		goto out_module_put;
 
 	sock_init_data(sock, sk);
 
@@ -251,6 +255,8 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
 		if (err)
 			goto out_sk_release;
 	}
+out_module_put:
+	module_put(answer_prot->owner);
 out:
 	return err;
 out_rcu_unlock:
@@ -259,7 +265,7 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
 out_sk_release:
 	sk_common_release(sk);
 	sock->sk = NULL;
-	goto out;
+	goto out_module_put;
 }
 
 int __inet6_bind(struct sock *sk, struct sockaddr_unsized *uaddr, int addr_len,
-- 
2.43.0