git: 5f2e1835054e - main - sctp: improve error handling in INIT/INIT-ACK processing

Michael Tuexen tuexen at FreeBSD.org
Sun May 2 23:24:39 UTC 2021


The branch main has been updated by tuexen:

URL: https://cgit.FreeBSD.org/src/commit/?id=5f2e1835054ee84f2e68ebc890d92716a91775b7

commit 5f2e1835054ee84f2e68ebc890d92716a91775b7
Author:     Michael Tuexen <tuexen at FreeBSD.org>
AuthorDate: 2021-05-02 20:38:27 +0000
Commit:     Michael Tuexen <tuexen at FreeBSD.org>
CommitDate: 2021-05-02 20:41:35 +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.
    
    MFC after:      3 days
---
 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,
 		    &notification, auth_skipped, auth_offset, auth_len,
 		    mflowtype, mflowid,
 		    vrf_id, port);
+		if (*stcb == NULL) {
+			*locked_tcb = NULL;
+		}
 	}
 
 	if (*stcb == NULL) {


More information about the dev-commits-src-all mailing list