From nobody Fri Aug 28 11:11:21 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4hWbL25bj7z6qBlf for ; Fri, 28 Aug 2026 11:11:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "YR2" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4hWbL22smDz4DqD for ; Fri, 28 Aug 2026 11:11:22 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1787915482; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=rGSVJFlUqDZtHNATqrAyjEOFpkIR+oMiVDQZc28RBxg=; b=wb41eR2Wejc1M3bLpJcuPK938wDZYitjm1cvCmigumlWjOiiQ+UINs5gzoG0j+SYzsJvXa ItsAhNDZc01PDMoWHnSJe9wG6EfYGtvVf89Ezq2tuFULK/mkTFfM9qg+V49qqWDfiV4kEa /zjbh8XLNmwOPllIkzc8N1pOqU6zlQ+QfFT+PaB3NuHCZrRoUk6EvjI2C82jqktjnBRGlO VTaUDC9RVc3VwoN+RZUpUBDa9S4a7RT9Ps7YpHMSPWqHI5pcaN4zDU4ao77SkLbLsG9W3I KKoFe3Pua0cX6lp9id886o9WPNWdBamSGpltBtWz7RGrfyGQZeIEWgqf/EbqCw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1787915482; a=rsa-sha256; cv=none; b=bWqZdT8FKoUeKdMQBkYSE0WVwFFZXCtajPblOpQ24pmtxCSHABwoA7TZbufil1G84yK4dx 1kiIW9aQyWCKW9IvXCuummUYCH+piPlg3DcaRJdN9J9M687cKd6aFiIWkfAcLhOcQccCDx KVhHW/mTFOch6Ta14Lv0ZFk6jZOul/aAFGgoEbOYvpDtl7VXs8A54cgYLhYK4bvBdUa2jv MaIFipy0veC/jczFlms/HyRe+WE3diF0Ys2YudPK62nnxzwtachf3wCd9LiqUoHHxYFrPL V1OXYReQldvP9EZGFAp4G0q8bfzJhSLsgJgnd2lzn+pzKr9rOqJwDGJIFYTENA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1787915482; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=rGSVJFlUqDZtHNATqrAyjEOFpkIR+oMiVDQZc28RBxg=; b=j595F17oj0sU7uLt/7JOn889g6N26EhLxWdk4ck+vJqGU58dtz97n2g2DBXhwORUO6dRJ+ UCB7vPxXVX7fQNqLEnKFoi3T/+lBm5YOMIhfzZuU8fuCF+nH8/CHAVNPsg4ZNeGe/Ex8/H 7pMuPTWaOo1JqDmiRWzvIbe6jZA2HuU1l98qUH4YVdSMzDlopv7FquCpZj6ZdpQEwKT0lM lczWEmkj/sVciY7FjuLOHCV0+oIa8GVytq8tLnT/cQrjH7xcRdSKMYgExoPGjHK4tneUsz pKiuku7MxYaHPtp7M0PWxnzzrteu+Ee0lBOi0zuFG+jjAp3xFxjfLyeqNa2XNQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4hWbL217FDz16jg for ; Fri, 28 Aug 2026 11:11:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 1d07d by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Fri, 28 Aug 2026 11:11:21 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: 44b83210c8ad - main - unix: Fix a socket leak List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 44b83210c8ad21e907de61584cfeb2752abee19b Auto-Submitted: auto-generated Date: Fri, 28 Aug 2026 11:11:21 +0000 Message-Id: <6a916cd9.1d07d.20c2f11@gitrepo.freebsd.org> The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=44b83210c8ad21e907de61584cfeb2752abee19b commit 44b83210c8ad21e907de61584cfeb2752abee19b Author: Mark Johnston AuthorDate: 2026-08-28 11:04:20 +0000 Commit: Mark Johnston CommitDate: 2026-08-28 11:10:22 +0000 unix: Fix a socket leak When connecting a unix domain stream socket, we 1. look up the peer (listening) socket, 2. allocate a new socket 3. add the new socket to the listening socket's queue Prior to commit 26147c51546e, this sequence of operations was synchronized by a pool mutex, also acquired in uipc_close(). After commit 26147c51546e, we drop the vnode pool lock immediately after finding the peer socket via a filesystem lookup. This creates a window where it's possible for a connection to add a new socket to the listening queue after the listening queue has been aborted. Fix the race by restoring the old behaviour of holding the pool lock across the solisten_enqueue() call. This is a bit ugly since we need to pass a mutex lock and a vnode through a couple of layers, but it seems like a low-risk solution. Alternately we could add some flag to the listening socket which indicates that no new connections are to be accepted, but I think this will require some changes to the generic socket code. Reported by: pho Fixes: 26147c51546e ("unix: pin the pathname peer by reference across the connect") Reviewed by: olce, kib, John Ericson Differential Revision: https://reviews.freebsd.org/D59201 --- sys/kern/uipc_usrreq.c | 108 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 70 insertions(+), 38 deletions(-) diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c index 284997be3e04..f30119693a76 100644 --- a/sys/kern/uipc_usrreq.c +++ b/sys/kern/uipc_usrreq.c @@ -295,9 +295,9 @@ static int unp_connectat(int, struct socket *, const char *, int, static int unp_connect_peer(struct socket *, struct unpcb *, struct sockaddr **, struct thread *, bool); static int unp_connectat_peer(struct thread *, int, const char *, - struct socket **); + struct socket **, struct mtx **, struct vnode **); static int unp_vnode_peer(struct vnode *, struct thread *, - struct socket **); + struct socket **, struct mtx **, struct vnode **); static void unp_connect2(struct socket *, struct socket *, bool); static void unp_disconnect(struct unpcb *unp, struct unpcb *unp2); static void unp_dispose(struct socket *so); @@ -2955,6 +2955,8 @@ unp_connectat(int fd, struct socket *so, const char *path, int len, struct unpcb *unp; char buf[SOCK_MAXADDRLEN]; struct sockaddr *sa; + struct mtx *mtxp; + struct vnode *vp; int error; bool connreq; @@ -3006,17 +3008,36 @@ unp_connectat(int fd, struct socket *so, const char *path, int len, else sa = NULL; - error = unp_connectat_peer(td, fd, buf, &so2); - if (error != 0) - goto out; - error = unp_connect_peer(so, sotounpcb(so2), &sa, td, - referenced_peerp != NULL); - /* Transfer the reference; the caller releases it after unlocking. */ - if (error == 0 && referenced_peerp != NULL) - *referenced_peerp = so2; - else - sorele(so2); -out: + /* + * Find the peer socket we're connecting to, and connect to it. + * + * If the peer is bound to a name in the filesystem, then we hold + * the vnode pool lock until the connection is established, so as to + * avoid racing with a close of the peer listening socket. If the peer + * is referenced by a file descriptor, then that reference prevents the + * race, so no extra synchronization is needed. + */ + error = unp_connectat_peer(td, fd, buf, &so2, &mtxp, &vp); + if (error == 0) { + error = unp_connect_peer(so, sotounpcb(so2), &sa, td, + referenced_peerp != NULL); + if (error == 0 && referenced_peerp != NULL) { + *referenced_peerp = so2; + so2 = NULL; + } + + /* + * Release references only after the pool lock is dropped in + * order to avoid potential lock ordering issues. + */ + if (mtxp != NULL) { + mtx_unlock(mtxp); + vput(vp); + } + if (so2 != NULL) + sorele(so2); + } + free(sa, M_SONAME); if (__predict_false(error)) { UNP_PCB_LOCK(unp); @@ -3066,7 +3087,7 @@ unp_socket_fd_peer(struct thread *td, int fd, struct socket **so2p) * descriptor in td_dupfd and fails with ENODEV, the same convention open(2) * follows via dupfdopen() for /dev/fd. We honour it here and resolve that * descriptor as the peer, so a plain connect(2) to /dev/fd/N reaches the - * socket. Does not consume 'vp'. + * socket. Consumes the vnode reference. */ static int unp_dupfd_peer(struct vnode *vp, struct thread *td, struct socket **so2p) @@ -3079,13 +3100,14 @@ unp_dupfd_peer(struct vnode *vp, struct thread *td, struct socket **so2p) error = VOP_OPEN(vp, FREAD, td->td_ucred, td, NULL); dupfd = td->td_dupfd; td->td_dupfd = 0; - if (error == ENODEV && dupfd >= 0) - return (unp_socket_fd_peer(td, dupfd, so2p)); - if (error == 0) { + if (error == ENODEV && dupfd >= 0) { + error = unp_socket_fd_peer(td, dupfd, so2p); + } else if (error == 0) { /* Not the dupfd convention: an openable node is not a peer. */ (void)VOP_CLOSE(vp, FREAD, td->td_ucred, td); error = ECONNREFUSED; } + vput(vp); return (error); } @@ -3099,16 +3121,21 @@ unp_dupfd_peer(struct vnode *vp, struct thread *td, struct socket **so2p) * /dev/fd/N pathname fdescfs names a descriptor * ordinary pathname a bound socket looked up by path * - * The caller must release the returned socket with sorele(). + * The caller must release the returned socket with sorele(). If the mutex + * and vnode pointers are filled, they must be released as well. */ static int unp_connectat_peer(struct thread *td, int fd, const char *buf, - struct socket **so2p) + struct socket **so2p, struct mtx **mtxp, struct vnode **vpp) { struct nameidata nd; cap_rights_t rights; int error; + *so2p = NULL; + *mtxp = NULL; + *vpp = NULL; + /* * An empty sun_path means 'fd' names the peer directly. If it is a * socket, it is the peer, so return success (or its error) with no @@ -3131,10 +3158,10 @@ unp_connectat_peer(struct thread *td, int fd, const char *buf, NDFREE_PNBUF(&nd); /* - * Dispatch on the resolved vnode, then drop it: for the socket cases - * the returned reference keeps the peer stable, so the caller holds - * no vnode lock across unp_connect_peer() (which matters for the - * return_locked datagram fast path). + * Find our peer socket. If it comes from a socket on the filesystem, + * then we hold on to the vnode pool lock so as to interlock with a + * close of the listening socket. If the peer comes to us via an fd, + * then the fd reference itself keeps the peer stable. * * A synthetic descriptor node -- as fdescfs fabricates for a /dev/fd/N * path -- carries no type of its own (VNON); opening it yields the @@ -3149,8 +3176,7 @@ unp_connectat_peer(struct thread *td, int fd, const char *buf, if (nd.ni_vp->v_type == VNON) error = unp_dupfd_peer(nd.ni_vp, td, so2p); else - error = unp_vnode_peer(nd.ni_vp, td, so2p); - vput(nd.ni_vp); + error = unp_vnode_peer(nd.ni_vp, td, so2p, mtxp, vpp); return (error); } @@ -3160,13 +3186,10 @@ unp_connectat_peer(struct thread *td, int fd, const char *buf, * enforces the caller's authorization to reach the socket -- filesystem * permission (VOP_ACCESS) and MAC (mac_vnode_check_open) -- which bare readers * of the vnode->pcb binding, such as vfs_unp_reclaim(), deliberately skip. - * - * The returned reference keeps the peer stable for unp_connect_peer() once vp's - * per-vnode binding lock is dropped, so the caller must release it with - * sorele(). Does not consume 'vp'. */ static int -unp_vnode_peer(struct vnode *vp, struct thread *td, struct socket **so2p) +unp_vnode_peer(struct vnode *vp, struct thread *td, struct socket **so2p, + struct mtx **mtxp, struct vnode **vpp) { struct mtx *vplock; struct unpcb *unp2; @@ -3174,25 +3197,34 @@ unp_vnode_peer(struct vnode *vp, struct thread *td, struct socket **so2p) ASSERT_VOP_LOCKED(vp, __func__); - if (vp->v_type != VSOCK) - return (ENOTSOCK); + if (vp->v_type != VSOCK) { + error = ENOTSOCK; + goto fail; + } #ifdef MAC error = mac_vnode_check_open(td->td_ucred, vp, VWRITE | VREAD); if (error != 0) - return (error); + goto fail; #endif error = VOP_ACCESS(vp, VWRITE, td->td_ucred, td); if (error != 0) - return (error); + goto fail; vplock = mtx_pool_find(unp_vp_mtxpool, vp); mtx_lock(vplock); VOP_UNP_CONNECT(vp, &unp2); - if (unp2 == NULL) + if (unp2 == NULL) { + mtx_unlock(vplock); error = ECONNREFUSED; - else - soref(*so2p = unp2->unp_socket); - mtx_unlock(vplock); + goto fail; + } + soref(*so2p = unp2->unp_socket); + *mtxp = vplock; + *vpp = vp; + return (0); + +fail: + vput(vp); return (error); }