From 75775149a8618ec44dca25335be3fdc851f1a03e Mon Sep 17 00:00:00 2001 From: tuexen Date: Thu, 23 May 2019 16:05:47 +0000 Subject: [PATCH] MFC r347975: Improve input validation for the IPPROTO_SCTP level socket options SCTP_CONNECT_X and SCTP_CONNECT_X_DELAYED. MFC r347976: Allow sending on demand SCTP HEARTBEATS only in the ESTABLISHED state. This issue was found by running syzkaller. Approved by: re (gjb@) --- sys/netinet/sctp_usrreq.c | 23 +++++++--------- sys/netinet/sctputil.c | 55 ++++++++++++++++----------------------- sys/netinet/sctputil.h | 7 +++-- 3 files changed, 35 insertions(+), 50 deletions(-) diff --git a/sys/netinet/sctp_usrreq.c b/sys/netinet/sctp_usrreq.c index 2104657c32e..6479436d432 100644 --- a/sys/netinet/sctp_usrreq.c +++ b/sys/netinet/sctp_usrreq.c @@ -1352,13 +1352,12 @@ static int sctp_do_connect_x(struct socket *so, struct sctp_inpcb *inp, void *optval, size_t optsize, void *p, int delay) { - int error = 0; + int error; int creat_lock_on = 0; struct sctp_tcb *stcb = NULL; struct sockaddr *sa; unsigned int num_v6 = 0, num_v4 = 0, *totaddrp, totaddr; uint32_t vrf_id; - int bad_addresses = 0; sctp_assoc_t *a_id; SCTPDBG(SCTP_DEBUG_PCB1, "Connectx called\n"); @@ -1397,17 +1396,12 @@ sctp_do_connect_x(struct socket *so, struct sctp_inpcb *inp, void *optval, totaddrp = (unsigned int *)optval; totaddr = *totaddrp; sa = (struct sockaddr *)(totaddrp + 1); - stcb = sctp_connectx_helper_find(inp, sa, &totaddr, &num_v4, &num_v6, &error, (unsigned int)(optsize - sizeof(int)), &bad_addresses); - if ((stcb != NULL) || bad_addresses) { + error = sctp_connectx_helper_find(inp, sa, totaddr, &num_v4, &num_v6, (unsigned int)(optsize - sizeof(int))); + if (error != 0) { /* Already have or am bring up an association */ SCTP_ASOC_CREATE_UNLOCK(inp); creat_lock_on = 0; - if (stcb) - SCTP_TCB_UNLOCK(stcb); - if (bad_addresses == 0) { - SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP_USRREQ, EALREADY); - error = EALREADY; - } + SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP_USRREQ, error); goto out_now; } #ifdef INET6 @@ -5338,10 +5332,11 @@ sctp_setopt(struct socket *so, int optname, void *optval, size_t optsize, net->dest_state &= ~SCTP_ADDR_NOHB; } if (paddrp->spp_flags & SPP_HB_DEMAND) { - /* on demand HB */ - sctp_send_hb(stcb, net, SCTP_SO_LOCKED); - sctp_chunk_output(inp, stcb, SCTP_OUTPUT_FROM_SOCKOPT, SCTP_SO_LOCKED); - sctp_timer_start(SCTP_TIMER_TYPE_HEARTBEAT, inp, stcb, net); + if (SCTP_GET_STATE(stcb) == SCTP_STATE_OPEN) { + sctp_send_hb(stcb, net, SCTP_SO_LOCKED); + sctp_chunk_output(inp, stcb, SCTP_OUTPUT_FROM_SOCKOPT, SCTP_SO_LOCKED); + sctp_timer_start(SCTP_TIMER_TYPE_HEARTBEAT, inp, stcb, net); + } } if ((paddrp->spp_flags & SPP_PMTUD_DISABLE) && (paddrp->spp_pathmtu >= SCTP_SMALLEST_PMTU)) { if (SCTP_OS_TIMER_PENDING(&net->pmtu_timer.timer)) { diff --git a/sys/netinet/sctputil.c b/sys/netinet/sctputil.c index 2b124dd2a01..8b78a96fa9a 100644 --- a/sys/netinet/sctputil.c +++ b/sys/netinet/sctputil.c @@ -6387,30 +6387,33 @@ sctp_connectx_helper_add(struct sctp_tcb *stcb, struct sockaddr *addr, return (added); } -struct sctp_tcb * +int sctp_connectx_helper_find(struct sctp_inpcb *inp, struct sockaddr *addr, - unsigned int *totaddr, - unsigned int *num_v4, unsigned int *num_v6, int *error, - unsigned int limit, int *bad_addr) + unsigned int totaddr, + unsigned int *num_v4, unsigned int *num_v6, + unsigned int limit) { struct sockaddr *sa; - struct sctp_tcb *stcb = NULL; + struct sctp_tcb *stcb; unsigned int incr, at, i; at = 0; sa = addr; - *error = *num_v6 = *num_v4 = 0; + *num_v6 = *num_v4 = 0; /* account and validate addresses */ - for (i = 0; i < *totaddr; i++) { + if (totaddr == 0) { + return (EINVAL); + } + for (i = 0; i < totaddr; i++) { + if (at + sizeof(struct sockaddr) > limit) { + return (EINVAL); + } switch (sa->sa_family) { #ifdef INET case AF_INET: incr = (unsigned int)sizeof(struct sockaddr_in); if (sa->sa_len != incr) { - SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTPUTIL, EINVAL); - *error = EINVAL; - *bad_addr = 1; - return (NULL); + return (EINVAL); } (*num_v4) += 1; break; @@ -6423,46 +6426,34 @@ sctp_connectx_helper_find(struct sctp_inpcb *inp, struct sockaddr *addr, sin6 = (struct sockaddr_in6 *)sa; if (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) { /* Must be non-mapped for connectx */ - SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTPUTIL, EINVAL); - *error = EINVAL; - *bad_addr = 1; - return (NULL); + return (EINVAL); } incr = (unsigned int)sizeof(struct sockaddr_in6); if (sa->sa_len != incr) { - SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTPUTIL, EINVAL); - *error = EINVAL; - *bad_addr = 1; - return (NULL); + return (EINVAL); } (*num_v6) += 1; break; } #endif default: - *totaddr = i; - incr = 0; - /* we are done */ - break; + return (EINVAL); } - if (i == *totaddr) { - break; + if ((at + incr) > limit) { + return (EINVAL); } SCTP_INP_INCR_REF(inp); stcb = sctp_findassociation_ep_addr(&inp, sa, NULL, NULL, NULL); if (stcb != NULL) { - /* Already have or am bring up an association */ - return (stcb); + SCTP_TCB_UNLOCK(stcb); + return (EALREADY); } else { SCTP_INP_DECR_REF(inp); } - if ((at + incr) > limit) { - *totaddr = i; - break; - } + at += incr; sa = (struct sockaddr *)((caddr_t)sa + incr); } - return ((struct sctp_tcb *)NULL); + return (0); } /* diff --git a/sys/netinet/sctputil.h b/sys/netinet/sctputil.h index 230a13bd5fe..a82813b18d5 100644 --- a/sys/netinet/sctputil.h +++ b/sys/netinet/sctputil.h @@ -209,10 +209,9 @@ int sctp_connectx_helper_add(struct sctp_tcb *stcb, struct sockaddr *addr, int totaddr, int *error); -struct sctp_tcb * -sctp_connectx_helper_find(struct sctp_inpcb *inp, struct sockaddr *addr, - unsigned int *totaddr, unsigned int *num_v4, unsigned int *num_v6, - int *error, unsigned int limit, int *bad_addr); +int +sctp_connectx_helper_find(struct sctp_inpcb *, struct sockaddr *, + unsigned int, unsigned int *, unsigned int *, unsigned int); int sctp_is_there_an_abort_here(struct mbuf *, int, uint32_t *); #ifdef INET6 -- 2.45.0