Page MenuHomeFreeBSD
Files F156543702

D53632.id166012.diff
No OneTemporary
Actions

D53632.id166012.diff
View Options

diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -1375,7 +1375,7 @@
struct mbuf *control, *m, *first, *last, *next;
u_int ctl, space, datalen, mbcnt, lastlen;
int error, flags;
- bool nonblock, waitall, peek;
+ bool nonblock, waitall, peek, last_trunc;
MPASS(mp0 == NULL);
@@ -1457,22 +1457,19 @@
control = NULL;
/*
- * Find split point for the next copyout. On exit from the loop:
- * last == NULL - socket to be flushed
- * last != NULL
- * lastlen > last->m_len - uio to be filled, last to be adjusted
- * lastlen == 0 - MT_CONTROL, M_EOR or M_NOTREADY encountered
+ * Find split point for the next copyout. On exit from the loop,
+ * 'last' points to the last mbuf to be copied out. In case if last
+ * mbuf is only partially copied out, 'last_trunc' is set accordingly.
+ * The copyout is protected by the I/O lock only, as writers can only
+ * append to the buffer. We need to record the socket buffer state
+ * and do all length adjustments before dropping the socket buffer lock.
+ * We also can't trust the mbuf queue linkage past the 'last'.
*/
- space = uio->uio_resid;
- datalen = 0;
- for (m = first, last = sb->uxst_fnrdy, lastlen = 0;
- m != sb->uxst_fnrdy;
+ last_trunc = false;
+ for (space = uio->uio_resid, m = last = first, datalen = 0;
+ m != sb->uxst_fnrdy && m->m_type == MT_DATA;
m = STAILQ_NEXT(m, m_stailq)) {
- if (m->m_type != MT_DATA) {
- last = m;
- lastlen = 0;
- break;
- }
+ last = m;
if (space >= m->m_len) {
space -= m->m_len;
datalen += m->m_len;
@@ -1480,29 +1477,29 @@
if (m->m_flags & M_EXT)
mbcnt += m->m_ext.ext_size;
if (m->m_flags & M_EOR) {
- last = STAILQ_NEXT(m, m_stailq);
- lastlen = 0;
flags |= MSG_EOR;
break;
}
} else {
datalen += space;
- last = m;
- lastlen = space;
+ last_trunc = true;
break;
}
}
+ lastlen = last_trunc ? space : last->m_len;
- UIPC_STREAM_SBCHECK(sb);
if (!peek) {
- if (last == NULL)
- STAILQ_INIT(&sb->uxst_mbq);
- else {
+ UIPC_STREAM_SBCHECK(sb);
+ if (last->m_len > lastlen) {
STAILQ_FIRST(&sb->uxst_mbq) = last;
- MPASS(last->m_len > lastlen);
last->m_len -= lastlen;
last->m_data += lastlen;
- }
+ } else if (STAILQ_NEXT(last, m_stailq) == NULL) {
+ /* XXX: just for INVARIANTS, could fall through. */
+ STAILQ_INIT(&sb->uxst_mbq);
+ } else
+ STAILQ_FIRST(&sb->uxst_mbq) =
+ STAILQ_NEXT(last, m_stailq);
MPASS(sb->sb_acc >= datalen);
sb->sb_acc -= datalen;
sb->sb_ccc -= datalen;
@@ -1629,34 +1626,29 @@
}
}
- for (m = first; m != last; m = next) {
+ next = first;
+ do {
+ m = next;
next = STAILQ_NEXT(m, m_stailq);
- error = uiomove(mtod(m, char *), m->m_len, uio);
+ error = uiomove(m == last && last_trunc ?
+ mtod(last, char *) - lastlen : mtod(m, char *),
+ m == last ? lastlen : m->m_len,
+ uio);
if (__predict_false(error)) {
SOCK_IO_RECV_UNLOCK(so);
- if (!peek)
+ if (!peek) {
for (; m != last; m = next) {
next = STAILQ_NEXT(m, m_stailq);
m_free(m);
}
+ if (!last_trunc)
+ m_free(last);
+ }
return (error);
}
- if (!peek)
+ if (!peek && (m != last || !last_trunc))
m_free(m);
- }
- if (last != NULL && lastlen > 0) {
- if (!peek) {
- MPASS(!(m->m_flags & M_PKTHDR));
- MPASS(last->m_data - M_START(last) >= lastlen);
- error = uiomove(mtod(last, char *) - lastlen,
- lastlen, uio);
- } else
- error = uiomove(mtod(last, char *), lastlen, uio);
- if (__predict_false(error)) {
- SOCK_IO_RECV_UNLOCK(so);
- return (error);
- }
- }
+ } while (m != last);
if (waitall && !(flags & MSG_EOR) && uio->uio_resid > 0)
goto restart;
SOCK_IO_RECV_UNLOCK(so);
diff --git a/tests/sys/kern/unix_seqpacket_test.c b/tests/sys/kern/unix_seqpacket_test.c
--- a/tests/sys/kern/unix_seqpacket_test.c
+++ b/tests/sys/kern/unix_seqpacket_test.c
@@ -1314,6 +1314,67 @@
free(params.records);
}
+/* See bug 290658. */
+#define PEEK_RACE_SIZE 10
+#define PEEK_RACE_TRIES 10000
+static void *
+peek_race_writer(void *args)
+{
+ struct timespec ts = {};
+ u_short seed[3];
+ char buf[PEEK_RACE_SIZE];
+ int fd = *(int *)args;
+
+ arc4random_buf(seed, sizeof(seed));
+ for (u_int i = 0; i < PEEK_RACE_TRIES; i++) {
+ ATF_REQUIRE_EQ(PEEK_RACE_SIZE,
+ send(fd, buf, sizeof(buf), MSG_EOR));
+ ts.tv_nsec = nrand48(seed) % 20;
+ (void)clock_nanosleep(CLOCK_MONOTONIC_FAST, 0, &ts, NULL);
+ }
+
+ return (NULL);
+}
+
+static void *
+peek_race_peeker(void *args)
+{
+ char buf[PEEK_RACE_SIZE * 10];
+ int fd = *(int *)args;
+
+ for (u_int i = 0; i < PEEK_RACE_TRIES; i++) {
+ ssize_t rcvd;
+
+ while ((rcvd = recv(fd, buf, sizeof(buf),
+ MSG_PEEK | MSG_DONTWAIT)) == -1)
+ ATF_REQUIRE(errno == EAGAIN);
+ ATF_REQUIRE(rcvd == PEEK_RACE_SIZE);
+
+ ATF_REQUIRE_EQ(PEEK_RACE_SIZE,
+ recv(fd, buf, sizeof(buf), 0));
+ }
+
+ return (NULL);
+}
+
+ATF_TC_WITHOUT_HEAD(peek_race);
+ATF_TC_BODY(peek_race, tc)
+{
+ pthread_t peeker, writer;
+ int sv[2];
+
+ do_socketpair(sv);
+
+ ATF_REQUIRE_EQ(0, pthread_create(&writer, NULL, peek_race_writer,
+ &sv[0]));
+ ATF_REQUIRE_EQ(0, pthread_create(&peeker, NULL, peek_race_peeker,
+ &sv[1]));
+ ATF_REQUIRE_EQ(0, pthread_join(writer, NULL));
+ ATF_REQUIRE_EQ(0, pthread_join(peeker, NULL));
+ close(sv[0]);
+ close(sv[1]);
+}
+
/*
* Main.
*/
@@ -1370,6 +1431,7 @@
ATF_TP_ADD_TC(tp, pipe_128k_8k);
ATF_TP_ADD_TC(tp, pipe_128k_128k);
ATF_TP_ADD_TC(tp, random_eor_and_waitall);
+ ATF_TP_ADD_TC(tp, peek_race);
return atf_no_error();
}

File Metadata

Mime Type
text/plain
Expires
Fri, May 15, 1:38 PM (9 h, 56 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
33085185
Default Alt Text
D53632.id166012.diff (5 KB)

Event Timeline