git: 44b83210c8ad - main - unix: Fix a socket leak
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 28 Aug 2026 11:11:21 UTC
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=44b83210c8ad21e907de61584cfeb2752abee19b
commit 44b83210c8ad21e907de61584cfeb2752abee19b
Author: Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-08-28 11:04:20 +0000
Commit: Mark Johnston <markj@FreeBSD.org>
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 <John.Ericson@Obsidian.Systems>
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);
}