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

[PATCH net v3] 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

A protosw can also be published while its owner is in
MODULE_STATE_COMING. try_module_get() accepts modules in that state, but
module initialization failure does not wait for such references before
freeing the module. A protocol initialization unwind can therefore destroy
the slab and module image despite the reference.

Cache the proto and its owner while still under RCU. Reject an owner in
MODULE_STATE_COMING, then pin it before leaving the RCU section. The RCU
section protects the rejection path against initialization failure, while
the module reference protects the allocation path against normal unload.
sk_prot_alloc() obtains the socket-lifetime reference. Drop the temporary
reference after sk_alloc(). In inet6_create(), use the cached proto rather
than dereferencing the protosw after leaving RCU.

Fixes: a79af59efd20 ("[NET]: Fix module reference counts for loadable protocol modules")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
Changes in v3:
- Reject protocol owners in MODULE_STATE_COMING before taking a temporary
  module reference.
- Cache the owner under RCU and use that pointer for module_put().
- Use the cached proto for the IPv6 backlog callback instead of
  dereferencing the protosw after RCU unlock.

Changes in v2:
- Pin answer_prot->owner in inet_create()/inet6_create() under RCU instead
  of reordering try_module_get() in sk_prot_alloc().

Link: https://lore.kernel.org/netdev/20260825172349.232794-1-nicoyip.dev@gmail.com/ [v2]
Link: https://lore.kernel.org/netdev/20260823171311.3857087-1-nicoyip.dev@gmail.com/ [v1]
---
 net/ipv4/af_inet.c  | 16 +++++++++++++---
 net/ipv6/af_inet6.c | 18 ++++++++++++++----
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 32d006c1a8ee..6b87f0b6cc22 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -263,6 +263,7 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
 	struct inet_protosw *answer;
 	struct inet_sock *inet;
 	struct proto *answer_prot;
+	struct module *answer_owner;
 	unsigned char answer_flags;
 	int try_loading_module = 0;
 	int err;
@@ -322,9 +323,16 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
 	    !ns_capable(net->user_ns, CAP_NET_RAW))
 		goto out_rcu_unlock;
 
-	sock->ops = answer->ops;
 	answer_prot = answer->prot;
+	answer_owner = answer_prot->owner;
 	answer_flags = answer->flags;
+	if (answer_owner &&
+	    (module_is_coming(answer_owner) ||
+	     !try_module_get(answer_owner))) {
+		err = -EPROTONOSUPPORT;
+		goto out_rcu_unlock;
+	}
+	sock->ops = answer->ops;
 	rcu_read_unlock();
 
 	WARN_ON(!answer_prot->slab);
@@ -332,7 +340,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 +406,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_owner);
 out:
 	return err;
 out_rcu_unlock:
@@ -406,7 +416,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..1df8b2e7e243 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -110,6 +110,7 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
 	struct sock *sk;
 	struct inet_protosw *answer;
 	struct proto *answer_prot;
+	struct module *answer_owner;
 	unsigned char answer_flags;
 	int try_loading_module = 0;
 	int err;
@@ -167,9 +168,16 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
 	    !ns_capable(net->user_ns, CAP_NET_RAW))
 		goto out_rcu_unlock;
 
-	sock->ops = answer->ops;
 	answer_prot = answer->prot;
+	answer_owner = answer_prot->owner;
 	answer_flags = answer->flags;
+	if (answer_owner &&
+	    (module_is_coming(answer_owner) ||
+	     !try_module_get(answer_owner))) {
+		err = -EPROTONOSUPPORT;
+		goto out_rcu_unlock;
+	}
+	sock->ops = answer->ops;
 	rcu_read_unlock();
 
 	WARN_ON(!answer_prot->slab);
@@ -177,7 +185,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);
 
@@ -201,7 +209,7 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
 	sk->sk_family		= PF_INET6;
 	sk->sk_protocol		= protocol;
 
-	sk->sk_backlog_rcv	= answer->prot->backlog_rcv;
+	sk->sk_backlog_rcv	= answer_prot->backlog_rcv;
 
 	inet_sk(sk)->pinet6 = np = inet6_sk_generic(sk);
 	np->hop_limit	= -1;
@@ -251,6 +259,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_owner);
 out:
 	return err;
 out_rcu_unlock:
@@ -259,7 +269,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

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] The newly added `module_is_coming(answer_owner)` rejection in
  `inet_create()` (net/ipv4/af_inet.c) and `inet6_create()`…

--

net: pin protocol module before inet socket allocation

The patch caches answer->prot and its owner module while still under
rcu_read_lock() in inet_create() and inet6_create(), rejects an owner that
is in MODULE_STATE_COMING, and otherwise pins the module with
try_module_get() before dropping RCU. The temporary reference is dropped
after sk_alloc() via a new out_module_put label.

Link: https://lore.kernel.org/netdev/20260825172349.232794-1-nicoyip.dev@gmail.com/ [v2]
Link: https://lore.kernel.org/netdev/20260823171311.3857087-1-nicoyip.dev@gmail.com/ [v1]

> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 32d006c1a8eed..6b87f0b6cc222 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c

[ ... ]

> @@ -322,9 +323,16 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
>  	    !ns_capable(net->user_ns, CAP_NET_RAW))
>  		goto out_rcu_unlock;
>  
> -	sock->ops = answer->ops;
>  	answer_prot = answer->prot;
> +	answer_owner = answer_prot->owner;
>  	answer_flags = answer->flags;
> +	if (answer_owner &&
> +	    (module_is_coming(answer_owner) ||
> +	     !try_module_get(answer_owner))) {
> +		err = -EPROTONOSUPPORT;
> +		goto out_rcu_unlock;
> +	}
> +	sock->ops = answer->ops;
>  	rcu_read_unlock();

[Severity: High]
Can this module_is_coming() rejection break modular protocols that create
a socket for their own protosw from their own module init path?

module_is_coming() is true for the whole duration of the owner module's
init function:

include/linux/module.h:
static inline bool module_is_coming(struct module *mod)
{
        return mod->state == MODULE_STATE_COMING;
}

kernel/module/main.c sets MODULE_STATE_COMING before calling
do_init_module(), and only sets MODULE_STATE_LIVE after mod->init()
returns.

SCTP (CONFIG_IP_SCTP=m) looks like it hits this. sctp_init() publishes the
protosws first and then calls register_pernet_subsys(), which runs the
.init hook immediately for existing net namespaces:

net/sctp/protocol.c:sctp_init() {
	...
	status = sctp_v4_protosw_init();
	...
	status = sctp_v6_protosw_init();
	...
	status = register_pernet_subsys(&sctp_ctrlsock_ops);
	if (status)
		goto err_register_ctrlsock;
	...
}

so sctp_ctrlsock_init() -> sctp_ctl_sock_init() runs inside sctp_init(),
while sctp.ko is still MODULE_STATE_COMING:

net/sctp/protocol.c:sctp_ctl_sock_init() {
	...
	err = inet_ctl_sock_create(&net->sctp.ctl_sock, family,
				   SOCK_SEQPACKET, IPPROTO_SCTP, net);

	/* If IPv6 socket could not be created, try the IPv4 socket */
	if (err < 0 && family == PF_INET6)
		err = inet_ctl_sock_create(&net->sctp.ctl_sock, AF_INET,
					   SOCK_SEQPACKET, IPPROTO_SCTP,
					   net);
	...
}

inet_ctl_sock_create() goes through sock_create_kern() -> __sock_create()
-> inet6_create()/inet_create(), and the new check does not exempt kern
sockets. sctp_prot.owner and sctpv6_prot.owner are THIS_MODULE, which is
non-NULL for a modular build, so both attempts would return
-EPROTONOSUPPORT, sctp_ctrlsock_init() would print "Failed to initialize
the SCTP control sock", register_pernet_subsys() would propagate the
error, and sctp_init() would unwind. Would modprobe sctp then fail
outright?

There is a note about exactly this pattern above module_is_coming():

include/linux/module.h:
/* FIXME: It'd be nice to isolate modules during init, too, so they
   aren't used before they (may) fail.  But presently too much code
   (IDE & SCSI) require entry into the module during init.*/

Separately, for tasks that are not the loading task: once a modular
protocol has published its protosw but has not finished init, a concurrent
socket() finds the protosw, so the existing try_loading_module /
request_module() retry loop is not entered (the lookup succeeded), and the
caller gets -EPROTONOSUPPORT for what is a transient state. Should that
window either succeed or wait rather than returning a user-visible error?

One correction to the reasoning behind this concern: the ipv6 module case
does not apply in this tree. menuconfig IPV6 in net/ipv6/Kconfig is bool
and inet6_init is registered with device_initcall(), so rawv6_prot.owner is
NULL and the answer_owner && test short-circuits. The inet6_create() site
below is still reached by SCTP's first PF_INET6 control-socket attempt,
whose proto owner is the still-coming sctp module.

> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index 282912a119999..1df8b2e7e2437 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c

[ ... ]

> @@ -167,9 +168,16 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
>  	    !ns_capable(net->user_ns, CAP_NET_RAW))
>  		goto out_rcu_unlock;
>  
> -	sock->ops = answer->ops;
>  	answer_prot = answer->prot;
> +	answer_owner = answer_prot->owner;
>  	answer_flags = answer->flags;
> +	if (answer_owner &&
> +	    (module_is_coming(answer_owner) ||
> +	     !try_module_get(answer_owner))) {
> +		err = -EPROTONOSUPPORT;
> +		goto out_rcu_unlock;
> +	}
> +	sock->ops = answer->ops;
>  	rcu_read_unlock();

[Severity: High]
Same question here for the IPv6 path: this is the site reached by
sctp_ctl_sock_init()'s first attempt, inet_ctl_sock_create(..., PF_INET6,
SOCK_SEQPACKET, IPPROTO_SCTP, net), where sctpv6_prot.owner is the sctp
module in MODULE_STATE_COMING. Does that attempt now return
-EPROTONOSUPPORT before the AF_INET fallback, which then fails for the same
reason?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260904111514.584264-1-nicoyip.dev%40gmail.com