From owner-svn-src-releng@freebsd.org Wed Aug 5 17:14:02 2020 Return-Path: Delivered-To: svn-src-releng@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 5AA9E37C02B; Wed, 5 Aug 2020 17:14:02 +0000 (UTC) (envelope-from gordon@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 "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BMJBL1mSXz3c0d; Wed, 5 Aug 2020 17:14:02 +0000 (UTC) (envelope-from gordon@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 062E71979E; Wed, 5 Aug 2020 17:14:02 +0000 (UTC) (envelope-from gordon@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 075HE1CM000521; Wed, 5 Aug 2020 17:14:01 GMT (envelope-from gordon@FreeBSD.org) Received: (from gordon@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 075HE1SU000519; Wed, 5 Aug 2020 17:14:01 GMT (envelope-from gordon@FreeBSD.org) Message-Id: <202008051714.075HE1SU000519@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: gordon set sender to gordon@FreeBSD.org using -f From: Gordon Tetlow Date: Wed, 5 Aug 2020 17:14:01 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-releng@freebsd.org Subject: svn commit: r363923 - in releng: 11.3/sys/compat/freebsd32 11.4/sys/compat/freebsd32 12.1/sys/compat/freebsd32 X-SVN-Group: releng X-SVN-Commit-Author: gordon X-SVN-Commit-Paths: in releng: 11.3/sys/compat/freebsd32 11.4/sys/compat/freebsd32 12.1/sys/compat/freebsd32 X-SVN-Commit-Revision: 363923 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-releng@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the release engineering / security commits to the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Aug 2020 17:14:02 -0000 Author: gordon Date: Wed Aug 5 17:14:01 2020 New Revision: 363923 URL: https://svnweb.freebsd.org/changeset/base/363923 Log: Fix sendmsg(2) privilege escalation. Approved by: so Security: FreeBSD-SA-20:23.sendmsg Security: CVE-2020-7460 Modified: releng/11.3/sys/compat/freebsd32/freebsd32_misc.c releng/11.4/sys/compat/freebsd32/freebsd32_misc.c releng/12.1/sys/compat/freebsd32/freebsd32_misc.c Modified: releng/11.3/sys/compat/freebsd32/freebsd32_misc.c ============================================================================== --- releng/11.3/sys/compat/freebsd32/freebsd32_misc.c Wed Aug 5 17:13:08 2020 (r363922) +++ releng/11.3/sys/compat/freebsd32/freebsd32_misc.c Wed Aug 5 17:14:01 2020 (r363923) @@ -1112,78 +1112,90 @@ freebsd32_recvmsg(td, uap) static int freebsd32_copyin_control(struct mbuf **mp, caddr_t buf, u_int buflen) { + struct cmsghdr *cm; struct mbuf *m; - void *md; - u_int idx, len, msglen; + void *in, *in1, *md; + u_int msglen, outlen; int error; - buflen = FREEBSD32_ALIGN(buflen); - if (buflen > MCLBYTES) return (EINVAL); + in = malloc(buflen, M_TEMP, M_WAITOK); + error = copyin(buf, in, buflen); + if (error != 0) + goto out; + /* - * Iterate over the buffer and get the length of each message - * in there. This has 32-bit alignment and padding. Use it to - * determine the length of these messages when using 64-bit - * alignment and padding. + * Make a pass over the input buffer to determine the amount of space + * required for 64 bit-aligned copies of the control messages. */ - idx = 0; - len = 0; - while (idx < buflen) { - error = copyin(buf + idx, &msglen, sizeof(msglen)); - if (error) - return (error); - if (msglen < sizeof(struct cmsghdr)) - return (EINVAL); - msglen = FREEBSD32_ALIGN(msglen); - if (idx + msglen > buflen) - return (EINVAL); - idx += msglen; - msglen += CMSG_ALIGN(sizeof(struct cmsghdr)) - - FREEBSD32_ALIGN(sizeof(struct cmsghdr)); - len += CMSG_ALIGN(msglen); - } - - if (len > MCLBYTES) - return (EINVAL); - - m = m_get(M_WAITOK, MT_CONTROL); - if (len > MLEN) - MCLGET(m, M_WAITOK); - m->m_len = len; - - md = mtod(m, void *); + in1 = in; + outlen = 0; while (buflen > 0) { - error = copyin(buf, md, sizeof(struct cmsghdr)); - if (error) + if (buflen < sizeof(*cm)) { + error = EINVAL; break; - msglen = *(u_int *)md; - msglen = FREEBSD32_ALIGN(msglen); + } + cm = (struct cmsghdr *)in1; + if (cm->cmsg_len < FREEBSD32_ALIGN(sizeof(*cm))) { + error = EINVAL; + break; + } + msglen = FREEBSD32_ALIGN(cm->cmsg_len); + if (msglen > buflen || msglen < cm->cmsg_len) { + error = EINVAL; + break; + } + buflen -= msglen; - /* Modify the message length to account for alignment. */ - *(u_int *)md = msglen + CMSG_ALIGN(sizeof(struct cmsghdr)) - - FREEBSD32_ALIGN(sizeof(struct cmsghdr)); + in1 = (char *)in1 + msglen; + outlen += CMSG_ALIGN(sizeof(*cm)) + + CMSG_ALIGN(msglen - FREEBSD32_ALIGN(sizeof(*cm))); + } + if (error == 0 && outlen > MCLBYTES) { + /* + * XXXMJ This implies that the upper limit on 32-bit aligned + * control messages is less than MCLBYTES, and so we are not + * perfectly compatible. However, there is no platform + * guarantee that mbuf clusters larger than MCLBYTES can be + * allocated. + */ + error = EINVAL; + } + if (error != 0) + goto out; - md = (char *)md + CMSG_ALIGN(sizeof(struct cmsghdr)); - buf += FREEBSD32_ALIGN(sizeof(struct cmsghdr)); - buflen -= FREEBSD32_ALIGN(sizeof(struct cmsghdr)); + m = m_get2(outlen, M_WAITOK, MT_CONTROL, 0); + m->m_len = outlen; + md = mtod(m, void *); - msglen -= FREEBSD32_ALIGN(sizeof(struct cmsghdr)); - if (msglen > 0) { - error = copyin(buf, md, msglen); - if (error) - break; - md = (char *)md + CMSG_ALIGN(msglen); - buf += msglen; - buflen -= msglen; - } + /* + * Make a second pass over input messages, copying them into the output + * buffer. + */ + in1 = in; + while (outlen > 0) { + /* Copy the message header and align the length field. */ + cm = md; + memcpy(cm, in1, sizeof(*cm)); + msglen = cm->cmsg_len - FREEBSD32_ALIGN(sizeof(*cm)); + cm->cmsg_len = CMSG_ALIGN(sizeof(*cm)) + msglen; + + /* Copy the message body. */ + in1 = (char *)in1 + FREEBSD32_ALIGN(sizeof(*cm)); + md = (char *)md + CMSG_ALIGN(sizeof(*cm)); + memcpy(md, in1, msglen); + in1 = (char *)in1 + FREEBSD32_ALIGN(msglen); + md = (char *)md + CMSG_ALIGN(msglen); + KASSERT(outlen >= CMSG_ALIGN(sizeof(*cm)) + CMSG_ALIGN(msglen), + ("outlen %u underflow, msglen %u", outlen, msglen)); + outlen -= CMSG_ALIGN(sizeof(*cm)) + CMSG_ALIGN(msglen); } - if (error) - m_free(m); - else - *mp = m; + *mp = m; +out: + free(in, M_TEMP); return (error); } Modified: releng/11.4/sys/compat/freebsd32/freebsd32_misc.c ============================================================================== --- releng/11.4/sys/compat/freebsd32/freebsd32_misc.c Wed Aug 5 17:13:08 2020 (r363922) +++ releng/11.4/sys/compat/freebsd32/freebsd32_misc.c Wed Aug 5 17:14:01 2020 (r363923) @@ -1112,78 +1112,90 @@ freebsd32_recvmsg(td, uap) static int freebsd32_copyin_control(struct mbuf **mp, caddr_t buf, u_int buflen) { + struct cmsghdr *cm; struct mbuf *m; - void *md; - u_int idx, len, msglen; + void *in, *in1, *md; + u_int msglen, outlen; int error; - buflen = FREEBSD32_ALIGN(buflen); - if (buflen > MCLBYTES) return (EINVAL); + in = malloc(buflen, M_TEMP, M_WAITOK); + error = copyin(buf, in, buflen); + if (error != 0) + goto out; + /* - * Iterate over the buffer and get the length of each message - * in there. This has 32-bit alignment and padding. Use it to - * determine the length of these messages when using 64-bit - * alignment and padding. + * Make a pass over the input buffer to determine the amount of space + * required for 64 bit-aligned copies of the control messages. */ - idx = 0; - len = 0; - while (idx < buflen) { - error = copyin(buf + idx, &msglen, sizeof(msglen)); - if (error) - return (error); - if (msglen < sizeof(struct cmsghdr)) - return (EINVAL); - msglen = FREEBSD32_ALIGN(msglen); - if (idx + msglen > buflen) - return (EINVAL); - idx += msglen; - msglen += CMSG_ALIGN(sizeof(struct cmsghdr)) - - FREEBSD32_ALIGN(sizeof(struct cmsghdr)); - len += CMSG_ALIGN(msglen); - } - - if (len > MCLBYTES) - return (EINVAL); - - m = m_get(M_WAITOK, MT_CONTROL); - if (len > MLEN) - MCLGET(m, M_WAITOK); - m->m_len = len; - - md = mtod(m, void *); + in1 = in; + outlen = 0; while (buflen > 0) { - error = copyin(buf, md, sizeof(struct cmsghdr)); - if (error) + if (buflen < sizeof(*cm)) { + error = EINVAL; break; - msglen = *(u_int *)md; - msglen = FREEBSD32_ALIGN(msglen); + } + cm = (struct cmsghdr *)in1; + if (cm->cmsg_len < FREEBSD32_ALIGN(sizeof(*cm))) { + error = EINVAL; + break; + } + msglen = FREEBSD32_ALIGN(cm->cmsg_len); + if (msglen > buflen || msglen < cm->cmsg_len) { + error = EINVAL; + break; + } + buflen -= msglen; - /* Modify the message length to account for alignment. */ - *(u_int *)md = msglen + CMSG_ALIGN(sizeof(struct cmsghdr)) - - FREEBSD32_ALIGN(sizeof(struct cmsghdr)); + in1 = (char *)in1 + msglen; + outlen += CMSG_ALIGN(sizeof(*cm)) + + CMSG_ALIGN(msglen - FREEBSD32_ALIGN(sizeof(*cm))); + } + if (error == 0 && outlen > MCLBYTES) { + /* + * XXXMJ This implies that the upper limit on 32-bit aligned + * control messages is less than MCLBYTES, and so we are not + * perfectly compatible. However, there is no platform + * guarantee that mbuf clusters larger than MCLBYTES can be + * allocated. + */ + error = EINVAL; + } + if (error != 0) + goto out; - md = (char *)md + CMSG_ALIGN(sizeof(struct cmsghdr)); - buf += FREEBSD32_ALIGN(sizeof(struct cmsghdr)); - buflen -= FREEBSD32_ALIGN(sizeof(struct cmsghdr)); + m = m_get2(outlen, M_WAITOK, MT_CONTROL, 0); + m->m_len = outlen; + md = mtod(m, void *); - msglen -= FREEBSD32_ALIGN(sizeof(struct cmsghdr)); - if (msglen > 0) { - error = copyin(buf, md, msglen); - if (error) - break; - md = (char *)md + CMSG_ALIGN(msglen); - buf += msglen; - buflen -= msglen; - } + /* + * Make a second pass over input messages, copying them into the output + * buffer. + */ + in1 = in; + while (outlen > 0) { + /* Copy the message header and align the length field. */ + cm = md; + memcpy(cm, in1, sizeof(*cm)); + msglen = cm->cmsg_len - FREEBSD32_ALIGN(sizeof(*cm)); + cm->cmsg_len = CMSG_ALIGN(sizeof(*cm)) + msglen; + + /* Copy the message body. */ + in1 = (char *)in1 + FREEBSD32_ALIGN(sizeof(*cm)); + md = (char *)md + CMSG_ALIGN(sizeof(*cm)); + memcpy(md, in1, msglen); + in1 = (char *)in1 + FREEBSD32_ALIGN(msglen); + md = (char *)md + CMSG_ALIGN(msglen); + KASSERT(outlen >= CMSG_ALIGN(sizeof(*cm)) + CMSG_ALIGN(msglen), + ("outlen %u underflow, msglen %u", outlen, msglen)); + outlen -= CMSG_ALIGN(sizeof(*cm)) + CMSG_ALIGN(msglen); } - if (error) - m_free(m); - else - *mp = m; + *mp = m; +out: + free(in, M_TEMP); return (error); } Modified: releng/12.1/sys/compat/freebsd32/freebsd32_misc.c ============================================================================== --- releng/12.1/sys/compat/freebsd32/freebsd32_misc.c Wed Aug 5 17:13:08 2020 (r363922) +++ releng/12.1/sys/compat/freebsd32/freebsd32_misc.c Wed Aug 5 17:14:01 2020 (r363923) @@ -1260,78 +1260,90 @@ freebsd32_recvmsg(td, uap) static int freebsd32_copyin_control(struct mbuf **mp, caddr_t buf, u_int buflen) { + struct cmsghdr *cm; struct mbuf *m; - void *md; - u_int idx, len, msglen; + void *in, *in1, *md; + u_int msglen, outlen; int error; - buflen = FREEBSD32_ALIGN(buflen); - if (buflen > MCLBYTES) return (EINVAL); + in = malloc(buflen, M_TEMP, M_WAITOK); + error = copyin(buf, in, buflen); + if (error != 0) + goto out; + /* - * Iterate over the buffer and get the length of each message - * in there. This has 32-bit alignment and padding. Use it to - * determine the length of these messages when using 64-bit - * alignment and padding. + * Make a pass over the input buffer to determine the amount of space + * required for 64 bit-aligned copies of the control messages. */ - idx = 0; - len = 0; - while (idx < buflen) { - error = copyin(buf + idx, &msglen, sizeof(msglen)); - if (error) - return (error); - if (msglen < sizeof(struct cmsghdr)) - return (EINVAL); - msglen = FREEBSD32_ALIGN(msglen); - if (idx + msglen > buflen) - return (EINVAL); - idx += msglen; - msglen += CMSG_ALIGN(sizeof(struct cmsghdr)) - - FREEBSD32_ALIGN(sizeof(struct cmsghdr)); - len += CMSG_ALIGN(msglen); - } - - if (len > MCLBYTES) - return (EINVAL); - - m = m_get(M_WAITOK, MT_CONTROL); - if (len > MLEN) - MCLGET(m, M_WAITOK); - m->m_len = len; - - md = mtod(m, void *); + in1 = in; + outlen = 0; while (buflen > 0) { - error = copyin(buf, md, sizeof(struct cmsghdr)); - if (error) + if (buflen < sizeof(*cm)) { + error = EINVAL; break; - msglen = *(u_int *)md; - msglen = FREEBSD32_ALIGN(msglen); + } + cm = (struct cmsghdr *)in1; + if (cm->cmsg_len < FREEBSD32_ALIGN(sizeof(*cm))) { + error = EINVAL; + break; + } + msglen = FREEBSD32_ALIGN(cm->cmsg_len); + if (msglen > buflen || msglen < cm->cmsg_len) { + error = EINVAL; + break; + } + buflen -= msglen; - /* Modify the message length to account for alignment. */ - *(u_int *)md = msglen + CMSG_ALIGN(sizeof(struct cmsghdr)) - - FREEBSD32_ALIGN(sizeof(struct cmsghdr)); + in1 = (char *)in1 + msglen; + outlen += CMSG_ALIGN(sizeof(*cm)) + + CMSG_ALIGN(msglen - FREEBSD32_ALIGN(sizeof(*cm))); + } + if (error == 0 && outlen > MCLBYTES) { + /* + * XXXMJ This implies that the upper limit on 32-bit aligned + * control messages is less than MCLBYTES, and so we are not + * perfectly compatible. However, there is no platform + * guarantee that mbuf clusters larger than MCLBYTES can be + * allocated. + */ + error = EINVAL; + } + if (error != 0) + goto out; - md = (char *)md + CMSG_ALIGN(sizeof(struct cmsghdr)); - buf += FREEBSD32_ALIGN(sizeof(struct cmsghdr)); - buflen -= FREEBSD32_ALIGN(sizeof(struct cmsghdr)); + m = m_get2(outlen, M_WAITOK, MT_CONTROL, 0); + m->m_len = outlen; + md = mtod(m, void *); - msglen -= FREEBSD32_ALIGN(sizeof(struct cmsghdr)); - if (msglen > 0) { - error = copyin(buf, md, msglen); - if (error) - break; - md = (char *)md + CMSG_ALIGN(msglen); - buf += msglen; - buflen -= msglen; - } + /* + * Make a second pass over input messages, copying them into the output + * buffer. + */ + in1 = in; + while (outlen > 0) { + /* Copy the message header and align the length field. */ + cm = md; + memcpy(cm, in1, sizeof(*cm)); + msglen = cm->cmsg_len - FREEBSD32_ALIGN(sizeof(*cm)); + cm->cmsg_len = CMSG_ALIGN(sizeof(*cm)) + msglen; + + /* Copy the message body. */ + in1 = (char *)in1 + FREEBSD32_ALIGN(sizeof(*cm)); + md = (char *)md + CMSG_ALIGN(sizeof(*cm)); + memcpy(md, in1, msglen); + in1 = (char *)in1 + FREEBSD32_ALIGN(msglen); + md = (char *)md + CMSG_ALIGN(msglen); + KASSERT(outlen >= CMSG_ALIGN(sizeof(*cm)) + CMSG_ALIGN(msglen), + ("outlen %u underflow, msglen %u", outlen, msglen)); + outlen -= CMSG_ALIGN(sizeof(*cm)) + CMSG_ALIGN(msglen); } - if (error) - m_free(m); - else - *mp = m; + *mp = m; +out: + free(in, M_TEMP); return (error); }