net/ipv4/af_inet.c | 10 ++++++++-- net/ipv6/af_inet6.c | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-)
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@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
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.0This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: pin protocol module before inet socket allocation
inet_create() and inet6_create() look up the inet_protosw under RCU and then
drop RCU before using answer_prot across the sleeping sk_alloc(). The patch
takes try_module_get(answer_prot->owner) while still under RCU and drops the
reference on every path after sk_alloc().
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 32d006c1a8eed..88a18ac0e6e42 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);
[Severity: High]
Does a plain try_module_get() here cover the case where the protocol module
is still inside its own init and then rolls back?
try_module_get() only gates on module_is_live():
kernel/module/main.c:try_module_get() {
...
if (likely(module_is_live(module) &&
atomic_inc_not_zero(&module->refcnt) != 0))
...
}
include/linux/module.h:
/* FIXME: It'd be nice to isolate modules during init, too, so they
aren't used before they (may) fail. */
static inline bool module_is_live(struct module *mod)
{
return mod->state != MODULE_STATE_GOING;
}
So the pin succeeds for a module in MODULE_STATE_COMING, i.e. while
do_one_initcall(mod->init) is still running with module_mutex dropped.
Some protocol modules publish the protosw early in init and then destroy the
slab from their init-failure unwind, which does not consult the module
refcount:
net/sctp/protocol.c:sctp_v4_protosw_exit() {
inet_unregister_protosw(&sctp_stream_protosw);
inet_unregister_protosw(&sctp_seqpacket_protosw);
proto_unregister(&sctp_prot);
}
reached from the err_protosw_init label in sctp_init() after
sctp_v4_protosw_init() has already registered the protosw. net/smc has the
same shape:
net/smc/smc_inet.c:smc_inet_init() {
...
out_inet6_protosw:
proto_unregister(&smc_inet6_prot);
out_inet6_prot:
inet_unregister_protosw(&smc_inet_protosw);
proto_unregister(&smc_inet_prot);
...
}
Can the sequence the commit message describes still happen with that?
CPU 0 inet_create CPU 1 modprobe sctp (mod->init)
rcu_read_lock() sctp_v4_protosw_init() /* published */
answer_prot = answer->prot
try_module_get() /* COMING -> ok */
rcu_read_unlock() /* later init step fails */
sctp_v4_protosw_exit()
inet_unregister_protosw()
synchronize_net() /* returns at once */
proto_unregister()
kmem_cache_destroy(slab)
WARN_ON(!answer_prot->slab)
sk_alloc() -> kmem_cache_alloc(stale)
The kernel's own helper rejects exactly this state, which suggests a plain
reference is not a lifetime guarantee during init:
kernel/module/main.c:
/*
* We require a truly strong try_module_get(): 0 means success.
* Otherwise an error is returned due to ongoing or failed
* initialization etc.
*/
static inline int strong_try_module_get(struct module *mod)
{
BUG_ON(mod && mod->state == MODULE_STATE_UNFORMED);
if (mod && mod->state == MODULE_STATE_COMING)
return -EBUSY;
Would rejecting owners in MODULE_STATE_COMING (or taking a reference tied to
the protosw/proto lifetime instead of the module refcount) close the residual
window?
> @@ -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:
[Severity: High]
On the same init-failure path, can module_put(answer_prot->owner) here read
freed memory?
answer_prot lives in the module's .data, and do_init_module() frees the module
image without waiting for foreign references:
kernel/module/main.c:do_init_module() {
...
fail:
/* Try to protect us from buggy refcounters. */
mod->state = MODULE_STATE_GOING;
synchronize_rcu();
module_put(mod);
...
free_module(mod);
}
That is unlike the delete_module() path, which does the refcount handshake in
try_stop_module()/try_release_module_ref(). So after free_module() runs,
both answer_prot->owner and the sock->ops pointer already stored above refer
to freed module memory.
[ ... ]
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index 282912a119999..82b390ce3e0cd 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);
[Severity: High]
inet6_create() has the same exclusion, and it also dereferences the protosw
after RCU is dropped:
sk->sk_backlog_rcv = answer->prot->backlog_rcv;
Does the COMING-module case above apply here too, given that sctp and smc
register their IPv6 protosw during init and unwind through
inet_unregister_protosw() + proto_unregister() on failure?
[ ... ]
--
pw-bot: cr
On Mon, Aug 31, 2026 at 7:57 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net: pin protocol module before inet socket allocation
>
> inet_create() and inet6_create() look up the inet_protosw under RCU and then
> drop RCU before using answer_prot across the sleeping sk_alloc(). The patch
> takes try_module_get(answer_prot->owner) while still under RCU and drops the
> reference on every path after sk_alloc().
>
> > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > index 32d006c1a8eed..88a18ac0e6e42 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);
>
> [Severity: High]
> Does a plain try_module_get() here cover the case where the protocol module
> is still inside its own init and then rolls back?
>
> try_module_get() only gates on module_is_live():
>
> kernel/module/main.c:try_module_get() {
> ...
> if (likely(module_is_live(module) &&
> atomic_inc_not_zero(&module->refcnt) != 0))
> ...
> }
>
> include/linux/module.h:
> /* FIXME: It'd be nice to isolate modules during init, too, so they
> aren't used before they (may) fail. */
> static inline bool module_is_live(struct module *mod)
> {
> return mod->state != MODULE_STATE_GOING;
> }
>
> So the pin succeeds for a module in MODULE_STATE_COMING, i.e. while
> do_one_initcall(mod->init) is still running with module_mutex dropped.
>
> Some protocol modules publish the protosw early in init and then destroy the
> slab from their init-failure unwind, which does not consult the module
> refcount:
>
> net/sctp/protocol.c:sctp_v4_protosw_exit() {
> inet_unregister_protosw(&sctp_stream_protosw);
> inet_unregister_protosw(&sctp_seqpacket_protosw);
> proto_unregister(&sctp_prot);
> }
>
> reached from the err_protosw_init label in sctp_init() after
> sctp_v4_protosw_init() has already registered the protosw. net/smc has the
> same shape:
>
> net/smc/smc_inet.c:smc_inet_init() {
> ...
> out_inet6_protosw:
> proto_unregister(&smc_inet6_prot);
> out_inet6_prot:
> inet_unregister_protosw(&smc_inet_protosw);
> proto_unregister(&smc_inet_prot);
> ...
> }
>
> Can the sequence the commit message describes still happen with that?
>
> CPU 0 inet_create CPU 1 modprobe sctp (mod->init)
> rcu_read_lock() sctp_v4_protosw_init() /* published */
> answer_prot = answer->prot
> try_module_get() /* COMING -> ok */
> rcu_read_unlock() /* later init step fails */
> sctp_v4_protosw_exit()
> inet_unregister_protosw()
> synchronize_net() /* returns at once */
> proto_unregister()
> kmem_cache_destroy(slab)
> WARN_ON(!answer_prot->slab)
> sk_alloc() -> kmem_cache_alloc(stale)
>
> The kernel's own helper rejects exactly this state, which suggests a plain
> reference is not a lifetime guarantee during init:
>
> kernel/module/main.c:
> /*
> * We require a truly strong try_module_get(): 0 means success.
> * Otherwise an error is returned due to ongoing or failed
> * initialization etc.
> */
> static inline int strong_try_module_get(struct module *mod)
> {
> BUG_ON(mod && mod->state == MODULE_STATE_UNFORMED);
> if (mod && mod->state == MODULE_STATE_COMING)
> return -EBUSY;
>
> Would rejecting owners in MODULE_STATE_COMING (or taking a reference tied to
> the protosw/proto lifetime instead of the module refcount) close the residual
> window?
>
> > @@ -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:
>
> [Severity: High]
> On the same init-failure path, can module_put(answer_prot->owner) here read
> freed memory?
>
> answer_prot lives in the module's .data, and do_init_module() frees the module
> image without waiting for foreign references:
>
> kernel/module/main.c:do_init_module() {
> ...
> fail:
> /* Try to protect us from buggy refcounters. */
> mod->state = MODULE_STATE_GOING;
> synchronize_rcu();
> module_put(mod);
> ...
> free_module(mod);
> }
>
> That is unlike the delete_module() path, which does the refcount handshake in
> try_stop_module()/try_release_module_ref(). So after free_module() runs,
> both answer_prot->owner and the sock->ops pointer already stored above refer
> to freed module memory.
>
> [ ... ]
>
> > diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> > index 282912a119999..82b390ce3e0cd 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);
>
> [Severity: High]
> inet6_create() has the same exclusion, and it also dereferences the protosw
> after RCU is dropped:
>
> sk->sk_backlog_rcv = answer->prot->backlog_rcv;
>
> Does the COMING-module case above apply here too, given that sctp and smc
> register their IPv6 protosw during init and unwind through
> inet_unregister_protosw() + proto_unregister() on failure?
>
> [ ... ]
> --
> pw-bot: cr
Thanks, a v3 is sent to address both the problems.
Best regards,
Chengfeng
© 2016 - 2026 Red Hat, Inc.