mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Kuniyuki Iwashima <kuniyu@google.com>
To: nicoyip.dev@gmail.com
Cc: davem@davemloft.net, edumazet@kernel.org, horms@kernel.org,
	 kuba@kernel.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org,  pabeni@redhat.com,
	stable@vger.kernel.org
Subject: Re: [PATCH net v4] net: pin protocol module before inet socket allocation
Date: Mon, 12 Oct 2026 20:07:37 +0000	[thread overview]
Message-ID: <20261012200752.3771786-1-kuniyu@google.com> (raw)
In-Reply-To: <20261002173844.3537111-1-nicoyip.dev@gmail.com>

From: Chengfeng Ye <nicoyip.dev@gmail.com>
Date: Sat,  3 Oct 2026 01:38:44 +0800
> inet_create() and inet6_create() look up the protocol under RCU, then drop
> RCU before using the resulting proto. A concurrent module unload can
> unregister the protosw, wait for the RCU readers, and destroy the proto's
> slab before sk_alloc() uses it. sk_alloc() may sleep, so the RCU read-side
> critical section cannot simply be extended across the allocation.
> 
> The reported failure occurred during concurrent L2TP socket creation and
> module unload:
> 
>   Oops: general protection fault, probably for non-canonical address
>         0xffff1108974182a0
>   KASAN: maybe wild-memory-access in range
>          [0xfff8a844ba0c1500-0xfff8a844ba0c1507]
>   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
> 
> Cache the proto and its owner under RCU and acquire a temporary module
> reference before dropping RCU. This prevents normal module unload while
> the proto is in use. sk_prot_alloc() takes the socket's own reference;
> drop the temporary reference on every exit after allocation. Use the
> cached proto for the IPv6 backlog callback as well.
> 
> Use try_module_get() with its existing initialization semantics. SCTP
> creates its kernel control socket during module initialization, so
> rejecting MODULE_STATE_COMING would prevent modular SCTP from loading.
> Failed module initialization can still unwind published protocols despite
> module references; that separate publication/unwind lifetime issue is not
> fixed here.
> 
> Fixes: a79af59efd20 ("[NET]: Fix module reference counts for loadable protocol modules")
> Cc: stable@vger.kernel.org
> Assisted-by: GPT-6-Astra
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
> Changes in v4:
> - Remove the MODULE_STATE_COMING rejection. SCTP creates its kernel control
>   socket from module init and must be allowed to use the published protosw.

You can filter the case by @kern.

sockets created from __init_net functions always have
@kern=true otherwise we cannot tear down netns.


> - Keep the owner cached under RCU and use the cached proto for IPv6 backlog
>   setup, retaining those v3 changes.
> - Scope the module-reference guarantee to normal module unload. The earlier
>   review's initialization-failure unwind concern remains a separate issue;
>   rejecting all initializing modules is not a compatible solution.
> - Rebase onto net/main at a7bfaba4823e.
> 
> v3: https://lore.kernel.org/r/20260904111514.584264-1-nicoyip.dev@gmail.com/
> Review: https://lore.kernel.org/r/178887699602.219967.13594894510710999619@kernel.org/
> v2: https://lore.kernel.org/r/20260825172349.232794-1-nicoyip.dev@gmail.com/
> v1: https://lore.kernel.org/r/20260823171311.3857087-1-nicoyip.dev@gmail.com/
> 
>  net/ipv4/af_inet.c  | 14 +++++++++++---
>  net/ipv6/af_inet6.c | 16 ++++++++++++----
>  2 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 32d006c1a8ee..c956afa65ec4 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,14 @@ 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 (!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 +338,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 +404,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 +414,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..9db60af6f38d 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,14 @@ 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 (!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 +183,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 +207,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 +257,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 +267,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,
> 
> base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7
> -- 
> 2.43.0

  parent reply	other threads:[~2026-10-12 20:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-02 17:38 Chengfeng Ye
2026-10-02 17:44 ` netdev-bot+sinfo
2026-10-08 23:49   ` Jakub Kicinski
2026-10-09  2:10     ` Chengfeng Ye
2026-10-12 20:07 ` Kuniyuki Iwashima [this message]
2026-10-13  8:01   ` Chengfeng Ye

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261012200752.3771786-1-kuniyu@google.com \
    --to=kuniyu@google.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nicoyip.dev@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®