diff options
| author | Gleb Smirnoff <glebius@FreeBSD.org> | 2025-11-14 02:39:48 +0000 |
|---|---|---|
| committer | Mark Johnston <markj@FreeBSD.org> | 2026-08-24 19:29:46 +0000 |
| commit | 59e5681527b004ea897ccf569a866725509abff7 (patch) | |
| tree | b628b2a9dea8bdae516df1f02f13bbc62b9fcd6c /tests | |
| parent | 57a2373f86acf8e2dbb334776044c7c1122a7432 (diff) | |
unix/stream: fix a race with MSG_PEEK on SOCK_SEQPACKET with MSG_EOR
The pr_soreceive method first scans the buffer holding the both I/O sx(9)
and socket buffer mutex(9) and after figuring out how much needs to be
copied out drops the mutex. Since the other side may only append to the
buffer, it is safe to continue the operation holding the sx(9) only.
However, the code had a bug that it used pointer in the very last mbuf as
marker of the place where to stop. This worked both in a case when we
drain a buffer completely (marker points at NULL) and in a case when we
wanted to stop at MSG_EOR (marker points at next mbuf after MSG_EOR).
However, this pointer is not consistent after we dropped the socket buffer
mutex.
Rewrite the logic to use the data length as bounds for the copyout cycle.
Provide a test case that reproduces the race. Note that the race is very
hard to hit, thus test will pass on unmodified kernel as well. In a
virtual machine I needed to add tsleep(9) for 10 nanoseconds into the
middle of function to be able to reproduce.
Approved by: so
Security: FreeBSD-SA-26:57.unix
PR: 290658
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D53632
Fixes: d15792780760ef94647af9b377b5f0a80e1826bc
(cherry picked from commit 69f61cee2efb1eec0640ca7de9b2d51599569a5d)
(cherry picked from commit bf22e0dd97a484eff4fde057a98d0ae102b90903)
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/sys/kern/unix_seqpacket_test.c | 62 |
1 files changed, 62 insertions, 0 deletions
diff --git a/tests/sys/kern/unix_seqpacket_test.c b/tests/sys/kern/unix_seqpacket_test.c index b9a6be015241..27bd430430b4 100644 --- a/tests/sys/kern/unix_seqpacket_test.c +++ b/tests/sys/kern/unix_seqpacket_test.c @@ -1314,6 +1314,67 @@ ATF_TC_BODY(random_eor_and_waitall, tc) 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_TCS(tp) 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(); } |