From owner-svn-src-all@freebsd.org Mon Aug 24 09:06:47 2020 Return-Path: Delivered-To: svn-src-all@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 B91603B7016; Mon, 24 Aug 2020 09:06:47 +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.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 4BZmTM4RnZz4R2F; Mon, 24 Aug 2020 09:06:47 +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 7C0011622D; Mon, 24 Aug 2020 09:06:47 +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 07O96lBC096824; Mon, 24 Aug 2020 09:06:47 GMT (envelope-from tuexen@FreeBSD.org) Received: (from tuexen@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 07O96l0x096822; Mon, 24 Aug 2020 09:06:47 GMT (envelope-from tuexen@FreeBSD.org) Message-Id: <202008240906.07O96l0x096822@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: tuexen set sender to tuexen@FreeBSD.org using -f From: Michael Tuexen Date: Mon, 24 Aug 2020 09:06:47 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r364638 - stable/12/sys/netinet X-SVN-Group: stable-12 X-SVN-Commit-Author: tuexen X-SVN-Commit-Paths: stable/12/sys/netinet X-SVN-Commit-Revision: 364638 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Aug 2020 09:06:47 -0000 Author: tuexen Date: Mon Aug 24 09:06:46 2020 New Revision: 364638 URL: https://svnweb.freebsd.org/changeset/base/364638 Log: MFC r363194: Improve the error handling in generating ASCONF chunks. In case of errors, the cleanup was not consistent. Thanks to Felix Weinrank for fuzzing the userland stack and making me aware of the issue. Modified: stable/12/sys/netinet/sctp_asconf.c stable/12/sys/netinet/sctp_input.c Directory Properties: stable/12/ (props changed) Modified: stable/12/sys/netinet/sctp_asconf.c ============================================================================== --- stable/12/sys/netinet/sctp_asconf.c Mon Aug 24 09:00:57 2020 (r364637) +++ stable/12/sys/netinet/sctp_asconf.c Mon Aug 24 09:06:46 2020 (r364638) @@ -2600,14 +2600,14 @@ sctp_compose_asconf(struct sctp_tcb *stcb, int *retlen if (m_asconf_chk == NULL) { /* no mbuf's */ SCTPDBG(SCTP_DEBUG_ASCONF1, - "compose_asconf: couldn't get chunk mbuf!\n"); + "sctp_compose_asconf: couldn't get chunk mbuf!\n"); return (NULL); } m_asconf = sctp_get_mbuf_for_msg(MCLBYTES, 0, M_NOWAIT, 1, MT_DATA); if (m_asconf == NULL) { /* no mbuf's */ SCTPDBG(SCTP_DEBUG_ASCONF1, - "compose_asconf: couldn't get mbuf!\n"); + "sctp_compose_asconf: couldn't get mbuf!\n"); sctp_m_freem(m_asconf_chk); return (NULL); } @@ -2732,10 +2732,12 @@ sctp_compose_asconf(struct sctp_tcb *stcb, int *retlen break; #endif default: - p_size = 0; - addr_size = 0; - addr_ptr = NULL; - break; + SCTPDBG(SCTP_DEBUG_ASCONF1, + "sctp_compose_asconf: no usable lookup addr (family = %d)!\n", + found_addr->sa_family); + sctp_m_freem(m_asconf_chk); + sctp_m_freem(m_asconf); + return (NULL); } lookup->ph.param_length = htons(SCTP_SIZE32(p_size)); memcpy(lookup->addr, addr_ptr, addr_size); @@ -2743,12 +2745,10 @@ sctp_compose_asconf(struct sctp_tcb *stcb, int *retlen } else { /* uh oh... don't have any address?? */ SCTPDBG(SCTP_DEBUG_ASCONF1, - "compose_asconf: no lookup addr!\n"); - /* XXX for now, we send a IPv4 address of 0.0.0.0 */ - lookup->ph.param_type = htons(SCTP_IPV4_ADDRESS); - lookup->ph.param_length = htons(SCTP_SIZE32(sizeof(struct sctp_ipv4addr_param))); - memset(lookup->addr, 0, sizeof(struct in_addr)); - SCTP_BUF_LEN(m_asconf_chk) += SCTP_SIZE32(sizeof(struct sctp_ipv4addr_param)); + "sctp_compose_asconf: no lookup addr!\n"); + sctp_m_freem(m_asconf_chk); + sctp_m_freem(m_asconf); + return (NULL); } } /* chain it all together */ @@ -3274,10 +3274,9 @@ sctp_addr_mgmt_ep_sa(struct sctp_inpcb *inp, struct so } void -sctp_asconf_send_nat_state_update(struct sctp_tcb *stcb, - struct sctp_nets *net) +sctp_asconf_send_nat_state_update(struct sctp_tcb *stcb, struct sctp_nets *net) { - struct sctp_asconf_addr *aa; + struct sctp_asconf_addr *aa_vtag, *aa_add, *aa_del; struct sctp_ifa *sctp_ifap; struct sctp_asconf_tag_param *vtag; #ifdef INET @@ -3286,6 +3285,7 @@ sctp_asconf_send_nat_state_update(struct sctp_tcb *stc #ifdef INET6 struct sockaddr_in6 *to6; #endif + if (net == NULL) { SCTPDBG(SCTP_DEBUG_ASCONF1, "sctp_asconf_send_nat_state_update: Missing net\n"); return; @@ -3295,105 +3295,81 @@ sctp_asconf_send_nat_state_update(struct sctp_tcb *stc return; } /* - * Need to have in the asconf: - vtagparam(my_vtag/peer_vtag) - - * add(0.0.0.0) - del(0.0.0.0) - Any global addresses add(addr) + * Need to have in the ASCONF: - VTAG(my_vtag/peer_vtag) - + * ADD(wildcard) - DEL(wildcard) - ADD(Any global addresses) */ - SCTP_MALLOC(aa, struct sctp_asconf_addr *, sizeof(*aa), - SCTP_M_ASC_ADDR); - if (aa == NULL) { - /* didn't get memory */ - SCTPDBG(SCTP_DEBUG_ASCONF1, - "sctp_asconf_send_nat_state_update: failed to get memory!\n"); + SCTP_MALLOC(aa_vtag, struct sctp_asconf_addr *, sizeof(struct sctp_asconf_addr), SCTP_M_ASC_ADDR); + SCTP_MALLOC(aa_add, struct sctp_asconf_addr *, sizeof(struct sctp_asconf_addr), SCTP_M_ASC_ADDR); + SCTP_MALLOC(aa_del, struct sctp_asconf_addr *, sizeof(struct sctp_asconf_addr), SCTP_M_ASC_ADDR); + + if ((aa_vtag == NULL) || (aa_add == NULL) || (aa_del == NULL)) { + /* Didn't get memory */ + SCTPDBG(SCTP_DEBUG_ASCONF1, "sctp_asconf_send_nat_state_update: failed to get memory!\n"); +out: + if (aa_vtag != NULL) { + SCTP_FREE(aa_vtag, SCTP_M_ASC_ADDR); + } + if (aa_add != NULL) { + SCTP_FREE(aa_add, SCTP_M_ASC_ADDR); + } + if (aa_del != NULL) { + SCTP_FREE(aa_del, SCTP_M_ASC_ADDR); + } return; } - aa->special_del = 0; - /* fill in asconf address parameter fields */ - /* top level elements are "networked" during send */ - aa->ifa = NULL; - aa->sent = 0; /* clear sent flag */ - vtag = (struct sctp_asconf_tag_param *)&aa->ap.aph; + memset(aa_vtag, 0, sizeof(struct sctp_asconf_addr)); + aa_vtag->special_del = 0; + /* Fill in ASCONF address parameter fields. */ + /* Top level elements are "networked" during send. */ + aa_vtag->ifa = NULL; + aa_vtag->sent = 0; /* clear sent flag */ + vtag = (struct sctp_asconf_tag_param *)&aa_vtag->ap.aph; vtag->aph.ph.param_type = SCTP_NAT_VTAGS; vtag->aph.ph.param_length = sizeof(struct sctp_asconf_tag_param); vtag->local_vtag = htonl(stcb->asoc.my_vtag); vtag->remote_vtag = htonl(stcb->asoc.peer_vtag); - TAILQ_INSERT_TAIL(&stcb->asoc.asconf_queue, aa, next); - SCTP_MALLOC(aa, struct sctp_asconf_addr *, sizeof(*aa), - SCTP_M_ASC_ADDR); - if (aa == NULL) { - /* didn't get memory */ - SCTPDBG(SCTP_DEBUG_ASCONF1, - "sctp_asconf_send_nat_state_update: failed to get memory!\n"); - return; - } - memset(aa, 0, sizeof(struct sctp_asconf_addr)); - /* fill in asconf address parameter fields */ - /* ADD(0.0.0.0) */ + memset(aa_add, 0, sizeof(struct sctp_asconf_addr)); + memset(aa_del, 0, sizeof(struct sctp_asconf_addr)); switch (net->ro._l_addr.sa.sa_family) { #ifdef INET case AF_INET: - aa->ap.aph.ph.param_type = SCTP_ADD_IP_ADDRESS; - aa->ap.aph.ph.param_length = sizeof(struct sctp_asconf_addrv4_param); - aa->ap.addrp.ph.param_type = SCTP_IPV4_ADDRESS; - aa->ap.addrp.ph.param_length = sizeof(struct sctp_ipv4addr_param); - /* No need to add an address, we are using 0.0.0.0 */ - TAILQ_INSERT_TAIL(&stcb->asoc.asconf_queue, aa, next); + aa_add->ap.aph.ph.param_type = SCTP_ADD_IP_ADDRESS; + aa_add->ap.aph.ph.param_length = sizeof(struct sctp_asconf_addrv4_param); + aa_add->ap.addrp.ph.param_type = SCTP_IPV4_ADDRESS; + aa_add->ap.addrp.ph.param_length = sizeof(struct sctp_ipv4addr_param); + /* No need to fill the address, we are using 0.0.0.0 */ + aa_del->ap.aph.ph.param_type = SCTP_ADD_IP_ADDRESS; + aa_del->ap.aph.ph.param_length = sizeof(struct sctp_asconf_addrv4_param); + aa_del->ap.addrp.ph.param_type = SCTP_IPV4_ADDRESS; + aa_del->ap.addrp.ph.param_length = sizeof(struct sctp_ipv4addr_param); + /* No need to fill the address, we are using 0.0.0.0 */ break; #endif #ifdef INET6 case AF_INET6: - aa->ap.aph.ph.param_type = SCTP_ADD_IP_ADDRESS; - aa->ap.aph.ph.param_length = sizeof(struct sctp_asconf_addr_param); - aa->ap.addrp.ph.param_type = SCTP_IPV6_ADDRESS; - aa->ap.addrp.ph.param_length = sizeof(struct sctp_ipv6addr_param); - /* No need to add an address, we are using 0.0.0.0 */ - TAILQ_INSERT_TAIL(&stcb->asoc.asconf_queue, aa, next); + aa_add->ap.aph.ph.param_type = SCTP_ADD_IP_ADDRESS; + aa_add->ap.aph.ph.param_length = sizeof(struct sctp_asconf_addr_param); + aa_add->ap.addrp.ph.param_type = SCTP_IPV6_ADDRESS; + aa_add->ap.addrp.ph.param_length = sizeof(struct sctp_ipv6addr_param); + /* No need to fill the address, we are using ::0 */ + aa_del->ap.aph.ph.param_type = SCTP_DEL_IP_ADDRESS; + aa_del->ap.aph.ph.param_length = sizeof(struct sctp_asconf_addr_param); + aa_del->ap.addrp.ph.param_type = SCTP_IPV6_ADDRESS; + aa_del->ap.addrp.ph.param_length = sizeof(struct sctp_ipv6addr_param); + /* No need to fill the address, we are using ::0 */ break; #endif default: SCTPDBG(SCTP_DEBUG_ASCONF1, - "sctp_asconf_send_nat_state_update: unknown address family\n"); - SCTP_FREE(aa, SCTP_M_ASC_ADDR); - return; + "sctp_asconf_send_nat_state_update: unknown address family %d\n", + net->ro._l_addr.sa.sa_family); + goto out; } - SCTP_MALLOC(aa, struct sctp_asconf_addr *, sizeof(*aa), - SCTP_M_ASC_ADDR); - if (aa == NULL) { - /* didn't get memory */ - SCTPDBG(SCTP_DEBUG_ASCONF1, - "sctp_asconf_send_nat_state_update: failed to get memory!\n"); - return; - } - memset(aa, 0, sizeof(struct sctp_asconf_addr)); - /* fill in asconf address parameter fields */ - /* ADD(0.0.0.0) */ - switch (net->ro._l_addr.sa.sa_family) { -#ifdef INET - case AF_INET: - aa->ap.aph.ph.param_type = SCTP_ADD_IP_ADDRESS; - aa->ap.aph.ph.param_length = sizeof(struct sctp_asconf_addrv4_param); - aa->ap.addrp.ph.param_type = SCTP_IPV4_ADDRESS; - aa->ap.addrp.ph.param_length = sizeof(struct sctp_ipv4addr_param); - /* No need to add an address, we are using 0.0.0.0 */ - TAILQ_INSERT_TAIL(&stcb->asoc.asconf_queue, aa, next); - break; -#endif -#ifdef INET6 - case AF_INET6: - aa->ap.aph.ph.param_type = SCTP_DEL_IP_ADDRESS; - aa->ap.aph.ph.param_length = sizeof(struct sctp_asconf_addr_param); - aa->ap.addrp.ph.param_type = SCTP_IPV6_ADDRESS; - aa->ap.addrp.ph.param_length = sizeof(struct sctp_ipv6addr_param); - /* No need to add an address, we are using 0.0.0.0 */ - TAILQ_INSERT_TAIL(&stcb->asoc.asconf_queue, aa, next); - break; -#endif - default: - SCTPDBG(SCTP_DEBUG_ASCONF1, - "sctp_asconf_send_nat_state_update: unknown address family\n"); - SCTP_FREE(aa, SCTP_M_ASC_ADDR); - return; - } + TAILQ_INSERT_TAIL(&stcb->asoc.asconf_queue, aa_vtag, next); + TAILQ_INSERT_TAIL(&stcb->asoc.asconf_queue, aa_add, next); + TAILQ_INSERT_TAIL(&stcb->asoc.asconf_queue, aa_del, next); + /* Now we must hunt the addresses and add all global addresses */ if (stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_BOUNDALL) { struct sctp_vrf *vrf = NULL; Modified: stable/12/sys/netinet/sctp_input.c ============================================================================== --- stable/12/sys/netinet/sctp_input.c Mon Aug 24 09:00:57 2020 (r364637) +++ stable/12/sys/netinet/sctp_input.c Mon Aug 24 09:06:46 2020 (r364638) @@ -800,13 +800,13 @@ sctp_handle_abort(struct sctp_abort_chunk *abort, cause = (struct sctp_error_cause *)(abort + 1); error = ntohs(cause->code); if (error == SCTP_CAUSE_NAT_COLLIDING_STATE) { - SCTPDBG(SCTP_DEBUG_INPUT2, "Received Colliding state abort flags:%x\n", + SCTPDBG(SCTP_DEBUG_INPUT2, "Received Colliding state, ABORT flags:%x\n", abort->ch.chunk_flags); if (sctp_handle_nat_colliding_state(stcb)) { return (0); } } else if (error == SCTP_CAUSE_NAT_MISSING_STATE) { - SCTPDBG(SCTP_DEBUG_INPUT2, "Received missing state abort flags:%x\n", + SCTPDBG(SCTP_DEBUG_INPUT2, "Received missing state, ABORT flags:%x\n", abort->ch.chunk_flags); if (sctp_handle_nat_missing_state(stcb, net)) { return (0); @@ -1146,14 +1146,14 @@ sctp_handle_error(struct sctp_chunkhdr *ch, cause_code); break; case SCTP_CAUSE_NAT_COLLIDING_STATE: - SCTPDBG(SCTP_DEBUG_INPUT2, "Received Colliding state abort flags: %x\n", + SCTPDBG(SCTP_DEBUG_INPUT2, "Received Colliding state, ERROR flags: %x\n", ch->chunk_flags); if (sctp_handle_nat_colliding_state(stcb)) { return (0); } break; case SCTP_CAUSE_NAT_MISSING_STATE: - SCTPDBG(SCTP_DEBUG_INPUT2, "Received missing state abort flags: %x\n", + SCTPDBG(SCTP_DEBUG_INPUT2, "Received missing state, ERROR flags: %x\n", ch->chunk_flags); if (sctp_handle_nat_missing_state(stcb, net)) { return (0);