Netdev List
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Daehyeon Ko <4ncienth@gmail.com>
Cc: netdev@vger.kernel.org, Jon Maloy <jmaloy@redhat.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Tung Quang Nguyen <tung.quang.nguyen@est.tech>,
	Breno Leitao <leitao@debian.org>,
	tipc-discussion@lists.sourceforge.net,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH net v3] tipc: clear sock->sk on the failed-insert path in tipc_sk_create()
Date: Tue, 21 Jul 2026 17:37:21 +0100	[thread overview]
Message-ID: <20260721163721.GA418547@horms.kernel.org> (raw)
In-Reply-To: <20260714131939.1255974-1-4ncienth@gmail.com>

On Tue, Jul 14, 2026 at 10:19:39PM +0900, Daehyeon Ko wrote:
> When tipc_sk_create() fails to insert the new socket (tipc_sk_insert()
> returns non-zero), its error path frees the sk with sk_free() but leaves
> sock->sk pointing at the freed object:
> 
> 	if (tipc_sk_insert(tsk)) {
> 		sk_free(sk);
> 		pr_warn("Socket create failed; port number exhausted\n");
> 		return -EINVAL;
> 	}
> 
> This is harmless for plain socket(): the syscall layer clears sock->ops
> before releasing, so tipc_release() is never called. It is not harmless
> on the accept() path. tipc_accept() creates the pre-allocated child
> socket with tipc_sk_create(net, new_sock, 0, kern); on failure it leaves
> new_sock->sk dangling and new_sock->ops non-NULL, and do_accept() then
> fput()s the new file, so __sock_release() -> tipc_release() runs
> lock_sock(new_sock->sk) on the freed sk -- a use-after-free write of the
> sk_lock spinlock.
> 
> tipc_release() already guards this exact "failed accept() releases a
> pre-allocated child" case with "if (sk == NULL) return 0;", but the
> guard is bypassed because tipc_sk_create() left sock->sk non-NULL
> (dangling) rather than NULL.
> 
> Clear sock->sk on the failed-insert path so the existing tipc_release()
> NULL check fires and the use-after-free is avoided.
> 
> The tipc_sk_insert() failure is reached when the per-netns socket
> rhashtable hits its max_size (tsk_rht_params.max_size = 1048576, ~2M
> elements) -- i.e. once a netns holds ~2M TIPC sockets every insert
> returns -E2BIG.
> 
>   BUG: KASAN: slab-use-after-free in lock_sock_nested (net/core/sock.c:3839)
>   Write of size 8 at addr ffff8880047cdc38 by task init/1
>    lock_sock_nested (net/core/sock.c:3839)
>    tipc_release (net/tipc/socket.c:638)
>    __sock_release (net/socket.c:710)
>    sock_close (net/socket.c:1501)
>    __fput (fs/file_table.c:512)
>   Allocated by task 1:
>    sk_alloc (net/core/sock.c:2308)
>    tipc_sk_create (net/tipc/socket.c:487)
>    tipc_accept (net/tipc/socket.c:2744)
>    do_accept (net/socket.c:2034)
>   Freed by task 1:
>    __sk_destruct (net/core/sock.c:2391)
>    tipc_sk_create (net/tipc/socket.c:504)
>    tipc_accept (net/tipc/socket.c:2744)
>    do_accept (net/socket.c:2034)
> 
> Fixes: 00aff3590fc0 ("net: tipc: fix possible refcount leak in tipc_sk_create()")
> Cc: stable@vger.kernel.org
> Reviewed-by: Tung Nguyen <tung.quang.nguyen@est.tech>
> Reviewed-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
> ---
> v3: correct the Fixes: tag to 00aff3590fc0 ("net: tipc: fix possible
>     refcount leak in tipc_sk_create()") -- that commit added the sk_free()
>     on the insert-failure path; before it the path leaked sk rather than
>     freeing it, so the use-after-free only exists since then (v5.19+).
>     Thanks Breno. Collect Reviewed-by from Tung and Breno. No code change.
> v2: https://lore.kernel.org/netdev/20260713082342.3803379-1-4ncienth@gmail.com/
> v1: https://lore.kernel.org/netdev/20260710014440.2055584-1-4ncienth@gmail.com/

Reviewed-by: Simon Horman <horms@kernel.org>


      reply	other threads:[~2026-07-21 16:37 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-14 13:19 [PATCH net v3] tipc: clear sock->sk on the failed-insert path in tipc_sk_create() Daehyeon Ko
2026-07-21 16:37 ` Simon Horman [this message]

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=20260721163721.GA418547@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=4ncienth@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jmaloy@redhat.com \
    --cc=kuba@kernel.org \
    --cc=leitao@debian.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=tipc-discussion@lists.sourceforge.net \
    --cc=tung.quang.nguyen@est.tech \
    /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