Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 7 Jan 2012 13:03:34 +0000 (UTC)
From:      Michael Tuexen <tuexen@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r229774 - head/sys/netinet
Message-ID:  <201201071303.q07D3Yv1040745@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: tuexen
Date: Sat Jan  7 13:03:33 2012
New Revision: 229774
URL: http://svn.freebsd.org/changeset/base/229774

Log:
  Improve the handling of received INITs. Send an ABORT when
  not accepting the connection. Also fix a crash, which
  could happen when the user closed the socket.
  
  MFC after: 1 month.

Modified:
  head/sys/netinet/sctp_input.c

Modified: head/sys/netinet/sctp_input.c
==============================================================================
--- head/sys/netinet/sctp_input.c	Sat Jan  7 12:40:45 2012	(r229773)
+++ head/sys/netinet/sctp_input.c	Sat Jan  7 13:03:33 2012	(r229774)
@@ -88,43 +88,14 @@ sctp_handle_init(struct mbuf *m, int iph
 {
 	struct sctp_init *init;
 	struct mbuf *op_err;
-	uint32_t init_limit;
 
 	SCTPDBG(SCTP_DEBUG_INPUT2, "sctp_handle_init: handling INIT tcb:%p\n",
 	    stcb);
 	if (stcb == NULL) {
 		SCTP_INP_RLOCK(inp);
-		if (inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_GONE) {
-			goto outnow;
-		}
-	}
-	op_err = NULL;
-	init = &cp->init;
-	/* First are we accepting? */
-	if ((inp->sctp_socket->so_qlimit == 0) && (stcb == NULL)) {
-		SCTPDBG(SCTP_DEBUG_INPUT2,
-		    "sctp_handle_init: Abort, so_qlimit:%d\n",
-		    inp->sctp_socket->so_qlimit);
-		/*
-		 * FIX ME ?? What about TCP model and we have a
-		 * match/restart case? Actually no fix is needed. the lookup
-		 * will always find the existing assoc so stcb would not be
-		 * NULL. It may be questionable to do this since we COULD
-		 * just send back the INIT-ACK and hope that the app did
-		 * accept()'s by the time the COOKIE was sent. But there is
-		 * a price to pay for COOKIE generation and I don't want to
-		 * pay it on the chance that the app will actually do some
-		 * accepts(). The App just looses and should NOT be in this
-		 * state :-)
-		 */
-		sctp_abort_association(inp, stcb, m, iphlen, sh, op_err,
-		    vrf_id, port);
-		if (stcb)
-			*abort_no_unlock = 1;
-		goto outnow;
 	}
+	/* validate length */
 	if (ntohs(cp->ch.chunk_length) < sizeof(struct sctp_init_chunk)) {
-		/* Invalid length */
 		op_err = sctp_generate_invmanparam(SCTP_CAUSE_INVALID_PARAM);
 		sctp_abort_association(inp, stcb, m, iphlen, sh, op_err,
 		    vrf_id, port);
@@ -133,6 +104,7 @@ sctp_handle_init(struct mbuf *m, int iph
 		goto outnow;
 	}
 	/* validate parameters */
+	init = &cp->init;
 	if (init->initiate_tag == 0) {
 		/* protocol error... send abort */
 		op_err = sctp_generate_invmanparam(SCTP_CAUSE_INVALID_PARAM);
@@ -169,19 +141,49 @@ sctp_handle_init(struct mbuf *m, int iph
 			*abort_no_unlock = 1;
 		goto outnow;
 	}
-	init_limit = offset + ntohs(cp->ch.chunk_length);
 	if (sctp_validate_init_auth_params(m, offset + sizeof(*cp),
-	    init_limit)) {
+	    offset + ntohs(cp->ch.chunk_length))) {
 		/* auth parameter(s) error... send abort */
 		sctp_abort_association(inp, stcb, m, iphlen, sh, NULL, vrf_id, port);
 		if (stcb)
 			*abort_no_unlock = 1;
 		goto outnow;
 	}
-	/* send an INIT-ACK w/cookie */
-	SCTPDBG(SCTP_DEBUG_INPUT3, "sctp_handle_init: sending INIT-ACK\n");
-	sctp_send_initiate_ack(inp, stcb, m, iphlen, offset, sh, cp, vrf_id, port,
-	    ((stcb == NULL) ? SCTP_HOLDS_LOCK : SCTP_NOT_LOCKED));
+	/*
+	 * We are only accepting if we have a socket with positive
+	 * so_qlimit.
+	 */
+	if ((stcb == NULL) &&
+	    ((inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_GONE) ||
+	    (inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_ALLGONE) ||
+	    (inp->sctp_socket == NULL) ||
+	    (inp->sctp_socket->so_qlimit == 0))) {
+		/*
+		 * FIX ME ?? What about TCP model and we have a
+		 * match/restart case? Actually no fix is needed. the lookup
+		 * will always find the existing assoc so stcb would not be
+		 * NULL. It may be questionable to do this since we COULD
+		 * just send back the INIT-ACK and hope that the app did
+		 * accept()'s by the time the COOKIE was sent. But there is
+		 * a price to pay for COOKIE generation and I don't want to
+		 * pay it on the chance that the app will actually do some
+		 * accepts(). The App just looses and should NOT be in this
+		 * state :-)
+		 */
+		sctp_abort_association(inp, stcb, m, iphlen, sh, NULL,
+		    vrf_id, port);
+		goto outnow;
+	}
+	if ((stcb != NULL) &&
+	    (SCTP_GET_STATE(&stcb->asoc) == SCTP_STATE_SHUTDOWN_ACK_SENT)) {
+		SCTPDBG(SCTP_DEBUG_INPUT3, "sctp_handle_init: sending SHUTDOWN-ACK\n");
+		sctp_send_shutdown_ack(stcb, NULL);
+		sctp_chunk_output(inp, stcb, SCTP_OUTPUT_FROM_CONTROL_PROC, SCTP_SO_NOT_LOCKED);
+	} else {
+		SCTPDBG(SCTP_DEBUG_INPUT3, "sctp_handle_init: sending INIT-ACK\n");
+		sctp_send_initiate_ack(inp, stcb, m, iphlen, offset, sh, cp, vrf_id, port,
+		    ((stcb == NULL) ? SCTP_HOLDS_LOCK : SCTP_NOT_LOCKED));
+	}
 outnow:
 	if (stcb == NULL) {
 		SCTP_INP_RUNLOCK(inp);
@@ -1025,7 +1027,7 @@ sctp_handle_shutdown_ack(struct sctp_shu
 		if ((stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_TCPTYPE) ||
 		    (stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_IN_TCPPOOL)) {
 			/* Set the connected flag to disconnected */
-			stcb->sctp_ep->sctp_socket->so_snd.sb_cc = 0;
+			stcb->sctp_socket->so_snd.sb_cc = 0;
 		}
 	}
 	SCTP_STAT_INCR_COUNTER32(sctps_shutdown);
@@ -4499,7 +4501,6 @@ __attribute__((noinline))
 	 * process all control chunks...
 	 */
 	if (((ch->chunk_type == SCTP_SELECTIVE_ACK) ||
-	/* EY */
 	    (ch->chunk_type == SCTP_NR_SELECTIVE_ACK) ||
 	    (ch->chunk_type == SCTP_HEARTBEAT_REQUEST)) &&
 	    (SCTP_GET_STATE(&stcb->asoc) == SCTP_STATE_COOKIE_ECHOED)) {
@@ -4613,54 +4614,30 @@ process_control_chunks:
 		}
 		switch (ch->chunk_type) {
 		case SCTP_INITIATION:
-			/* must be first and only chunk */
 			SCTPDBG(SCTP_DEBUG_INPUT3, "SCTP_INIT\n");
-			if (inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_GONE) {
-				/* We are not interested anymore? */
-				if ((stcb) && (stcb->asoc.total_output_queue_size)) {
-					/*
-					 * collision case where we are
-					 * sending to them too
-					 */
-					;
-				} else {
-					if (locked_tcb) {
-						SCTP_TCB_UNLOCK(locked_tcb);
-					}
-					*offset = length;
-					return (NULL);
-				}
-			}
-			if ((chk_length > SCTP_LARGEST_INIT_ACCEPTED) ||
-			    (num_chunks > 1) ||
+			/* The INIT chunk must be the only chunk. */
+			if ((num_chunks > 1) ||
 			    (SCTP_BASE_SYSCTL(sctp_strict_init) && (length - *offset > (int)SCTP_SIZE32(chk_length)))) {
+				sctp_abort_association(inp, stcb, m,
+				    iphlen, sh, NULL, vrf_id, port);
 				*offset = length;
-				if (locked_tcb) {
-					SCTP_TCB_UNLOCK(locked_tcb);
-				}
 				return (NULL);
 			}
-			if ((stcb != NULL) &&
-			    (SCTP_GET_STATE(&stcb->asoc) ==
-			    SCTP_STATE_SHUTDOWN_ACK_SENT)) {
-				sctp_send_shutdown_ack(stcb, NULL);
+			/* Honor our resource limit. */
+			if (chk_length > SCTP_LARGEST_INIT_ACCEPTED) {
+				struct mbuf *op_err;
+
+				op_err = sctp_generate_invmanparam(SCTP_CAUSE_OUT_OF_RESC);
+				sctp_abort_association(inp, stcb, m,
+				    iphlen, sh, op_err, vrf_id, port);
 				*offset = length;
-				sctp_chunk_output(inp, stcb, SCTP_OUTPUT_FROM_CONTROL_PROC, SCTP_SO_NOT_LOCKED);
-				if (locked_tcb) {
-					SCTP_TCB_UNLOCK(locked_tcb);
-				}
 				return (NULL);
 			}
-			if (netp) {
-				sctp_handle_init(m, iphlen, *offset, sh,
-				    (struct sctp_init_chunk *)ch, inp,
-				    stcb, &abort_no_unlock, vrf_id, port);
-			}
-			if (abort_no_unlock)
-				return (NULL);
-
+			sctp_handle_init(m, iphlen, *offset, sh,
+			    (struct sctp_init_chunk *)ch, inp,
+			    stcb, &abort_no_unlock, vrf_id, port);
 			*offset = length;
-			if (locked_tcb) {
+			if ((!abort_no_unlock) && (locked_tcb)) {
 				SCTP_TCB_UNLOCK(locked_tcb);
 			}
 			return (NULL);
@@ -4668,7 +4645,6 @@ process_control_chunks:
 		case SCTP_PAD_CHUNK:
 			break;
 		case SCTP_INITIATION_ACK:
-			/* must be first and only chunk */
 			SCTPDBG(SCTP_DEBUG_INPUT3, "SCTP_INIT-ACK\n");
 			if (inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_GONE) {
 				/* We are not interested anymore */
@@ -4697,6 +4673,7 @@ process_control_chunks:
 					return (NULL);
 				}
 			}
+			/* The INIT-CK chunk must be the only chunk. */
 			if ((num_chunks > 1) ||
 			    (SCTP_BASE_SYSCTL(sctp_strict_init) && (length - *offset > (int)SCTP_SIZE32(chk_length)))) {
 				*offset = length;
@@ -4711,16 +4688,17 @@ process_control_chunks:
 			} else {
 				ret = -1;
 			}
+			*offset = length;
+			if (abort_no_unlock) {
+				return (NULL);
+			}
 			/*
 			 * Special case, I must call the output routine to
 			 * get the cookie echoed
 			 */
-			if (abort_no_unlock)
-				return (NULL);
-
-			if ((stcb) && ret == 0)
+			if ((stcb != NULL) && (ret == 0)) {
 				sctp_chunk_output(stcb->sctp_ep, stcb, SCTP_OUTPUT_FROM_CONTROL_PROC, SCTP_SO_NOT_LOCKED);
-			*offset = length;
+			}
 			if (locked_tcb) {
 				SCTP_TCB_UNLOCK(locked_tcb);
 			}
@@ -4977,7 +4955,6 @@ process_control_chunks:
 		case SCTP_OPERATION_ERROR:
 			SCTPDBG(SCTP_DEBUG_INPUT3, "SCTP_OP-ERR\n");
 			if ((stcb) && netp && *netp && sctp_handle_error(ch, stcb, *netp) < 0) {
-
 				*offset = length;
 				return (NULL);
 			}
@@ -5009,23 +4986,11 @@ process_control_chunks:
 			if ((stcb == NULL) && (inp->sctp_socket->so_qlen >= inp->sctp_socket->so_qlimit)) {
 				if ((inp->sctp_flags & SCTP_PCB_FLAGS_TCPTYPE) &&
 				    (SCTP_BASE_SYSCTL(sctp_abort_if_one_2_one_hits_limit))) {
-					struct mbuf *oper;
-					struct sctp_paramhdr *phdr;
+					struct mbuf *op_err;
 
-					oper = sctp_get_mbuf_for_msg(sizeof(struct sctp_paramhdr),
-					    0, M_DONTWAIT, 1, MT_DATA);
-					if (oper) {
-						SCTP_BUF_LEN(oper) =
-						    sizeof(struct sctp_paramhdr);
-						phdr = mtod(oper,
-						    struct sctp_paramhdr *);
-						phdr->param_type =
-						    htons(SCTP_CAUSE_OUT_OF_RESC);
-						phdr->param_length =
-						    htons(sizeof(struct sctp_paramhdr));
-					}
+					op_err = sctp_generate_invmanparam(SCTP_CAUSE_OUT_OF_RESC);
 					sctp_abort_association(inp, stcb, m,
-					    iphlen, sh, oper, vrf_id, port);
+					    iphlen, sh, op_err, vrf_id, port);
 				}
 				*offset = length;
 				return (NULL);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201201071303.q07D3Yv1040745>