Re: [PATCH net-next] net: kcm: Fix race condition in kcm_unattach()
From: Sven Stegemann <hidden>
Date: 2025-08-09 07:25:52
Also in:
lkml
Subsystem:
networking [general], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
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);
}
}