All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] tipc: clear sock->sk on the failed-insert path in tipc_sk_create()
@ 2026-07-13  8:23 Daehyeon Ko
  2026-07-14  9:36 ` Tung Quang Nguyen
  2026-07-14 11:55 ` Breno Leitao
  0 siblings, 2 replies; 3+ messages in thread
From: Daehyeon Ko @ 2026-07-13  8:23 UTC (permalink / raw)
  To: netdev
  Cc: Jon Maloy, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Tung Quang Nguyen, tipc-discussion,
	linux-kernel, Daehyeon Ko, stable

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: 07f6c4bc048a ("tipc: convert tipc reference table to use generic rhashtable")
Cc: stable@vger.kernel.org
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
v2: replace the raw KASAN backtrace in the commit message with the
    decoded (scripts/decode_stacktrace.sh) file:line form, as requested
    by Tung Quang Nguyen. No code change.

Link to v1: https://lore.kernel.org/netdev/20260710014440.2055584-1-4ncienth@gmail.com/

 net/tipc/socket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index e564341e0216..55e695748332 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -502,6 +502,7 @@ static int tipc_sk_create(struct net *net, struct socket *sock,
 	tipc_set_sk_state(sk, TIPC_OPEN);
 	if (tipc_sk_insert(tsk)) {
 		sk_free(sk);
+		sock->sk = NULL;
 		pr_warn("Socket create failed; port number exhausted\n");
 		return -EINVAL;
 	}
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: [PATCH net v2] tipc: clear sock->sk on the failed-insert path in tipc_sk_create()
  2026-07-13  8:23 [PATCH net v2] tipc: clear sock->sk on the failed-insert path in tipc_sk_create() Daehyeon Ko
@ 2026-07-14  9:36 ` Tung Quang Nguyen
  2026-07-14 11:55 ` Breno Leitao
  1 sibling, 0 replies; 3+ messages in thread
From: Tung Quang Nguyen @ 2026-07-14  9:36 UTC (permalink / raw)
  To: Daehyeon Ko
  Cc: Jon Maloy, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, tipc-discussion@lists.sourceforge.net,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	netdev@vger.kernel.org

>Subject: [PATCH net v2] tipc: clear sock->sk on the failed-insert path in
>tipc_sk_create()
>
>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.

Reviewed-by: Tung Nguyen <tung.quang.nguyen@est.tech>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] tipc: clear sock->sk on the failed-insert path in tipc_sk_create()
  2026-07-13  8:23 [PATCH net v2] tipc: clear sock->sk on the failed-insert path in tipc_sk_create() Daehyeon Ko
  2026-07-14  9:36 ` Tung Quang Nguyen
@ 2026-07-14 11:55 ` Breno Leitao
  1 sibling, 0 replies; 3+ messages in thread
From: Breno Leitao @ 2026-07-14 11:55 UTC (permalink / raw)
  To: Daehyeon Ko
  Cc: netdev, Jon Maloy, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Tung Quang Nguyen, tipc-discussion,
	linux-kernel, stable

On Mon, Jul 13, 2026 at 05:23:42PM +0900, Daehyeon Ko wrote:
> Clear sock->sk on the failed-insert path so the existing tipc_release()
> NULL check fires and the use-after-free is avoided.

The fix itself looks right: clearing sock->sk on the failed-insert path
is what __sock_create() expects from pf->create() on failure, and it
mirrors the same dangling-sk fix done for AF_SMC in commit d293958a8595
("net/smc: do not leave a dangling sk pointer in __smc_create()").

Reviewed-by: Breno Leitao <leitao@debian.org>

> Fixes: 07f6c4bc048a ("tipc: convert tipc reference table to use generic rhashtable")

Is 07f6c4bc048a the commit that actually introduced this? Or the
sk_free() that got added by commit 00aff3590fc0a ("net: tipc: fix
possible refcount leak in tipc_sk_create()") ?


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-14 11:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-13  8:23 [PATCH net v2] tipc: clear sock->sk on the failed-insert path in tipc_sk_create() Daehyeon Ko
2026-07-14  9:36 ` Tung Quang Nguyen
2026-07-14 11:55 ` Breno Leitao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.