From owner-svn-src-head@freebsd.org Sun Sep 30 16:21:32 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id F1AEF10C3C49; Sun, 30 Sep 2018 16:21:31 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A7D8289357; Sun, 30 Sep 2018 16:21:31 +0000 (UTC) (envelope-from tuexen@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 A2B87149BE; Sun, 30 Sep 2018 16:21:31 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w8UGLVRe045251; Sun, 30 Sep 2018 16:21:31 GMT (envelope-from tuexen@FreeBSD.org) Received: (from tuexen@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w8UGLVJs045250; Sun, 30 Sep 2018 16:21:31 GMT (envelope-from tuexen@FreeBSD.org) Message-Id: <201809301621.w8UGLVJs045250@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: tuexen set sender to tuexen@FreeBSD.org using -f From: Michael Tuexen Date: Sun, 30 Sep 2018 16:21:31 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r339024 - head/sys/netinet X-SVN-Group: head X-SVN-Commit-Author: tuexen X-SVN-Commit-Paths: head/sys/netinet X-SVN-Commit-Revision: 339024 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 30 Sep 2018 16:21:32 -0000 Author: tuexen Date: Sun Sep 30 16:21:31 2018 New Revision: 339024 URL: https://svnweb.freebsd.org/changeset/base/339024 Log: Fix the handling of ancillary data for SCTP socket. Implement sctp_process_cmsgs_for_init() and sctp_findassociation_cmsgs() similar to sctp_find_cmsg() to improve consistency and avoid the signed/unsigned issues in sctp_process_cmsgs_for_init() and sctp_findassociation_cmsgs(). Thanks to andrew@ for reporting the problem he found using syzcaller. Approved by: re (kib@) MFC after: 1 week Modified: head/sys/netinet/sctp_output.c Modified: head/sys/netinet/sctp_output.c ============================================================================== --- head/sys/netinet/sctp_output.c Sun Sep 30 12:25:38 2018 (r339023) +++ head/sys/netinet/sctp_output.c Sun Sep 30 16:21:31 2018 (r339024) @@ -3572,7 +3572,6 @@ static int sctp_process_cmsgs_for_init(struct sctp_tcb *stcb, struct mbuf *control, int *error) { struct cmsghdr cmh; - int tlen, at; struct sctp_initmsg initmsg; #ifdef INET struct sockaddr_in sin; @@ -3580,34 +3579,37 @@ sctp_process_cmsgs_for_init(struct sctp_tcb *stcb, str #ifdef INET6 struct sockaddr_in6 sin6; #endif + int tot_len, rem_len, cmsg_data_len, cmsg_data_off, off; - tlen = SCTP_BUF_LEN(control); - at = 0; - while (at < tlen) { - if ((tlen - at) < (int)CMSG_ALIGN(sizeof(cmh))) { + tot_len = SCTP_BUF_LEN(control); + for (off = 0; off < tot_len; off += CMSG_ALIGN(cmh.cmsg_len)) { + rem_len = tot_len - off; + if (rem_len < (int)CMSG_ALIGN(sizeof(cmh))) { /* There is not enough room for one more. */ *error = EINVAL; return (1); } - m_copydata(control, at, sizeof(cmh), (caddr_t)&cmh); + m_copydata(control, off, sizeof(cmh), (caddr_t)&cmh); if (cmh.cmsg_len < CMSG_ALIGN(sizeof(cmh))) { /* We dont't have a complete CMSG header. */ *error = EINVAL; return (1); } - if (((int)cmh.cmsg_len + at) > tlen) { + if ((cmh.cmsg_len > INT_MAX) || ((int)cmh.cmsg_len > rem_len)) { /* We don't have the complete CMSG. */ *error = EINVAL; return (1); } + cmsg_data_len = (int)cmh.cmsg_len - CMSG_ALIGN(sizeof(cmh)); + cmsg_data_off = off + CMSG_ALIGN(sizeof(cmh)); if (cmh.cmsg_level == IPPROTO_SCTP) { switch (cmh.cmsg_type) { case SCTP_INIT: - if ((size_t)(cmh.cmsg_len - CMSG_ALIGN(sizeof(cmh))) < sizeof(struct sctp_initmsg)) { + if (cmsg_data_len < (int)sizeof(struct sctp_initmsg)) { *error = EINVAL; return (1); } - m_copydata(control, at + CMSG_ALIGN(sizeof(cmh)), sizeof(struct sctp_initmsg), (caddr_t)&initmsg); + m_copydata(control, cmsg_data_off, sizeof(struct sctp_initmsg), (caddr_t)&initmsg); if (initmsg.sinit_max_attempts) stcb->asoc.max_init_times = initmsg.sinit_max_attempts; if (initmsg.sinit_num_ostreams) @@ -3662,7 +3664,7 @@ sctp_process_cmsgs_for_init(struct sctp_tcb *stcb, str break; #ifdef INET case SCTP_DSTADDRV4: - if ((size_t)(cmh.cmsg_len - CMSG_ALIGN(sizeof(cmh))) < sizeof(struct in_addr)) { + if (cmsg_data_len < (int)sizeof(struct in_addr)) { *error = EINVAL; return (1); } @@ -3670,7 +3672,7 @@ sctp_process_cmsgs_for_init(struct sctp_tcb *stcb, str sin.sin_family = AF_INET; sin.sin_len = sizeof(struct sockaddr_in); sin.sin_port = stcb->rport; - m_copydata(control, at + CMSG_ALIGN(sizeof(cmh)), sizeof(struct in_addr), (caddr_t)&sin.sin_addr); + m_copydata(control, cmsg_data_off, sizeof(struct in_addr), (caddr_t)&sin.sin_addr); if ((sin.sin_addr.s_addr == INADDR_ANY) || (sin.sin_addr.s_addr == INADDR_BROADCAST) || IN_MULTICAST(ntohl(sin.sin_addr.s_addr))) { @@ -3686,7 +3688,7 @@ sctp_process_cmsgs_for_init(struct sctp_tcb *stcb, str #endif #ifdef INET6 case SCTP_DSTADDRV6: - if ((size_t)(cmh.cmsg_len - CMSG_ALIGN(sizeof(cmh))) < sizeof(struct in6_addr)) { + if (cmsg_data_len < (int)sizeof(struct in6_addr)) { *error = EINVAL; return (1); } @@ -3694,7 +3696,7 @@ sctp_process_cmsgs_for_init(struct sctp_tcb *stcb, str sin6.sin6_family = AF_INET6; sin6.sin6_len = sizeof(struct sockaddr_in6); sin6.sin6_port = stcb->rport; - m_copydata(control, at + CMSG_ALIGN(sizeof(cmh)), sizeof(struct in6_addr), (caddr_t)&sin6.sin6_addr); + m_copydata(control, cmsg_data_off, sizeof(struct in6_addr), (caddr_t)&sin6.sin6_addr); if (IN6_IS_ADDR_UNSPECIFIED(&sin6.sin6_addr) || IN6_IS_ADDR_MULTICAST(&sin6.sin6_addr)) { *error = EINVAL; @@ -3727,7 +3729,6 @@ sctp_process_cmsgs_for_init(struct sctp_tcb *stcb, str break; } } - at += CMSG_ALIGN(cmh.cmsg_len); } return (0); } @@ -3740,7 +3741,6 @@ sctp_findassociation_cmsgs(struct sctp_inpcb **inp_p, int *error) { struct cmsghdr cmh; - int tlen, at; struct sctp_tcb *stcb; struct sockaddr *addr; #ifdef INET @@ -3749,31 +3749,34 @@ sctp_findassociation_cmsgs(struct sctp_inpcb **inp_p, #ifdef INET6 struct sockaddr_in6 sin6; #endif + int tot_len, rem_len, cmsg_data_len, cmsg_data_off, off; - tlen = SCTP_BUF_LEN(control); - at = 0; - while (at < tlen) { - if ((tlen - at) < (int)CMSG_ALIGN(sizeof(cmh))) { + tot_len = SCTP_BUF_LEN(control); + for (off = 0; off < tot_len; off += CMSG_ALIGN(cmh.cmsg_len)) { + rem_len = tot_len - off; + if (rem_len < (int)CMSG_ALIGN(sizeof(cmh))) { /* There is not enough room for one more. */ *error = EINVAL; return (NULL); } - m_copydata(control, at, sizeof(cmh), (caddr_t)&cmh); + m_copydata(control, off, sizeof(cmh), (caddr_t)&cmh); if (cmh.cmsg_len < CMSG_ALIGN(sizeof(cmh))) { /* We dont't have a complete CMSG header. */ *error = EINVAL; return (NULL); } - if (((int)cmh.cmsg_len + at) > tlen) { + if ((cmh.cmsg_len > INT_MAX) || ((int)cmh.cmsg_len > rem_len)) { /* We don't have the complete CMSG. */ *error = EINVAL; return (NULL); } + cmsg_data_len = (int)cmh.cmsg_len - CMSG_ALIGN(sizeof(cmh)); + cmsg_data_off = off + CMSG_ALIGN(sizeof(cmh)); if (cmh.cmsg_level == IPPROTO_SCTP) { switch (cmh.cmsg_type) { #ifdef INET case SCTP_DSTADDRV4: - if ((size_t)(cmh.cmsg_len - CMSG_ALIGN(sizeof(cmh))) < sizeof(struct in_addr)) { + if (cmsg_data_len < (int)sizeof(struct in_addr)) { *error = EINVAL; return (NULL); } @@ -3781,13 +3784,13 @@ sctp_findassociation_cmsgs(struct sctp_inpcb **inp_p, sin.sin_family = AF_INET; sin.sin_len = sizeof(struct sockaddr_in); sin.sin_port = port; - m_copydata(control, at + CMSG_ALIGN(sizeof(cmh)), sizeof(struct in_addr), (caddr_t)&sin.sin_addr); + m_copydata(control, cmsg_data_off, sizeof(struct in_addr), (caddr_t)&sin.sin_addr); addr = (struct sockaddr *)&sin; break; #endif #ifdef INET6 case SCTP_DSTADDRV6: - if ((size_t)(cmh.cmsg_len - CMSG_ALIGN(sizeof(cmh))) < sizeof(struct in6_addr)) { + if (cmsg_data_len < (int)sizeof(struct in6_addr)) { *error = EINVAL; return (NULL); } @@ -3795,7 +3798,7 @@ sctp_findassociation_cmsgs(struct sctp_inpcb **inp_p, sin6.sin6_family = AF_INET6; sin6.sin6_len = sizeof(struct sockaddr_in6); sin6.sin6_port = port; - m_copydata(control, at + CMSG_ALIGN(sizeof(cmh)), sizeof(struct in6_addr), (caddr_t)&sin6.sin6_addr); + m_copydata(control, cmsg_data_off, sizeof(struct in6_addr), (caddr_t)&sin6.sin6_addr); #ifdef INET if (IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr)) { in6_sin6_2_sin(&sin, &sin6); @@ -3816,7 +3819,6 @@ sctp_findassociation_cmsgs(struct sctp_inpcb **inp_p, } } } - at += CMSG_ALIGN(cmh.cmsg_len); } return (NULL); }