RE: [PATCH net] tipc: clear sock->sk on the failed-insert path in tipc_sk_create()

[Tung Quang Nguyen]

>Subject: [PATCH net] 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.
>
>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+0x98/0x150
>  Write of size 8 at addr ffff8880047cdc38 by task init/1
>   lock_sock_nested+0x98/0x150
>   tipc_release+0xa4/0x7a0
>   __sock_release+0x61/0x120
>   sock_close+0x10/0x20
>   __fput+0x1d6/0x490
>  Allocated by task 1:
>   sk_alloc+0x2b/0x380
>   tipc_sk_create+0x82/0xb90
>   tipc_accept+0x14c/0x650
>  Freed by task 1:
>   __sk_destruct+0x22d/0x2d0
>   tipc_sk_create+0x7b8/0xb90
>   tipc_accept+0x14c/0x650
>   do_accept+0x1d2/0x2a0
>
>Fixes: 07f6c4bc048a ("tipc: convert tipc reference table to use generic
>rhashtable")
>Cc: stable@xxxxxxxxxxxxxxx
>Signed-off-by: Daehyeon Ko <4ncienth@xxxxxxxxx>
>---
>This was reported to security@xxxxxxxxxx (Cc: the TIPC maintainer) with no
>response; posting the fix directly to netdev as it is a straightforward one-line
>fix. Full C reproducer available on request.

Yes, please send me your C reproducer.

>
> 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
>