When socket splicing is involved, delay the pool_put() after the

splicing thread has finished sotask() with the socket to be freed.
Use after free reported and fix successfully tested by Rivo Nurges.
discussed with mpi@

1 files changed, 27 insertions, 4 deletions

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 2f696b36001..f13f25fc459 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_socket.c,v 1.217 2018/02/19 11:35:41 mpi Exp $	*/
+/*	$OpenBSD: uipc_socket.c,v 1.218 2018/03/01 14:11:11 bluhm Exp $	*/
 /*	$NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $	*/
 
 /*
@@ -60,6 +60,7 @@ int	sosplice(struct socket *, int, off_t, struct timeval *);
 void	sounsplice(struct socket *, struct socket *, int);
 void	soidle(void *);
 void	sotask(void *);
+void	soput(void *);
 int	somove(struct socket *, int);
 
 void	filt_sordetach(struct knote *kn);
@@ -212,13 +213,20 @@ sofree(struct socket *so)
 			    so->so_sp->ssp_soback != so);
 		if (isspliced(so))
 			sounsplice(so, so->so_sp->ssp_socket, 0);
-		pool_put(&sosplice_pool, so->so_sp);
-		so->so_sp = NULL;
 	}
 #endif /* SOCKET_SPLICE */
 	sbrelease(so, &so->so_snd);
 	sorflush(so);
-	pool_put(&socket_pool, so);
+#ifdef SOCKET_SPLICE
+	if (so->so_sp) {
+		/* 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);
+	} else 
+#endif /* SOCKET_SPLICE */
+	{
+		pool_put(&socket_pool, so);
+	}
 }
 
 /*
@@ -1237,6 +1245,21 @@ sotask(void *arg)
 }
 
 /*
+ * The socket splicing task may sleep while grabbing the net lock.  As sofree()
+ * can be called anytime, sotask() can access the socket memory of a freed
+ * socket after wakeup.  So delay the pool_put() after all pending socket
+ * splicing tasks have finished.  Do this by scheduling it on the same thread.
+ */
+void
+soput(void *arg)
+{
+	struct socket *so = arg;
+
+	pool_put(&sosplice_pool, so->so_sp);
+	pool_put(&socket_pool, so);
+}
+
+/*
  * Move data from receive buffer of spliced source socket to send
  * buffer of drain socket.  Try to move as much as possible in one
  * big chunk.  It is a TCP only implementation.