Message-ID: <c1a23c4d-3ef2-45bd-ad9a-616528e22635@stegemann.de>
Date: Sat, 9 Aug 2025 09:25:48 +0200
From: Sven Stegemann <sven@...gemann.de>
To: "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 syzbot+e62c9db591c30e174662@...kaller.appspotmail.com,
 syzbot+d199b52665b6c3069b94@...kaller.appspotmail.com
Subject: Re: [PATCH net-next] net: kcm: Fix race condition in kcm_unattach()

On 8/9/2025 8:36 AM, Sven Stegemann wrote:
> syzbot found a race condition when kcm_unattach(psock)
> and kcm_release(kcm) are executed at the same time.
> 
> kcm_unattach is missing a check of the flag
> kcm->tx_stopped before calling queue_work().
> 
> If the kcm has a reserved psock, kcm_unattach() might get executed
> between cancel_work_sync() and unreserve_psock() in kcm_release(),
> requeuing kcm->tx_work right before kcm gets freed in kcm_done().
> 
> Remove kcm->tx_stopped and replace it by the less
> error-prone disable_work().

I made a mistake in the subject line. It is supposed to say "[PATCH net]"
instead of "[PATCH net-next".

Also some information about the testing I have done:

I patched msleep() calls into the race windows and wrote a reproducer in C
that reliably triggers a KASAN use-after-free read at the beginning of kcm_tx_work
if run against that kernel build.

With the proposed patch the reproducer does not trigger any crashes or warnings.
The syscalls also return non-negative values.

These are the files I used for debugging:

- Kernel patch:

diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index a4971e6fa943..df61f4715747 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -446,6 +446,8 @@ static struct kcm_psock *reserve_psock(struct kcm_sock *kcm)
 	struct kcm_mux *mux = kcm->mux;
 	struct kcm_psock *psock;
 
+	printk("reserve_psock: call function");
+
 	psock = kcm->tx_psock;
 
 	smp_rmb(); /* Must read tx_psock before tx_wait */
@@ -527,6 +529,8 @@ static void unreserve_psock(struct kcm_sock *kcm)
 	struct kcm_psock *psock;
 	struct kcm_mux *mux = kcm->mux;
 
+	printk("unreserve_psock: call function");
+
 	spin_lock_bh(&mux->lock);
 
 	psock = kcm->tx_psock;
@@ -715,6 +719,10 @@ static void kcm_tx_work(struct work_struct *w)
 	struct sock *sk = &kcm->sk;
 	int err;
 
+	printk("kcm_tx_work: entered function");
+
+	msleep(200);
+
 	lock_sock(sk);
 
 	/* Primarily for SOCK_DGRAM sockets, also handle asynchronous tx
@@ -737,6 +745,9 @@ static void kcm_tx_work(struct work_struct *w)
 
 out:
 	release_sock(sk);
+
+	printk("kcm_tx_work: exiting function");
+
 }
 
 static void kcm_push(struct kcm_sock *kcm)
@@ -1357,6 +1368,8 @@ static void kcm_unattach(struct kcm_psock *psock)
 	struct sock *csk = psock->sk;
 	struct kcm_mux *mux = psock->mux;
 
+	printk("kcm_unattach: entered function");
+
 	lock_sock(csk);
 
 	/* Stop getting callbacks from TCP socket. After this there should
@@ -1419,6 +1432,9 @@ static void kcm_unattach(struct kcm_psock *psock)
 		 */
 		kcm_abort_tx_psock(psock, EPIPE, false);
 
+		printk("kcm_unattach: sleeping before queue_work");
+		msleep(100);
+
 		spin_lock_bh(&mux->lock);
 		if (!psock->tx_kcm) {
 			/* psock now unreserved in window mux was unlocked */
@@ -1429,6 +1445,8 @@ static void kcm_unattach(struct kcm_psock *psock)
 		/* Commit done before queuing work to process it */
 		smp_mb();
 
+		printk("kcm_unattach: queueing tx_work");
+
 		/* Queue tx work to make sure psock->done is handled */
 		queue_work(kcm_wq, &psock->tx_kcm->tx_work);
 		spin_unlock_bh(&mux->lock);
@@ -1446,6 +1464,8 @@ static void kcm_unattach(struct kcm_psock *psock)
 	}
 
 	release_sock(csk);
+
+	printk("kcm_unattach: exiting function");
 }
 
 static int kcm_unattach_ioctl(struct socket *sock, struct kcm_unattach *info)
@@ -1677,6 +1697,8 @@ static int kcm_release(struct socket *sock)
 	struct kcm_mux *mux;
 	struct kcm_psock *psock;
 
+	printk("kcm_release: entered function");
+
 	if (!sk)
 		return 0;
 
@@ -1716,6 +1738,9 @@ static int kcm_release(struct socket *sock)
 	 */
 	cancel_work_sync(&kcm->tx_work);
 
+	printk("kcm_release: sleeping after cancel_work_sync");
+	msleep(150);
+
 	lock_sock(sk);
 	psock = kcm->tx_psock;
 	if (psock) {
@@ -1733,8 +1758,12 @@ static int kcm_release(struct socket *sock)
 
 	sock->sk = NULL;
 
+	printk("kcm_release: freeing kcm");
+
 	kcm_done(kcm);
 
+	printk("kcm_release: exiting function");
+
 	return 0;
 }
--

- Reproducer:

#include <arpa/inet.h>
#include <linux/bpf.h>
#include <linux/socket.h>
#include <linux/in.h>
#include <linux/kcm.h>
#include <linux/bpf_common.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/socket.h>
#include <sys/syscall.h>
#include <sys/wait.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int check_error(int ret, const char *err_message)
{
    if(ret < 0) {
        perror(err_message);
        exit(ret);
    }

    return ret;
}

int main()
{
    // system("busybox ip l set lo up");

    union bpf_attr prog = {
        .prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
        .insn_cnt = 2,
        .insns = (uint64_t)(struct bpf_insn[]){
            {.code=BPF_ALU64|BPF_MOV|BPF_K, .dst_reg=BPF_REG_0, .imm=0},
            {.code=BPF_JMP|BPF_EXIT},
        },
        .license = (__u64) "",
    };

    int tcp_fd, listen_fd, accept_fd, bpf_fd, kcm_fd, mux_fd;

    struct sockaddr_in addr = {
        .sin_family = AF_INET,
        .sin_port = htons(3270),
        .sin_addr.s_addr = inet_addr("127.0.0.1")
    };
    
    check_error( listen_fd = socket(AF_INET, SOCK_STREAM, 0), "socket(tcp)" );
    check_error( bind(listen_fd, (void *)&addr, sizeof(addr)), "bind" ); 
    check_error( listen(listen_fd, 1), "listen" );
    
    check_error( tcp_fd = socket(AF_INET, SOCK_STREAM, 0), "socket(tcp)" );
    check_error( connect(tcp_fd, (void *)&addr, sizeof(addr)), "connect" ); 

    check_error( bpf_fd = syscall(__NR_bpf, BPF_PROG_LOAD, &prog, 48), "bpf" );
    check_error( mux_fd = socket(AF_KCM, SOCK_SEQPACKET, 0), "socket(mux)" );

    check_error( ioctl(mux_fd, SIOCKCMCLONE, &kcm_fd), "clone" );

    struct kcm_attach attach = {tcp_fd, bpf_fd};
    check_error( ioctl(mux_fd, SIOCKCMATTACH, &attach), "attach" );

    size_t msg_len = (1<<24);
    struct iovec iov = {
        .iov_base = mmap(NULL, msg_len, PROT_READ, MAP_SHARED | MAP_ANONYMOUS, -1, 0),
        .iov_len = msg_len,
    };

    struct msghdr msg = {
        .msg_name = "R",
        .msg_namelen = 1,
        .msg_iov = &iov,
        .msg_iovlen = 1,
        0
    };

    check_error( sendmsg(kcm_fd, &msg, MSG_EOR), "sendmsg" );

    printf("Wait 30s for worker threads to finish\n");
    sleep(30);

    if (fork() == 0) {
        sleep(1);
        printf("Calling close from child\n");
        check_error( close(kcm_fd), "close(kcm) (child)" );   

        printf("Child done\n");
    } else {
        check_error( close(kcm_fd), "close(kcm) (parent)" );

        sleep(1);

        printf("Calling unattach from parent\n");
        struct kcm_unattach unattach = {tcp_fd};    
        check_error( ioctl(mux_fd, SIOCKCMUNATTACH, &unattach), "unattach" );

        printf("Parent done\n");
        int wstatus;
        wait(&wstatus);
    }
}

Powered by blists - more mailing lists