From owner-dev-commits-src-all@freebsd.org Sun Jun 6 23:39:04 2021 Return-Path: Delivered-To: dev-commits-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 A0FD46416D5; Sun, 6 Jun 2021 23:39:04 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4FytHr48nDz4ldH; Sun, 6 Jun 2021 23:39:04 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 76DCA48C9; Sun, 6 Jun 2021 23:39:04 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 156Nd4tZ079344; Sun, 6 Jun 2021 23:39:04 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 156Nd4ad079343; Sun, 6 Jun 2021 23:39:04 GMT (envelope-from git) Date: Sun, 6 Jun 2021 23:39:04 GMT Message-Id: <202106062339.156Nd4ad079343@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Michael Tuexen Subject: git: 7f1b88c36e06 - stable/13 - sctp: improve error handling in INIT/INIT-ACK processing MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: tuexen X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 7f1b88c36e067695392b0e3b64aff36bef82bb45 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 06 Jun 2021 23:39:04 -0000 The branch stable/13 has been updated by tuexen: URL: https://cgit.FreeBSD.org/src/commit/?id=7f1b88c36e067695392b0e3b64aff36bef82bb45 commit 7f1b88c36e067695392b0e3b64aff36bef82bb45 Author: Michael Tuexen AuthorDate: 2021-05-02 20:38:27 +0000 Commit: Michael Tuexen CommitDate: 2021-06-02 21:48:13 +0000 sctp: improve error handling in INIT/INIT-ACK processing When processing INIT and INIT-ACK information, also during COOKIE processing, delete the current association, when it would end up in an inconsistent state. (cherry picked from commit 5f2e1835054ee84f2e68ebc890d92716a91775b7) --- sys/netinet/sctp_input.c | 101 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 72 insertions(+), 29 deletions(-) diff --git a/sys/netinet/sctp_input.c b/sys/netinet/sctp_input.c index 0790e6f4d2db..072322ea074a 100644 --- a/sys/netinet/sctp_input.c +++ b/sys/netinet/sctp_input.c @@ -449,18 +449,24 @@ sctp_process_init_ack(struct mbuf *m, int iphlen, int offset, asoc = &stcb->asoc; asoc->peer_supports_nat = (uint8_t)nat_friendly; /* process the peer's parameters in the INIT-ACK */ - retval = sctp_process_init((struct sctp_init_chunk *)cp, stcb); - if (retval < 0) { + if (sctp_process_init((struct sctp_init_chunk *)cp, stcb) < 0) { if (op_err != NULL) { sctp_m_freem(op_err); } - return (retval); + op_err = sctp_generate_cause(SCTP_CAUSE_OUT_OF_RESC, ""); + SCTPDBG(SCTP_DEBUG_INPUT1, "sctp_process_init() failed\n"); + sctp_abort_association(stcb->sctp_ep, stcb, m, iphlen, + src, dst, sh, op_err, + mflowtype, mflowid, + vrf_id, net->port); + *abort_no_unlock = 1; + return (-1); } initack_limit = offset + ntohs(cp->ch.chunk_length); /* load all addresses */ if ((retval = sctp_load_addresses_from_init(stcb, m, - (offset + sizeof(struct sctp_init_chunk)), initack_limit, - src, dst, NULL, stcb->asoc.port))) { + offset + sizeof(struct sctp_init_chunk), + initack_limit, src, dst, NULL, stcb->asoc.port)) < 0) { if (op_err != NULL) { sctp_m_freem(op_err); } @@ -1458,10 +1464,15 @@ sctp_process_cookie_existing(struct mbuf *m, int iphlen, int offset, * the right seq no's. */ /* First we must process the INIT !! */ - retval = sctp_process_init(init_cp, stcb); - if (retval < 0) { + if (sctp_process_init(init_cp, stcb) < 0) { if (how_indx < sizeof(asoc->cookie_how)) asoc->cookie_how[how_indx] = 3; + op_err = sctp_generate_cause(SCTP_CAUSE_OUT_OF_RESC, ""); + SCTPDBG(SCTP_DEBUG_INPUT1, "sctp_process_init() failed\n"); + sctp_abort_association(stcb->sctp_ep, stcb, m, iphlen, + src, dst, sh, op_err, + mflowtype, mflowid, + vrf_id, net->port); return (NULL); } /* we have already processed the INIT so no problem */ @@ -1523,16 +1534,20 @@ sctp_process_cookie_existing(struct mbuf *m, int iphlen, int offset, break; } /* end switch */ sctp_stop_all_cookie_timers(stcb); - /* - * We ignore the return code here.. not sure if we should - * somehow abort.. but we do have an existing asoc. This - * really should not fail. - */ - if (sctp_load_addresses_from_init(stcb, m, + if ((ret = sctp_load_addresses_from_init(stcb, m, init_offset + sizeof(struct sctp_init_chunk), - initack_offset, src, dst, init_src, stcb->asoc.port)) { + initack_offset, src, dst, init_src, stcb->asoc.port)) < 0) { if (how_indx < sizeof(asoc->cookie_how)) asoc->cookie_how[how_indx] = 4; + op_err = sctp_generate_cause(SCTP_BASE_SYSCTL(sctp_diag_info_code), + "Problem with address parameters"); + SCTPDBG(SCTP_DEBUG_INPUT1, + "Load addresses from INIT causes an abort %d\n", + retval); + sctp_abort_association(stcb->sctp_ep, stcb, m, iphlen, + src, dst, sh, op_err, + mflowtype, mflowid, + vrf_id, net->port); return (NULL); } /* respond with a COOKIE-ACK */ @@ -1650,17 +1665,31 @@ sctp_process_cookie_existing(struct mbuf *m, int iphlen, int offset, } } /* process the INIT info (peer's info) */ - retval = sctp_process_init(init_cp, stcb); - if (retval < 0) { + if (sctp_process_init(init_cp, stcb) < 0) { if (how_indx < sizeof(asoc->cookie_how)) asoc->cookie_how[how_indx] = 9; + op_err = sctp_generate_cause(SCTP_CAUSE_OUT_OF_RESC, ""); + SCTPDBG(SCTP_DEBUG_INPUT1, "sctp_process_init() failed\n"); + sctp_abort_association(stcb->sctp_ep, stcb, m, iphlen, + src, dst, sh, op_err, + mflowtype, mflowid, + vrf_id, net->port); return (NULL); } - if (sctp_load_addresses_from_init(stcb, m, + if ((retval = sctp_load_addresses_from_init(stcb, m, init_offset + sizeof(struct sctp_init_chunk), - initack_offset, src, dst, init_src, stcb->asoc.port)) { + initack_offset, src, dst, init_src, stcb->asoc.port)) < 0) { if (how_indx < sizeof(asoc->cookie_how)) asoc->cookie_how[how_indx] = 10; + op_err = sctp_generate_cause(SCTP_BASE_SYSCTL(sctp_diag_info_code), + "Problem with address parameters"); + SCTPDBG(SCTP_DEBUG_INPUT1, + "Load addresses from INIT causes an abort %d\n", + retval); + sctp_abort_association(stcb->sctp_ep, stcb, m, iphlen, + src, dst, sh, op_err, + mflowtype, mflowid, + vrf_id, net->port); return (NULL); } if ((SCTP_GET_STATE(stcb) == SCTP_STATE_COOKIE_WAIT) || @@ -1874,11 +1903,15 @@ sctp_process_cookie_existing(struct mbuf *m, int iphlen, int offset, asoc->total_flight = 0; asoc->total_flight_count = 0; /* process the INIT info (peer's info) */ - retval = sctp_process_init(init_cp, stcb); - if (retval < 0) { + if (sctp_process_init(init_cp, stcb) < 0) { if (how_indx < sizeof(asoc->cookie_how)) asoc->cookie_how[how_indx] = 13; - + op_err = sctp_generate_cause(SCTP_CAUSE_OUT_OF_RESC, ""); + SCTPDBG(SCTP_DEBUG_INPUT1, "sctp_process_init() failed\n"); + sctp_abort_association(stcb->sctp_ep, stcb, m, iphlen, + src, dst, sh, op_err, + mflowtype, mflowid, + vrf_id, net->port); return (NULL); } /* @@ -1887,12 +1920,20 @@ sctp_process_cookie_existing(struct mbuf *m, int iphlen, int offset, */ net->hb_responded = 1; - if (sctp_load_addresses_from_init(stcb, m, + if ((retval = sctp_load_addresses_from_init(stcb, m, init_offset + sizeof(struct sctp_init_chunk), - initack_offset, src, dst, init_src, stcb->asoc.port)) { + initack_offset, src, dst, init_src, stcb->asoc.port)) < 0) { if (how_indx < sizeof(asoc->cookie_how)) asoc->cookie_how[how_indx] = 14; - + op_err = sctp_generate_cause(SCTP_BASE_SYSCTL(sctp_diag_info_code), + "Problem with address parameters"); + SCTPDBG(SCTP_DEBUG_INPUT1, + "Load addresses from INIT causes an abort %d\n", + retval); + sctp_abort_association(stcb->sctp_ep, stcb, m, iphlen, + src, dst, sh, op_err, + mflowtype, mflowid, + vrf_id, net->port); return (NULL); } /* respond with a COOKIE-ACK */ @@ -2047,16 +2088,15 @@ sctp_process_cookie_new(struct mbuf *m, int iphlen, int offset, asoc->advanced_peer_ack_point = asoc->last_acked_seq; /* process the INIT info (peer's info) */ - retval = sctp_process_init(init_cp, stcb); - if (retval < 0) { + if (sctp_process_init(init_cp, stcb) < 0) { (void)sctp_free_assoc(inp, stcb, SCTP_NORMAL_PROC, SCTP_FROM_SCTP_INPUT + SCTP_LOC_19); return (NULL); } /* load all addresses */ - if (sctp_load_addresses_from_init(stcb, m, - init_offset + sizeof(struct sctp_init_chunk), initack_offset, - src, dst, init_src, port)) { + if ((retval = sctp_load_addresses_from_init(stcb, m, + init_offset + sizeof(struct sctp_init_chunk), + initack_offset, src, dst, init_src, port)) < 0) { (void)sctp_free_assoc(inp, stcb, SCTP_NORMAL_PROC, SCTP_FROM_SCTP_INPUT + SCTP_LOC_20); return (NULL); @@ -2518,6 +2558,9 @@ sctp_handle_cookie_echo(struct mbuf *m, int iphlen, int offset, ¬ification, auth_skipped, auth_offset, auth_len, mflowtype, mflowid, vrf_id, port); + if (*stcb == NULL) { + *locked_tcb = NULL; + } } if (*stcb == NULL) {