From: mvs <mvs@openbsd.org>
Date: Sat, 30 Oct 2021 16:24:18 +0000 (+0000)
Subject: Fix the UNIX domain sockets leak in soclose().
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=6cfc9990f5d5342b9f9d525711cf9073055db58a;p=openbsd

Fix the UNIX domain sockets leak in soclose().

Each listening socket has two queues, the `so_q0' where partial connected
sockets linked and the `so_q' where connected but not yet accept(2)ed
sockets linked. Such sockets has no file descriptor allocated, so they
have no access from the userland. When the socket linked to `so_q0' or
`so_q' it has it's `so_head' pointed to the listening socket. The userland
receive sockets from `so_q' by accept(2) which allocates the file
descriptor to the socket.

When userland close(2) listening socket, soclose() should release the
sockets linked to `so_q0' and `so_q' because it's the only place where
they are referenced. It removes the socket from the queue by soqremque().
Since socket is not in the queue it's `so_head' is NULL. Then the socket
passed to soabort() which should destroy it by (*pr_usrreq)() call with
'PRU_ABORT' request.

In UNIX domain sockets layer the unp_drop() only disconnects passed socket
and doesn't destroy it because it's `so_head' is NULL. This socket has the
only access by the UNIX domain sockets garbage collector which leaves it
alive, so the socket is permanently leaked.

This leak was introduced in the revision 1.26 of sys/uipc_socket.c when
soqremque() was placed before soabort(). To fix this the unp_drop() was
replaced by unp_detach() and sofree() in the 'PRU_ABORT' path. unp_drop()
only sets the error and disconnects passed socket. We don't expose this
error and unp_detach() also disconnects the socket before destroy it's
protocol control block. sofree() destroys the rest.

The socket passed to soabort() has no vnode(9) associated, so unp_detach()
don't release `unp_lock'. Also this socket never had associated file
descriptor so it already has 'SS_NOFDREF' flag set.

This diff was also applied to 6.9 and 7.0 branches as errata.
---

diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index c85b72e80d3..a90e88d5b07 100644
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_usrreq.c,v 1.151 2021/10/23 20:44:42 mvs Exp $	*/
+/*	$OpenBSD: uipc_usrreq.c,v 1.152 2021/10/30 16:24:18 mvs Exp $	*/
 /*	$NetBSD: uipc_usrreq.c,v 1.18 1996/02/09 19:00:50 christos Exp $	*/
 
 /*
@@ -305,7 +305,13 @@ uipc_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam,
 		break;
 
 	case PRU_ABORT:
-		unp_drop(unp, ECONNABORTED);
+		unp_detach(unp);
+		/*
+		 * As long as `unp_lock' is taken before entering
+		 * uipc_usrreq() releasing it here would lead to a
+		 * double unlock.
+		 */
+		sofree(so, SL_NOUNLOCK);
 		break;
 
 	case PRU_SENSE: {