From 6cfc9990f5d5342b9f9d525711cf9073055db58a Mon Sep 17 00:00:00 2001 From: mvs Date: Sat, 30 Oct 2021 16:24:18 +0000 Subject: [PATCH] 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. --- sys/kern/uipc_usrreq.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) 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: { -- 2.20.1