From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Unsplice socket before start the teardown
To: Alexander Bluhm <bluhm@openbsd.org>, tech@openbsd.org
Date: Thu, 15 Feb 2024 12:28:28 +0300

Use barriers instead of `ssp_idleto' task and timeout(9)
re-initialization to wait concurrent somove() and soidle().

Index: sys/kern/uipc_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.320
diff -u -p -r1.320 uipc_socket.c
--- sys/kern/uipc_socket.c	12 Feb 2024 22:48:27 -0000	1.320
+++ sys/kern/uipc_socket.c	15 Feb 2024 09:23:28 -0000
@@ -62,8 +62,6 @@ int	sosplice(struct socket *, int, off_t
 void	sounsplice(struct socket *, struct socket *, int);
 void	soidle(void *);
 void	sotask(void *);
-void	soreaper(void *);
-void	soput(void *);
 int	somove(struct socket *, int);
 void	sorflush(struct socket *);
 
@@ -316,36 +314,11 @@ sofree(struct socket *so, int keep_lock)
 	sigio_free(&so->so_sigio);
 	klist_free(&so->so_rcv.sb_klist);
 	klist_free(&so->so_snd.sb_klist);
-#ifdef SOCKET_SPLICE
-	if (issplicedback(so)) {
-		int freeing = SOSP_FREEING_WRITE;
-
-		if (so->so_sp->ssp_soback == so)
-			freeing |= SOSP_FREEING_READ;
-		sounsplice(so->so_sp->ssp_soback, so, freeing);
-	}
-	if (isspliced(so)) {
-		int freeing = SOSP_FREEING_READ;
-
-		if (so == so->so_sp->ssp_socket)
-			freeing |= SOSP_FREEING_WRITE;
-		sounsplice(so, so->so_sp->ssp_socket, freeing);
-	}
-#endif /* SOCKET_SPLICE */
 	sbrelease(so, &so->so_snd);
 	sorflush(so);
 	if (!keep_lock)
 		sounlock(so);
-#ifdef SOCKET_SPLICE
-	if (so->so_sp) {
-		/* Reuse splice idle, sounsplice() has been called before. */
-		timeout_set_proc(&so->so_sp->ssp_idleto, soreaper, so);
-		timeout_add(&so->so_sp->ssp_idleto, 0);
-	} else
-#endif /* SOCKET_SPLICE */
-	{
-		pool_put(&socket_pool, so);
-	}
+	pool_put(&socket_pool, so);
 }
 
 static inline uint64_t
@@ -371,6 +344,31 @@ soclose(struct socket *so, int flags)
 	solock(so);
 	/* Revoke async IO early. There is a final revocation in sofree(). */
 	sigio_free(&so->so_sigio);
+
+#ifdef SOCKET_SPLICE
+	if (issplicedback(so)) {
+		int freeing = SOSP_FREEING_WRITE;
+
+		if (so->so_sp->ssp_soback == so)
+			freeing |= SOSP_FREEING_READ;
+		sounsplice(so->so_sp->ssp_soback, so, freeing);
+	}
+	if (isspliced(so)) {
+		int freeing = SOSP_FREEING_READ;
+
+		if (so == so->so_sp->ssp_socket)
+			freeing |= SOSP_FREEING_WRITE;
+		sounsplice(so, so->so_sp->ssp_socket, freeing);
+	}
+	if (so->so_sp) {
+		sounlock(so);
+		taskq_barrier(sosplice_taskq);
+		timeout_del_barrier(&so->so_sp->ssp_idleto);
+		pool_put(&sosplice_pool, so->so_sp);
+		solock(so);
+	}
+#endif /* SOCKET_SPLICE */
+
 	if (so->so_state & SS_ISCONNECTED) {
 		if (so->so_pcb == NULL)
 			goto discard;
@@ -1441,32 +1439,6 @@ sotask(void *arg)
 
 	/* Avoid user land starvation. */
 	yield();
-}
-
-/*
- * The socket splicing task or idle timeout may sleep while grabbing the net
- * lock.  As sofree() can be called anytime, sotask() or soidle() could access
- * the socket memory of a freed socket after wakeup.  So delay the pool_put()
- * after all pending socket splicing tasks or timeouts have finished.  Do this
- * by scheduling it on the same threads.
- */
-void
-soreaper(void *arg)
-{
-	struct socket *so = arg;
-
-	/* Reuse splice task, sounsplice() has been called before. */
-	task_set(&so->so_sp->ssp_task, soput, so);
-	task_add(sosplice_taskq, &so->so_sp->ssp_task);
-}
-
-void
-soput(void *arg)
-{
-	struct socket *so = arg;
-
-	pool_put(&sosplice_pool, so->so_sp);
-	pool_put(&socket_pool, so);
 }
 
 /*