svn commit: r359778 - head/sys/kern
Mark Johnston
markj at FreeBSD.org
Fri Apr 10 20:42:00 UTC 2020
Author: markj
Date: Fri Apr 10 20:41:59 2020
New Revision: 359778
URL: https://svnweb.freebsd.org/changeset/base/359778
Log:
Properly handle disconnected sockets in uipc_ready().
When transmitting over a unix socket, data is placed directly into the
receiving socket's receive buffer, instead of the transmitting socket's
send buffer. This means that when pru_ready is called during
sendfile(), the passed socket does not contain M_NOTREADY mbufs in its
buffers; uipc_ready() must locate the linked socket.
Currently uipc_ready() frees the mbufs if the socket is disconnected,
but this is wrong since the mbufs may still be present in the receiving
socket's buffer after a disconnect. This can result in a use-after-free
and potentially a double free if the receive buffer is flushed after
uipc_ready() frees the mbufs.
Fix the problem by trying harder to locate the correct socket buffer and
calling sbready(): use the global list of SOCK_STREAM unix sockets to
search for a sockbuf containing the now-ready mbufs. Only free the
mbufs if we fail this search.
Reviewed by: jah, kib
Reported and tested by: pho
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D24332
Modified:
head/sys/kern/uipc_usrreq.c
Modified: head/sys/kern/uipc_usrreq.c
==============================================================================
--- head/sys/kern/uipc_usrreq.c Fri Apr 10 18:38:42 2020 (r359777)
+++ head/sys/kern/uipc_usrreq.c Fri Apr 10 20:41:59 2020 (r359778)
@@ -775,6 +775,18 @@ uipc_detach(struct socket *so)
vplock = NULL;
local_unp_rights = 0;
+ SOCK_LOCK(so);
+ if (!SOLISTENING(so)) {
+ /*
+ * Once the socket is removed from the global lists,
+ * uipc_ready() will not be able to locate its socket buffer, so
+ * clear the buffer now. At this point internalized rights have
+ * already been disposed of.
+ */
+ sbrelease(&so->so_rcv, so);
+ }
+ SOCK_UNLOCK(so);
+
UNP_LINK_WLOCK();
LIST_REMOVE(unp, unp_link);
if (unp->unp_gcflag & UNPGC_DEAD)
@@ -1244,19 +1256,54 @@ release:
return (error);
}
+static bool
+uipc_ready_scan(struct socket *so, struct mbuf *m, int count, int *errorp)
+{
+ struct mbuf *mb, *n;
+ struct sockbuf *sb;
+
+ SOCK_LOCK(so);
+ if (SOLISTENING(so)) {
+ SOCK_UNLOCK(so);
+ return (false);
+ }
+ mb = NULL;
+ sb = &so->so_rcv;
+ SOCKBUF_LOCK(sb);
+ if (sb->sb_fnrdy != NULL) {
+ for (mb = sb->sb_mb, n = mb->m_nextpkt; mb != NULL;) {
+ if (mb == m) {
+ *errorp = sbready(sb, m, count);
+ break;
+ }
+ mb = mb->m_next;
+ if (mb == NULL) {
+ mb = n;
+ n = mb->m_nextpkt;
+ }
+ }
+ }
+ SOCKBUF_UNLOCK(sb);
+ SOCK_UNLOCK(so);
+ return (mb != NULL);
+}
+
static int
uipc_ready(struct socket *so, struct mbuf *m, int count)
{
struct unpcb *unp, *unp2;
struct socket *so2;
- int error;
+ int error, i;
unp = sotounpcb(so);
+ KASSERT(so->so_type == SOCK_STREAM,
+ ("%s: unexpected socket type for %p", __func__, so));
+
UNP_PCB_LOCK(unp);
if ((unp2 = unp->unp_conn) == NULL) {
UNP_PCB_UNLOCK(unp);
- goto error;
+ goto search;
}
if (unp != unp2) {
if (UNP_PCB_TRYLOCK(unp2) == 0) {
@@ -1264,25 +1311,39 @@ uipc_ready(struct socket *so, struct mbuf *m, int coun
UNP_PCB_UNLOCK(unp);
UNP_PCB_LOCK(unp2);
if (unp_pcb_rele(unp2))
- goto error;
+ 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);
+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
+ * buffer, so perform an exhaustive search before giving up and freeing
+ * the mbufs.
+ */
+ UNP_LINK_RLOCK();
+ LIST_FOREACH(unp, &unp_shead, unp_link) {
+ if (uipc_ready_scan(unp->unp_socket, m, count, &error))
+ break;
+ }
+ UNP_LINK_RUNLOCK();
+
+ if (unp == NULL) {
+ for (i = 0; i < count; i++)
+ m = m_free(m);
+ error = ECONNRESET;
+ }
return (error);
- error:
- for (int i = 0; i < count; i++)
- m = m_free(m);
- return (ECONNRESET);
}
static int
More information about the svn-src-all
mailing list