From nobody Tue Aug 25 16:01:22 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4hTsw24vDVz6pbTN for ; Tue, 25 Aug 2026 16:01:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "YR2" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4hTsw22dQ3z44LP for ; Tue, 25 Aug 2026 16:01:22 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1787673682; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=/+59bYKNFGt8hik873R/bouUW0SH22MRWWPUr1d9hXg=; b=kKhTjv+8nrSCkrpzdBsLOTUhzh2HKH+D0+SdB4RviVDbtcNjBfwiCHm0n2tXYQ6pzDJ3U7 iQvtEFd79FCMyh8YZBw+NCQ0SoduZRqd90eHWbgYQdo2X2APOZO4TazjChJXb+VWkNLiSq zfBk9IyIKwTlzKc/Q+8k06OjEHfEbzkPQGVixaqCJ5ALwPkWZDOuoBHmdkhn+vQHDg7MX4 9K+vt2boXEu/YBNRK92noyHuKXe9EBLnZrUXUqARO97eCcKeWzqoGeJ5Jq9E3cdXBrSbxH HXzO2tCskKtW5RMxh38pyZycC8+P7CcSBxN40hjcKM4rf64xefEzeOfvDchT4Q== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1787673682; a=rsa-sha256; cv=none; b=sjeeVPzzLi/Y8L3LhkcMWlaAUGnN50zO1YezZB57mSIgmD6LH1QOIO0LIbJcEnRmqaubA9 cU/YU6/PPUXh1CVyn7OGZrl0mzpLmkw2q3u8bMcpcrL5FxtKYG+ty8b8uHZO/Z4Z2XcGY5 0qKDwme8PkffOoRNEkrlmHspZRj6qB3n/+k6LdoL2jAi0TZrz0NUw9CWmiZemYG6kK3Owq LG+qUXzJB9qdXxKFBIyIua460KxMoWuZceGC8Qq/pBh6kVxR9D+wrklAe0u/QRkSFW1XcP tM71BUtO30Ap7JW/PCTXyHrJPv9WxaiGZurazAY0o+t7Bd2rSCpAqSgiwugn7w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1787673682; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=/+59bYKNFGt8hik873R/bouUW0SH22MRWWPUr1d9hXg=; b=SlQkZv8yKXxrX4GagQe5UGc+fcS+XUWA9fU+ZAPuxCEik2YslgRdKJRee1tZGAVWp2N0Ax gJCCOvagfKXZzyf09s4jPvLJwQB7HtABTDRoE10Wn26QNuF1/R1Ime2DqzqgkHKqFXtmk8 +X1DUdmtJk5NE2Z4eXN38eoM7fVevZHgbQ5emmWmwIW+XoagSm0bbaVfGIQnptRYLgyvsM jTbmoh8kNdEIdOo94+KwCEdXU8v8MZnACn55qI53+/Z7RkFnzVDAf8uBqdACk0YqGodv0F IyTHKkGnbifIJ7dJUPLg5gF6WtArhyM+51KY4Plf2v0jMp56UJQVuoS3mDs0bg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4hTsw21cbdzxWW for ; Tue, 25 Aug 2026 16:01:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 313e5 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Tue, 25 Aug 2026 16:01:22 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: 0e4c26a82cb1 - releng/15.1 - unix: Fix some bugs in the SOCK_STREAM receive path List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/releng/15.1 X-Git-Reftype: branch X-Git-Commit: 0e4c26a82cb125be77e8c76160f2397c4bf1ea7b Auto-Submitted: auto-generated Date: Tue, 25 Aug 2026 16:01:22 +0000 Message-Id: <6a8dbc52.313e5.513b1f9a@gitrepo.freebsd.org> The branch releng/15.1 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=0e4c26a82cb125be77e8c76160f2397c4bf1ea7b commit 0e4c26a82cb125be77e8c76160f2397c4bf1ea7b Author: Mark Johnston AuthorDate: 2026-08-10 14:41:33 +0000 Commit: Mark Johnston CommitDate: 2026-08-24 19:34:13 +0000 unix: Fix some bugs in the SOCK_STREAM receive path The main problem is with the handling of errors from unp_externalize(). It turns out that this was quite broken, and unfortunately it's easy to trigger such errors (e.g., by setting a low per-process fd limit with setrlimit()). In non-peek mode, uipc_soreceive_stream_or_seqpacket() cuts a bunch of mbufs from the head of the socket buffer, to be consumed by userspace. When unp_externalize() returns an error, we splice the removed mbuf chain back onto the head of the socket buffer. This is expensive, but that's ok since such errors are rare. The problem is that this cutting is not correctly implemented: it does not clear the "next" pointer for the last mbuf in the chain, so it still points to the first mbuf still resident in the socket buffer. This means that mc_init_m() creates a chain that still includes the rest of the socket buffer, so splicing the chain back into the socket buffer does not work properly. Fix this: fully detach the control chain from the socket buffer so that we can safely use mc_init_m(). Then, incrementally add data mbufs, taking care to handle "part". Fix some related bugs while here: - Don't swallow the error if unp_externalize() fails and there's nothing left in the socket buffer (i.e., control->m_next == NULL). - Roll back changes to the partially read mbuf. Approved by: so Security: FreeBSD-SA-26:57.unix Security: CVE-2026-58090 Reviewed by: glebius MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D58695 (cherry picked from commit 3b93d3597cc336d5637e12ade8642d589cb0bc8a) (cherry picked from commit 06833c3e98137c70309a0c92a96425f521e88c50) --- sys/kern/uipc_usrreq.c | 39 +++++++++++-- sys/sys/mbuf.h | 7 +++ tests/sys/kern/unix_passfd_test.c | 120 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 4 deletions(-) diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c index 3b7048be5fec..6b3ef005b597 100644 --- a/sys/kern/uipc_usrreq.c +++ b/sys/kern/uipc_usrreq.c @@ -1461,14 +1461,26 @@ restart: ctl = 0; first = STAILQ_FIRST(&sb->uxst_mbq); if (first->m_type == MT_CONTROL) { + struct mbuf *prev; + control = first; + prev = NULL; + + /* + * Unlink control messages from the socket buffer. The head of + * the socket buffer queue is updated below. + */ STAILQ_FOREACH_FROM(first, &sb->uxst_mbq, m_stailq) { - if (first->m_type != MT_CONTROL) + if (first->m_type != MT_CONTROL) { + if (!peek && prev != NULL) + STAILQ_NEXT(prev, m_stailq) = NULL; break; + } ctl += first->m_len; mbcnt += MSIZE; if (first->m_flags & M_EXT) mbcnt += first->m_ext.ext_size; + prev = first; } } else control = NULL; @@ -1563,10 +1575,25 @@ restart: */ error = unp_externalize(control, controlp, flags); control = m_free(control); - if (__predict_false(error && control != NULL)) { + if (__predict_false(error != 0)) { struct mchain cmc; - mc_init_m(&cmc, control); + /* + * Build an mbuf chain containing the remainder + * of the control messages and the subsequent + * data, to be prepended back to the socket + * buffer. + */ + if (control != NULL) + mc_init_m(&cmc, control); + else + mc_init(&cmc); + for (m = first; datalen > 0 && m != part; + m = next) { + datalen -= m->m_len; + next = STAILQ_NEXT(m, m_stailq); + mc_append(&cmc, m); + } SOCK_RECVBUF_LOCK(so); if (__predict_false( @@ -1576,7 +1603,7 @@ restart: /* * While the lock was dropped and we * were failing in unp_externalize(), - * the peer could has a) disconnected, + * the peer could have a) disconnected, * b) filled the buffer so that we * can't prepend data back. * These are two edge conditions that @@ -1600,6 +1627,10 @@ restart: sb->sb_mbcnt = 0; STAILQ_FOREACH(m, &sb->uxst_mbq, m_stailq) { if (m->m_type == MT_DATA) { + if (m == part) { + m->m_len += partlen; + m->m_data -= partlen; + } sb->sb_acc += m->m_len; sb->sb_ccc += m->m_len; } else { diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h index f9141bf70742..247112fc72a6 100644 --- a/sys/sys/mbuf.h +++ b/sys/sys/mbuf.h @@ -1745,6 +1745,13 @@ mc_dec(struct mchain *mc, struct mbuf *m) } } +static inline void +mc_init(struct mchain *mc) +{ + STAILQ_INIT(&mc->mc_q); + mc->mc_len = mc->mc_mlen = 0; +} + /* * Get mchain from a classic mbuf chain linked by m_next. Two hacks here: * we use the fact that m_next is alias to m_stailq, we use internal queue(3) diff --git a/tests/sys/kern/unix_passfd_test.c b/tests/sys/kern/unix_passfd_test.c index 66bb406ea14e..0635bddb786e 100644 --- a/tests/sys/kern/unix_passfd_test.c +++ b/tests/sys/kern/unix_passfd_test.c @@ -939,6 +939,125 @@ ATF_TC_BODY(copyout_rights_error, tc) closesocketpair(fd); } +/* + * Exercise handling of errors from unp_externalize(). + */ +ATF_TC_WITHOUT_HEAD(externalize_error_partial_read); +ATF_TC_BODY(externalize_error_partial_read, tc) +{ + struct iovec iovec; + struct msghdr msghdr; + struct rlimit rl, orl; + struct stat sb; + char cmsgbuf[CMSG_SPACE(sizeof(int))]; + char msg1[16]; + char *fill, *rbuf; + size_t fillsz; +#if TEST_PROTO == SOCK_STREAM + size_t got; +#endif + ssize_t len; + int fd[2], nfds, putfd; + + memset(msg1, 'A', sizeof(msg1)); + + domainsocketpair(fd); + devnull(&putfd); + dofstat(putfd, &sb); + nfds = getnfds(); + +#if TEST_PROTO == SOCK_STREAM + fillsz = (size_t)getrecvspace() * 3 / 5; +#elif TEST_PROTO == SOCK_DGRAM + fillsz = 128; +#endif + + fill = malloc(fillsz); + ATF_REQUIRE(fill != NULL); + memset(fill, 'B', fillsz); + rbuf = malloc(sizeof(msg1) + fillsz); + ATF_REQUIRE(rbuf != NULL); + + /* + * The first message carries the rights and a small payload; the second + * queues more data behind it, so that the read below leaves the receive + * buffer non-empty. + */ + len = sendfd_payload(fd[0], putfd, msg1, sizeof(msg1)); + ATF_REQUIRE_MSG(len == (ssize_t)sizeof(msg1), + "sendmsg: %zd bytes sent; expected %zu: %s", len, sizeof(msg1), + strerror(errno)); + len = send(fd[0], fill, fillsz, 0); + ATF_REQUIRE_MSG(len == (ssize_t)fillsz, + "send: %zd bytes sent; expected %zu: %s", len, fillsz, + strerror(errno)); + + /* + * Use fd limits to force receive to fail. + */ + ATF_REQUIRE_MSG(getrlimit(RLIMIT_NOFILE, &orl) == 0, + "getrlimit failed: %s", strerror(errno)); + rl = orl; + rl.rlim_cur = 1; + ATF_REQUIRE_MSG(setrlimit(RLIMIT_NOFILE, &rl) == 0, + "setrlimit failed: %s", strerror(errno)); + + bzero(&msghdr, sizeof(msghdr)); + iovec.iov_base = rbuf; + iovec.iov_len = sizeof(msg1); + msghdr.msg_iov = &iovec; + msghdr.msg_iovlen = 1; + msghdr.msg_control = cmsgbuf; + msghdr.msg_controllen = sizeof(cmsgbuf); + + ATF_REQUIRE_ERRNO(EMFILE, recvmsg(fd[1], &msghdr, 0) == -1); + + ATF_REQUIRE_MSG(setrlimit(RLIMIT_NOFILE, &orl) == 0, + "setrlimit failed: %s", strerror(errno)); + + /* The rights must have been disposed of rather than installed. */ + ATF_REQUIRE_MSG(getnfds() == nfds, "descriptor leaked"); + + /* + * The failed read must leave the socket usable with both payloads still + * queued. + */ +#if TEST_PROTO == SOCK_STREAM + for (got = 0; got < sizeof(msg1) + fillsz; got += (size_t)len) { + len = recv(fd[1], rbuf + got, sizeof(msg1) + fillsz - got, 0); + if (len <= 0) + break; + } + ATF_REQUIRE_MSG(got == sizeof(msg1) + fillsz, + "recovered %zu of %zu bytes after the failed read: %s", got, + sizeof(msg1) + fillsz, strerror(errno)); + ATF_REQUIRE_MSG(memcmp(rbuf, msg1, sizeof(msg1)) == 0, + "first payload corrupted"); + ATF_REQUIRE_MSG(memcmp(rbuf + sizeof(msg1), fill, fillsz) == 0, + "second payload corrupted"); +#elif TEST_PROTO == SOCK_DGRAM + /* + * For datagrams, soreceive_dgram() dequeues the record before + * processing control messages, so the first datagram's payload is + * consumed even when externalize fails. Only the second datagram + * should remain queued. + */ + len = recv(fd[1], rbuf, fillsz, 0); + ATF_REQUIRE_MSG(len == (ssize_t)fillsz, + "second datagram: got %zd bytes, expected %zu: %s", len, fillsz, + strerror(errno)); + ATF_REQUIRE_MSG(memcmp(rbuf, fill, fillsz) == 0, + "second payload corrupted"); +#endif + + dofstat(putfd, &sb); + + free(rbuf); + free(fill); + close(putfd); + closesocketpair(fd); +} + /* * Verify that we can handle empty rights messages. */ @@ -1236,6 +1355,7 @@ ATF_TP_ADD_TCS(tp) ATF_TP_ADD_TC(tp, rights_creds_payload); ATF_TP_ADD_TC(tp, truncated_rights); ATF_TP_ADD_TC(tp, copyout_rights_error); + ATF_TP_ADD_TC(tp, externalize_error_partial_read); ATF_TP_ADD_TC(tp, empty_rights_message); ATF_TP_ADD_TC(tp, control_creates_records); ATF_TP_ADD_TC(tp, cross_jail_dirfd);