Page Menu
Home
FreeBSD
Search
Configure Global Search
Log In
Files
F136349346
D26299.id76551.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Flag For Later
Award Token
Size
11 KB
Referenced Files
None
Subscribers
None
D26299.id76551.diff
View Options
Index: sys/kern/uipc_usrreq.c
===================================================================
--- sys/kern/uipc_usrreq.c
+++ sys/kern/uipc_usrreq.c
@@ -368,35 +368,56 @@
UNP_PCB_UNLOCK(unp2);
}
-static __noinline void
-unp_pcb_owned_lock2_slowpath(struct unpcb *unp, struct unpcb **unp2p,
- int *freed)
+/*
+ * Try to lock the connected peer of an already locked socket. In some cases
+ * this requires that we unlock the current socket. The UNP_CONNECTING flag is
+ * used to block concurrent connection attempts while the lock is dropped. The
+ * caller must be careful to revalidate PCB state.
+ */
+static struct unpcb *
+unp_pcb_lock_peer(struct unpcb *unp)
{
struct unpcb *unp2;
- unp2 = *unp2p;
+ UNP_PCB_LOCK_ASSERT(unp);
+ unp2 = unp->unp_conn;
+ if (__predict_false(unp2 == NULL))
+ return (NULL);
+ if (__predict_false(unp == unp2))
+ return (unp);
+
+ KASSERT((unp->unp_flags & UNP_CONNECTING) == 0,
+ ("%s: socket %p is connecting", __func__, unp));
+ UNP_PCB_UNLOCK_ASSERT(unp2);
+
+ if (__predict_true(UNP_PCB_TRYLOCK(unp2)))
+ return (unp2);
+ if ((uintptr_t)unp2 > (uintptr_t)unp) {
+ UNP_PCB_LOCK(unp2);
+ return (unp2);
+ }
+ unp->unp_flags |= UNP_CONNECTING;
unp_pcb_hold(unp2);
UNP_PCB_UNLOCK(unp);
+
UNP_PCB_LOCK(unp2);
UNP_PCB_LOCK(unp);
- *freed = unp_pcb_rele(unp2);
- if (*freed)
- *unp2p = NULL;
+ KASSERT(unp->unp_conn == unp2 || unp->unp_conn == NULL,
+ ("%s: socket %p was reconnected", __func__, unp));
+ KASSERT((unp->unp_flags & UNP_CONNECTING) != 0,
+ ("%s: socket %p has UNP_CONNECTING clear", __func__, unp));
+ unp->unp_flags &= ~UNP_CONNECTING;
+ if (unp_pcb_rele(unp2)) {
+ /* unp2 is unlocked. */
+ return (NULL);
+ }
+ if (unp->unp_conn == NULL) {
+ UNP_PCB_UNLOCK(unp2);
+ return (NULL);
+ }
+ return (unp2);
}
-#define unp_pcb_owned_lock2(unp, unp2, freed) do { \
- freed = 0; \
- UNP_PCB_LOCK_ASSERT(unp); \
- UNP_PCB_UNLOCK_ASSERT(unp2); \
- MPASS((unp) != (unp2)); \
- if (__predict_true(UNP_PCB_TRYLOCK(unp2))) \
- break; \
- else if ((uintptr_t)(unp2) > (uintptr_t)(unp)) \
- UNP_PCB_LOCK(unp2); \
- else \
- unp_pcb_owned_lock2_slowpath((unp), &(unp2), &freed); \
-} while (0)
-
/*
* Definitions of protocols supported in the LOCAL domain.
*/
@@ -710,7 +731,7 @@
struct unpcb *unp, *unp2;
struct vnode *vp = NULL;
struct mtx *vplock;
- int freed;
+
unp = sotounpcb(so);
KASSERT(unp != NULL, ("uipc_close: unp == NULL"));
@@ -728,15 +749,10 @@
VOP_UNP_DETACH(vp);
unp->unp_vnode = NULL;
}
- unp2 = unp->unp_conn;
- if (__predict_false(unp == unp2)) {
- unp_disconnect(unp, unp2);
- } else if (unp2 != NULL) {
- unp_pcb_owned_lock2(unp, unp2, freed);
+ if ((unp2 = unp_pcb_lock_peer(unp)) != NULL)
unp_disconnect(unp, unp2);
- } else {
+ else
UNP_PCB_UNLOCK(unp);
- }
if (vp) {
mtx_unlock(vplock);
vrele(vp);
@@ -765,14 +781,13 @@
struct unpcb *unp, *unp2;
struct mtx *vplock;
struct vnode *vp;
- int freeunp, local_unp_rights;
+ int local_unp_rights;
unp = sotounpcb(so);
KASSERT(unp != NULL, ("uipc_detach: unp == NULL"));
vp = NULL;
vplock = NULL;
- local_unp_rights = 0;
SOCK_LOCK(so);
if (!SOLISTENING(so)) {
@@ -811,21 +826,11 @@
VOP_UNP_DETACH(vp);
unp->unp_vnode = NULL;
}
- if (__predict_false(unp == unp->unp_conn)) {
- unp_disconnect(unp, unp);
- unp2 = NULL;
- } else {
- if ((unp2 = unp->unp_conn) != NULL) {
- unp_pcb_owned_lock2(unp, unp2, freeunp);
- if (freeunp)
- unp2 = NULL;
- }
- unp_pcb_hold(unp);
- if (unp2 != NULL)
- unp_disconnect(unp, unp2);
- else
- UNP_PCB_UNLOCK(unp);
- }
+ if ((unp2 = unp_pcb_lock_peer(unp)) != NULL)
+ unp_disconnect(unp, unp2);
+ else
+ UNP_PCB_UNLOCK(unp);
+
UNP_REF_LIST_LOCK();
while (!LIST_EMPTY(&unp->unp_refs)) {
struct unpcb *ref = LIST_FIRST(&unp->unp_refs);
@@ -838,11 +843,9 @@
unp_drop(ref);
UNP_REF_LIST_LOCK();
}
-
UNP_REF_LIST_UNLOCK();
+
UNP_PCB_LOCK(unp);
- freeunp = unp_pcb_rele(unp);
- MPASS(freeunp == 0);
local_unp_rights = unp_rights;
unp->unp_socket->so_pcb = NULL;
unp->unp_socket = NULL;
@@ -862,24 +865,15 @@
uipc_disconnect(struct socket *so)
{
struct unpcb *unp, *unp2;
- int freed;
unp = sotounpcb(so);
KASSERT(unp != NULL, ("uipc_disconnect: unp == NULL"));
UNP_PCB_LOCK(unp);
- if ((unp2 = unp->unp_conn) == NULL) {
+ if ((unp2 = unp_pcb_lock_peer(unp)) != NULL)
+ unp_disconnect(unp, unp2);
+ else
UNP_PCB_UNLOCK(unp);
- return (0);
- }
- if (__predict_true(unp != unp2)) {
- unp_pcb_owned_lock2(unp, unp2, freed);
- if (__predict_false(freed)) {
- UNP_PCB_UNLOCK(unp);
- return (0);
- }
- }
- unp_disconnect(unp, unp2);
return (0);
}
@@ -997,27 +991,6 @@
return (0);
}
-static int
-connect_internal(struct socket *so, struct sockaddr *nam, struct thread *td)
-{
- int error;
- struct unpcb *unp;
-
- unp = so->so_pcb;
- if (unp->unp_conn != NULL)
- return (EISCONN);
- error = unp_connect(so, nam, td);
- if (error)
- return (error);
- UNP_PCB_LOCK(unp);
- if (unp->unp_conn == NULL) {
- UNP_PCB_UNLOCK(unp);
- if (error == 0)
- error = ENOTCONN;
- }
- return (error);
-}
-
static int
uipc_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
struct mbuf *control, struct thread *td)
@@ -1048,57 +1021,31 @@
const struct sockaddr *from;
if (nam != NULL) {
- /*
- * We return with UNP_PCB_LOCK_HELD so we know that
- * the reference is live if the pointer is valid.
- */
- if ((error = connect_internal(so, nam, td)))
- break;
- MPASS(unp->unp_conn != NULL);
- unp2 = unp->unp_conn;
- } else {
- UNP_PCB_LOCK(unp);
-
- /*
- * Because connect() and send() are non-atomic in a sendto()
- * with a target address, it's possible that the socket will
- * have disconnected before the send() can run. In that case
- * return the slightly counter-intuitive but otherwise
- * correct error that the socket is not connected.
- */
- if ((unp2 = unp->unp_conn) == NULL) {
- UNP_PCB_UNLOCK(unp);
- error = ENOTCONN;
+ /* XXX racy. */
+ if (unp->unp_conn != NULL) {
+ error = EISCONN;
break;
}
- }
- if (__predict_false(unp == unp2)) {
- if (unp->unp_socket == NULL) {
- error = ENOTCONN;
+ error = unp_connect(so, nam, td);
+ if (error != 0)
break;
- }
- goto connect_self;
- }
- unp_pcb_owned_lock2(unp, unp2, freed);
- if (__predict_false(freed)) {
- UNP_PCB_UNLOCK(unp);
- error = ENOTCONN;
- break;
}
+ UNP_PCB_LOCK(unp);
+
/*
- * The socket referencing unp2 may have been closed
- * or unp may have been disconnected if the unp lock
- * was dropped to acquire unp2.
+ * Because connect() and send() are non-atomic in a sendto()
+ * with a target address, it's possible that the socket will
+ * have disconnected before the send() can run. In that case
+ * return the slightly counter-intuitive but otherwise
+ * correct error that the socket is not connected.
*/
- if (__predict_false(unp->unp_conn == NULL) ||
- unp2->unp_socket == NULL) {
+ unp2 = unp_pcb_lock_peer(unp);
+ if (unp2 == NULL) {
UNP_PCB_UNLOCK(unp);
- if (unp_pcb_rele(unp2) == 0)
- UNP_PCB_UNLOCK(unp2);
error = ENOTCONN;
break;
}
- connect_self:
+
if (unp2->unp_flags & UNP_WANTCRED)
control = unp_addsockcred(td, control);
if (unp->unp_addr != NULL)
@@ -1127,36 +1074,31 @@
case SOCK_STREAM:
if ((so->so_state & SS_ISCONNECTED) == 0) {
if (nam != NULL) {
- error = connect_internal(so, nam, td);
+ /* XXX racy. */
+ if (unp->unp_conn != NULL) {
+ error = EISCONN;
+ break;
+ }
+ error = unp_connect(so, nam, td);
if (error != 0)
break;
} else {
error = ENOTCONN;
break;
}
- } else {
- UNP_PCB_LOCK(unp);
}
- if ((unp2 = unp->unp_conn) == NULL) {
+ UNP_PCB_LOCK(unp);
+ if ((unp2 = unp_pcb_lock_peer(unp)) == NULL) {
UNP_PCB_UNLOCK(unp);
error = ENOTCONN;
break;
} else if (so->so_snd.sb_state & SBS_CANTSENDMORE) {
- UNP_PCB_UNLOCK(unp);
+ unp_pcb_unlock_pair(unp, unp2);
error = EPIPE;
break;
- } else if ((unp2 = unp->unp_conn) == NULL) {
- UNP_PCB_UNLOCK(unp);
- error = ENOTCONN;
- break;
}
- unp_pcb_owned_lock2(unp, unp2, freed);
UNP_PCB_UNLOCK(unp);
- if (__predict_false(freed)) {
- error = ENOTCONN;
- break;
- }
if ((so2 = unp2->unp_socket) == NULL) {
UNP_PCB_UNLOCK(unp2);
error = ENOTCONN;
@@ -1291,30 +1233,19 @@
("%s: unexpected socket type for %p", __func__, so));
UNP_PCB_LOCK(unp);
- if ((unp2 = unp->unp_conn) == NULL) {
+ if ((unp2 = unp_pcb_lock_peer(unp)) != NULL) {
UNP_PCB_UNLOCK(unp);
- goto search;
- }
- if (unp != unp2) {
- if (UNP_PCB_TRYLOCK(unp2) == 0) {
- unp_pcb_hold(unp2);
- UNP_PCB_UNLOCK(unp);
- UNP_PCB_LOCK(unp2);
- if (unp_pcb_rele(unp2))
- goto search;
- } else
- UNP_PCB_UNLOCK(unp);
+ so2 = unp2->unp_socket;
+ SOCKBUF_LOCK(&so2->so_rcv);
+ if ((error = sbready(&so2->so_rcv, m, count)) == 0)
+ sorwakeup_locked(so2);
+ else
+ SOCKBUF_UNLOCK(&so2->so_rcv);
+ UNP_PCB_UNLOCK(unp2);
+ return (error);
}
- so2 = unp2->unp_socket;
- SOCKBUF_LOCK(&so2->so_rcv);
- if ((error = sbready(&so2->so_rcv, m, count)) == 0)
- sorwakeup_locked(so2);
- else
- SOCKBUF_UNLOCK(&so2->so_rcv);
- UNP_PCB_UNLOCK(unp2);
- return (error);
+ UNP_PCB_UNLOCK(unp);
-search:
/*
* The receiving socket has been disconnected, but may still be valid.
* In this case, the now-ready mbufs are still present in its socket
@@ -1566,7 +1497,7 @@
char buf[SOCK_MAXADDRLEN];
struct sockaddr *sa;
cap_rights_t rights;
- int error, len, freed;
+ int error, len;
bool connreq;
if (nam->sa_family != AF_UNIX)
@@ -1657,12 +1588,12 @@
UNP_PCB_UNLOCK(unp2);
unp2 = unp3;
- unp_pcb_owned_lock2(unp2, unp, freed);
- if (__predict_false(freed)) {
- UNP_PCB_UNLOCK(unp2);
- error = ECONNREFUSED;
- goto bad2;
- }
+
+ /*
+ * It is safe to block on the PCB lock here since unp2 is
+ * nascent and cannot be connected to any other sockets.
+ */
+ UNP_PCB_LOCK(unp);
#ifdef MAC
mac_socketpeer_set_from_socket(so, so2);
mac_socketpeer_set_from_socket(so2, so);
@@ -1683,6 +1614,8 @@
}
free(sa, M_SONAME);
UNP_PCB_LOCK(unp);
+ KASSERT((unp->unp_flags & UNP_CONNECTING) != 0,
+ ("%s: unp %p has UNP_CONNECTING clear", __func__, unp));
unp->unp_flags &= ~UNP_CONNECTING;
UNP_PCB_UNLOCK(unp);
return (error);
@@ -1992,26 +1925,22 @@
{
struct socket *so = unp->unp_socket;
struct unpcb *unp2;
- int freed;
/*
* Regardless of whether the socket's peer dropped the connection
* with this socket by aborting or disconnecting, POSIX requires
* that ECONNRESET is returned.
*/
- /* acquire a reference so that unp isn't freed from underneath us */
UNP_PCB_LOCK(unp);
if (so)
so->so_error = ECONNRESET;
- unp2 = unp->unp_conn;
- /* Last reference dropped in unp_disconnect(). */
- unp_pcb_rele_notlast(unp);
- if (unp2 == unp) {
- unp_disconnect(unp, unp2);
- } else if (unp2 != NULL) {
- unp_pcb_owned_lock2(unp, unp2, freed);
+ if ((unp2 = unp_pcb_lock_peer(unp)) != NULL) {
+ /* Last reference dropped in unp_disconnect(). */
+ unp_pcb_rele_notlast(unp);
unp_disconnect(unp, unp2);
+ } else if (!unp_pcb_rele(unp)) {
+ UNP_PCB_UNLOCK(unp);
}
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Tue, Nov 18, 8:53 AM (23 m, 34 s)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
25494735
Default Alt Text
D26299.id76551.diff (11 KB)
Attached To
Mode
D26299: [unix(4) cleanup 6/8] Simplify connection peer locking.
Attached
Detach File
Event Timeline
Log In to Comment