Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 1 Feb 2023 22:23:18 GMT
From:      Michael Tuexen <tuexen@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: bbd822b52f6b - stable/13 - sctp: get rid of stcb send lock
Message-ID:  <202302012223.311MNIQS071246@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by tuexen:

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

commit bbd822b52f6b72da09a445624931d9226d97a385
Author:     Michael Tuexen <tuexen@FreeBSD.org>
AuthorDate: 2022-03-28 23:50:17 +0000
Commit:     Michael Tuexen <tuexen@FreeBSD.org>
CommitDate: 2023-02-01 22:22:59 +0000

    sctp: get rid of stcb send lock
    
    Just use the stcb lock instead to simplify locking.
    
    Reported by:    syzbot+d00b202063150f85b110@syzkaller.appspotmail.com
    Reported by:    syzbot+87f268a0a6d2d6383306@syzkaller.appspotmail.com
    
    (cherry picked from commit 5ac91821f5d7dd701752ba76041720d240a507c5)
---
 sys/netinet/sctp_input.c        |  11 +-
 sys/netinet/sctp_lock_bsd.h     |  22 --
 sys/netinet/sctp_output.c       | 556 ++++++++++++++++++----------------------
 sys/netinet/sctp_pcb.c          |  12 +-
 sys/netinet/sctp_ss_functions.c |  70 +++--
 sys/netinet/sctp_sysctl.c       |  46 ++--
 sys/netinet/sctp_timer.c        |   4 +-
 sys/netinet/sctp_uio.h          |   2 +-
 sys/netinet/sctp_usrreq.c       |   4 -
 sys/netinet/sctputil.c          |  12 +-
 10 files changed, 333 insertions(+), 406 deletions(-)

diff --git a/sys/netinet/sctp_input.c b/sys/netinet/sctp_input.c
index c86e369e2b92..13e41748a82e 100644
--- a/sys/netinet/sctp_input.c
+++ b/sys/netinet/sctp_input.c
@@ -179,6 +179,8 @@ sctp_is_there_unsent_data(struct sctp_tcb *stcb, int so_locked)
 	struct sctp_stream_queue_pending *sp;
 	struct sctp_association *asoc;
 
+	SCTP_TCB_LOCK_ASSERT(stcb);
+
 	/*
 	 * This function returns if any stream has true unsent data on it.
 	 * Note that as it looks through it will clean up any places that
@@ -186,7 +188,6 @@ sctp_is_there_unsent_data(struct sctp_tcb *stcb, int so_locked)
 	 */
 	asoc = &stcb->asoc;
 	unsent_data = 0;
-	SCTP_TCB_SEND_LOCK(stcb);
 	if (!stcb->asoc.ss_functions.sctp_ss_is_empty(stcb, asoc)) {
 		/* Check to see if some data queued */
 		for (i = 0; i < stcb->asoc.streamoutcnt; i++) {
@@ -234,7 +235,6 @@ sctp_is_there_unsent_data(struct sctp_tcb *stcb, int so_locked)
 			}
 		}
 	}
-	SCTP_TCB_SEND_UNLOCK(stcb);
 	return (unsent_data);
 }
 
@@ -246,6 +246,8 @@ sctp_process_init(struct sctp_init_chunk *cp, struct sctp_tcb *stcb)
 	struct sctp_nets *lnet;
 	unsigned int i;
 
+	SCTP_TCB_LOCK_ASSERT(stcb);
+
 	init = &cp->init;
 	asoc = &stcb->asoc;
 	/* save off parameters */
@@ -263,7 +265,6 @@ sctp_process_init(struct sctp_init_chunk *cp, struct sctp_tcb *stcb)
 			}
 		}
 	}
-	SCTP_TCB_SEND_LOCK(stcb);
 	if (asoc->pre_open_streams > ntohs(init->num_inbound_streams)) {
 		unsigned int newcnt;
 		struct sctp_stream_out *outs;
@@ -323,7 +324,6 @@ sctp_process_init(struct sctp_init_chunk *cp, struct sctp_tcb *stcb)
 		/* cut back the count */
 		asoc->pre_open_streams = newcnt;
 	}
-	SCTP_TCB_SEND_UNLOCK(stcb);
 	asoc->streamoutcnt = asoc->pre_open_streams;
 	if (asoc->strmout) {
 		for (i = 0; i < asoc->streamoutcnt; i++) {
@@ -1808,8 +1808,6 @@ sctp_process_cookie_existing(struct mbuf *m, int iphlen, int offset,
 		SCTP_TCB_LOCK(stcb);
 		atomic_subtract_int(&stcb->asoc.refcnt, 1);
 		/* send up all the data */
-		SCTP_TCB_SEND_LOCK(stcb);
-
 		sctp_report_all_outbound(stcb, 0, SCTP_SO_LOCKED);
 		for (i = 0; i < stcb->asoc.streamoutcnt; i++) {
 			stcb->asoc.strmout[i].chunks_on_queues = 0;
@@ -1896,7 +1894,6 @@ sctp_process_cookie_existing(struct mbuf *m, int iphlen, int offset,
 		 */
 		LIST_INSERT_HEAD(head, stcb, sctp_asocs);
 
-		SCTP_TCB_SEND_UNLOCK(stcb);
 		SCTP_INP_WUNLOCK(stcb->sctp_ep);
 		SCTP_INP_INFO_WUNLOCK();
 		asoc->total_flight = 0;
diff --git a/sys/netinet/sctp_lock_bsd.h b/sys/netinet/sctp_lock_bsd.h
index cd20a730e5b8..2087db337a37 100644
--- a/sys/netinet/sctp_lock_bsd.h
+++ b/sys/netinet/sctp_lock_bsd.h
@@ -337,28 +337,6 @@ __FBSDID("$FreeBSD$");
 #define SCTP_ASOC_CREATE_LOCK_CONTENDED(_inp)				\
 	((_inp)->inp_create_mtx.mtx_lock & MTX_CONTESTED)
 
-#define SCTP_TCB_SEND_LOCK_INIT(_tcb) do {				\
-	mtx_init(&(_tcb)->tcb_send_mtx, "sctp-send-tcb", "tcbs",	\
-	         MTX_DEF | MTX_DUPOK);					\
-} while (0)
-
-#define SCTP_TCB_SEND_LOCK_DESTROY(_tcb) do {				\
-	mtx_destroy(&(_tcb)->tcb_send_mtx);				\
-} while (0)
-
-#define SCTP_TCB_SEND_LOCK(_tcb) do {					\
-	mtx_lock(&(_tcb)->tcb_send_mtx);				\
-} while (0)
-
-#define SCTP_TCB_SEND_UNLOCK(_tcb) do {					\
-	mtx_unlock(&(_tcb)->tcb_send_mtx);				\
-} while (0)
-
-#define SCTP_TCB_SEND_LOCK_ASSERT(_tcb) do {				\
-	KASSERT(mtx_owned(&(_tcb)->tcb_send_mtx),			\
-	        ("Don't own TCB send lock"));				\
-} while (0)
-
 /*
  * For the majority of things (once we have found the association) we will
  * lock the actual association mutex. This will protect all the assoiciation
diff --git a/sys/netinet/sctp_output.c b/sys/netinet/sctp_output.c
index 348ad5f83bd3..c8e1cc4e3bb2 100644
--- a/sys/netinet/sctp_output.c
+++ b/sys/netinet/sctp_output.c
@@ -6318,13 +6318,15 @@ static int
 sctp_msg_append(struct sctp_tcb *stcb,
     struct sctp_nets *net,
     struct mbuf *m,
-    struct sctp_sndrcvinfo *srcv, int hold_stcb_lock)
+    struct sctp_sndrcvinfo *srcv)
 {
 	int error = 0;
 	struct mbuf *at;
 	struct sctp_stream_queue_pending *sp = NULL;
 	struct sctp_stream_out *strm;
 
+	SCTP_TCB_LOCK_ASSERT(stcb);
+
 	/*
 	 * Given an mbuf chain, put it into the association send queue and
 	 * place it on the wheel
@@ -6396,18 +6398,12 @@ sctp_msg_append(struct sctp_tcb *stcb,
 		sctp_auth_key_acquire(stcb, sp->auth_keyid);
 		sp->holds_key_ref = 1;
 	}
-	if (hold_stcb_lock == 0) {
-		SCTP_TCB_SEND_LOCK(stcb);
-	}
 	strm = &stcb->asoc.strmout[srcv->sinfo_stream];
 	sctp_snd_sb_alloc(stcb, sp->length);
 	atomic_add_int(&stcb->asoc.stream_queue_cnt, 1);
 	TAILQ_INSERT_TAIL(&strm->outqueue, sp, next);
 	stcb->asoc.ss_functions.sctp_ss_add_to_stream(stcb, &stcb->asoc, strm, sp);
 	m = NULL;
-	if (hold_stcb_lock == 0) {
-		SCTP_TCB_SEND_UNLOCK(stcb);
-	}
 out_now:
 	if (m) {
 		sctp_m_freem(m);
@@ -6656,9 +6652,8 @@ sctp_sendall_iterator(struct sctp_inpcb *inp, struct sctp_tcb *stcb, void *ptr,
 		atomic_subtract_int(&stcb->asoc.refcnt, 1);
 		goto no_chunk_output;
 	} else {
-		if (m) {
-			ret = sctp_msg_append(stcb, net, m,
-			    &ca->sndrcv, 1);
+		if (m != NULL) {
+			ret = sctp_msg_append(stcb, net, m, &ca->sndrcv);
 		}
 		asoc = &stcb->asoc;
 		if (ca->sndrcv.sinfo_flags & SCTP_EOF) {
@@ -7166,7 +7161,6 @@ sctp_move_to_outqueue(struct sctp_tcb *stcb,
 	int leading;
 	uint8_t rcv_flags = 0;
 	uint8_t some_taken;
-	uint8_t send_lock_up = 0;
 
 	SCTP_TCB_LOCK_ASSERT(stcb);
 	asoc = &stcb->asoc;
@@ -7174,10 +7168,6 @@ one_more_time:
 	/* sa_ignore FREED_MEMORY */
 	sp = TAILQ_FIRST(&strq->outqueue);
 	if (sp == NULL) {
-		if (send_lock_up == 0) {
-			SCTP_TCB_SEND_LOCK(stcb);
-			send_lock_up = 1;
-		}
 		sp = TAILQ_FIRST(&strq->outqueue);
 		if (sp) {
 			goto one_more_time;
@@ -7191,10 +7181,6 @@ one_more_time:
 			strq->last_msg_incomplete = 0;
 		}
 		to_move = 0;
-		if (send_lock_up) {
-			SCTP_TCB_SEND_UNLOCK(stcb);
-			send_lock_up = 0;
-		}
 		goto out_of;
 	}
 	if ((sp->msg_is_complete) && (sp->length == 0)) {
@@ -7206,16 +7192,11 @@ one_more_time:
 			 */
 			if ((sp->put_last_out == 0) && (sp->discard_rest == 0)) {
 				SCTP_PRINTF("Gak, put out entire msg with NO end!-1\n");
-				SCTP_PRINTF("sender_done:%d len:%d msg_comp:%d put_last_out:%d send_lock:%d\n",
+				SCTP_PRINTF("sender_done:%d len:%d msg_comp:%d put_last_out:%d\n",
 				    sp->sender_all_done,
 				    sp->length,
 				    sp->msg_is_complete,
-				    sp->put_last_out,
-				    send_lock_up);
-			}
-			if (send_lock_up == 0) {
-				SCTP_TCB_SEND_LOCK(stcb);
-				send_lock_up = 1;
+				    sp->put_last_out);
 			}
 			atomic_subtract_int(&asoc->stream_queue_cnt, 1);
 			TAILQ_REMOVE(&strq->outqueue, sp, next);
@@ -7234,11 +7215,6 @@ one_more_time:
 				sp->data = NULL;
 			}
 			sctp_free_a_strmoq(stcb, sp, so_locked);
-			/* we can't be locked to it */
-			if (send_lock_up) {
-				SCTP_TCB_SEND_UNLOCK(stcb);
-				send_lock_up = 0;
-			}
 			/* back to get the next msg */
 			goto one_more_time;
 		} else {
@@ -7258,10 +7234,6 @@ one_more_time:
 			to_move = 0;
 			goto out_of;
 		} else if (sp->discard_rest) {
-			if (send_lock_up == 0) {
-				SCTP_TCB_SEND_LOCK(stcb);
-				send_lock_up = 1;
-			}
 			/* Whack down the size */
 			atomic_subtract_int(&stcb->asoc.total_output_queue_size, sp->length);
 			if ((stcb->sctp_socket != NULL) &&
@@ -7282,7 +7254,6 @@ one_more_time:
 		}
 	}
 	some_taken = sp->some_taken;
-re_look:
 	length = sp->length;
 	if (sp->msg_is_complete) {
 		/* The message is complete */
@@ -7307,31 +7278,9 @@ re_look:
 		}
 	} else {
 		to_move = sctp_can_we_split_this(stcb, length, space_left, frag_point, eeor_mode);
-		if (to_move) {
-			/*-
-			 * We use a snapshot of length in case it
-			 * is expanding during the compare.
-			 */
-			uint32_t llen;
-
-			llen = length;
-			if (to_move >= llen) {
-				to_move = llen;
-				if (send_lock_up == 0) {
-					/*-
-					 * We are taking all of an incomplete msg
-					 * thus we need a send lock.
-					 */
-					SCTP_TCB_SEND_LOCK(stcb);
-					send_lock_up = 1;
-					if (sp->msg_is_complete) {
-						/*
-						 * the sender finished the
-						 * msg
-						 */
-						goto re_look;
-					}
-				}
+		if (to_move > 0) {
+			if (to_move >= length) {
+				to_move = length;
 			}
 			if (sp->some_taken == 0) {
 				rcv_flags |= SCTP_DATA_FIRST_FRAG;
@@ -7370,10 +7319,6 @@ re_look:
 
 	if (to_move >= length) {
 		/* we think we can steal the whole thing */
-		if ((sp->sender_all_done == 0) && (send_lock_up == 0)) {
-			SCTP_TCB_SEND_LOCK(stcb);
-			send_lock_up = 1;
-		}
 		if (to_move < sp->length) {
 			/* bail, it changed */
 			goto dont_do_it;
@@ -7467,10 +7412,6 @@ dont_do_it:
 			 * all the data if there is no leading space, so we
 			 * must put the data back and restore.
 			 */
-			if (send_lock_up == 0) {
-				SCTP_TCB_SEND_LOCK(stcb);
-				send_lock_up = 1;
-			}
 			if (sp->data == NULL) {
 				/* unsteal the data */
 				sp->data = chk->data;
@@ -7579,8 +7520,8 @@ dont_do_it:
 	 * previous loop prior to padding.
 	 */
 
-#ifdef SCTP_ASOCLOG_OF_TSNS
 	SCTP_TCB_LOCK_ASSERT(stcb);
+#ifdef SCTP_ASOCLOG_OF_TSNS
 	if (asoc->tsn_out_at >= SCTP_TSN_LOG_SIZE) {
 		asoc->tsn_out_at = 0;
 		asoc->tsn_out_wrapped = 1;
@@ -7638,16 +7579,11 @@ dont_do_it:
 		/* All done pull and kill the message */
 		if (sp->put_last_out == 0) {
 			SCTP_PRINTF("Gak, put out entire msg with NO end!-2\n");
-			SCTP_PRINTF("sender_done:%d len:%d msg_comp:%d put_last_out:%d send_lock:%d\n",
+			SCTP_PRINTF("sender_done:%d len:%d msg_comp:%d put_last_out:%d\n",
 			    sp->sender_all_done,
 			    sp->length,
 			    sp->msg_is_complete,
-			    sp->put_last_out,
-			    send_lock_up);
-		}
-		if (send_lock_up == 0) {
-			SCTP_TCB_SEND_LOCK(stcb);
-			send_lock_up = 1;
+			    sp->put_last_out);
 		}
 		atomic_subtract_int(&asoc->stream_queue_cnt, 1);
 		TAILQ_REMOVE(&strq->outqueue, sp, next);
@@ -7672,9 +7608,6 @@ dont_do_it:
 	TAILQ_INSERT_TAIL(&asoc->send_queue, chk, sctp_next);
 	asoc->send_queue_cnt++;
 out_of:
-	if (send_lock_up) {
-		SCTP_TCB_SEND_UNLOCK(stcb);
-	}
 	return (to_move);
 }
 
@@ -12079,6 +12012,8 @@ sctp_send_str_reset_req(struct sctp_tcb *stcb,
 	int can_send_out_req = 0;
 	uint32_t seq;
 
+	SCTP_TCB_LOCK_ASSERT(stcb);
+
 	asoc = &stcb->asoc;
 	if (asoc->stream_reset_outstanding) {
 		/*-
@@ -12182,7 +12117,6 @@ sctp_send_str_reset_req(struct sctp_tcb *stcb,
 		 * Ok now we proceed with copying the old out stuff and
 		 * initializing the new stuff.
 		 */
-		SCTP_TCB_SEND_LOCK(stcb);
 		stcb->asoc.ss_functions.sctp_ss_clear(stcb, &stcb->asoc, false);
 		for (i = 0; i < stcb->asoc.streamoutcnt; i++) {
 			TAILQ_INIT(&stcb->asoc.strmout[i].outqueue);
@@ -12236,7 +12170,6 @@ sctp_send_str_reset_req(struct sctp_tcb *stcb,
 		}
 		stcb->asoc.strm_realoutsize = stcb->asoc.streamoutcnt + adding_o;
 		SCTP_FREE(oldstream, SCTP_M_STRMO);
-		SCTP_TCB_SEND_UNLOCK(stcb);
 	}
 skip_stuff:
 	if ((add_stream & 1) && (adding_o > 0)) {
@@ -12487,7 +12420,7 @@ int
 sctp_lower_sosend(struct socket *so,
     struct sockaddr *addr,
     struct uio *uio,
-    struct mbuf *i_pak,
+    struct mbuf *top,
     struct mbuf *control,
     int flags,
     struct sctp_sndrcvinfo *srcv
@@ -12498,7 +12431,6 @@ sctp_lower_sosend(struct socket *so,
 	struct epoch_tracker et;
 	ssize_t sndlen = 0, max_len, local_add_more;
 	int error;
-	struct mbuf *top = NULL;
 	int queue_only = 0, queue_only_for_init = 0;
 	bool free_cnt_applied = false;
 	int un_sent;
@@ -12514,10 +12446,10 @@ sctp_lower_sosend(struct socket *so,
 	int user_marks_eor;
 	bool create_lock_applied = false;
 	int nagle_applies = 0;
-	int some_on_control = 0;
-	int got_all_of_the_send = 0;
+	bool some_on_control;
+	bool got_all_of_the_send = false;
 	bool hold_tcblock = false;
-	int non_blocking = 0;
+	bool non_blocking = false;
 	ssize_t local_soresv = 0;
 	uint16_t port;
 	uint16_t sinfo_flags;
@@ -12526,47 +12458,35 @@ sctp_lower_sosend(struct socket *so,
 	error = 0;
 	net = NULL;
 	stcb = NULL;
-	asoc = NULL;
 
 	t_inp = inp = (struct sctp_inpcb *)so->so_pcb;
 	if (inp == NULL) {
-		SCTP_LTRACE_ERR_RET(NULL, NULL, NULL, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 		error = EINVAL;
-		if (i_pak != NULL) {
-			SCTP_RELEASE_PKT(i_pak);
-		}
-		return (error);
+		goto out_unlocked;
 	}
-	if ((uio == NULL) && (i_pak == NULL)) {
-		SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
-		return (EINVAL);
+	if ((uio == NULL) && (top == NULL)) {
+		error = EINVAL;
+		goto out_unlocked;
 	}
 	user_marks_eor = sctp_is_feature_on(inp, SCTP_PCB_FLAGS_EXPLICIT_EOR);
 	atomic_add_int(&inp->total_sends, 1);
 	if (uio != NULL) {
 		if (uio->uio_resid < 0) {
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
-			return (EINVAL);
+			error = EINVAL;
+			goto out_unlocked;
 		}
 		sndlen = uio->uio_resid;
 	} else {
-		top = SCTP_HEADER_TO_CHAIN(i_pak);
-		sndlen = SCTP_HEADER_LEN(i_pak);
+		sndlen = SCTP_HEADER_LEN(top);
 	}
 	SCTPDBG(SCTP_DEBUG_OUTPUT1, "Send called addr:%p send length %zd\n",
-	    (void *)addr,
-	    sndlen);
+	    (void *)addr, sndlen);
 	if ((inp->sctp_flags & SCTP_PCB_FLAGS_TCPTYPE) &&
 	    SCTP_IS_LISTENING(inp)) {
-		/* The listener can NOT send */
-		SCTP_LTRACE_ERR_RET(NULL, NULL, NULL, SCTP_FROM_SCTP_OUTPUT, ENOTCONN);
-		error = ENOTCONN;
+		/* The listener can NOT send. */
+		error = EINVAL;
 		goto out_unlocked;
 	}
-	/**
-	 * Pre-screen address, if one is given the sin-len
-	 * must be set correctly!
-	 */
 	if (addr != NULL) {
 		union sctp_sockstore *raddr = (union sctp_sockstore *)addr;
 
@@ -12574,7 +12494,6 @@ sctp_lower_sosend(struct socket *so,
 #ifdef INET
 		case AF_INET:
 			if (raddr->sin.sin_len != sizeof(struct sockaddr_in)) {
-				SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 				error = EINVAL;
 				goto out_unlocked;
 			}
@@ -12584,7 +12503,6 @@ sctp_lower_sosend(struct socket *so,
 #ifdef INET6
 		case AF_INET6:
 			if (raddr->sin6.sin6_len != sizeof(struct sockaddr_in6)) {
-				SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 				error = EINVAL;
 				goto out_unlocked;
 			}
@@ -12592,7 +12510,6 @@ sctp_lower_sosend(struct socket *so,
 			break;
 #endif
 		default:
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EAFNOSUPPORT);
 			error = EAFNOSUPPORT;
 			goto out_unlocked;
 		}
@@ -12605,7 +12522,6 @@ sctp_lower_sosend(struct socket *so,
 		sinfo_assoc_id = srcv->sinfo_assoc_id;
 		if (INVALID_SINFO_FLAG(sinfo_flags) ||
 		    PR_SCTP_INVALID_POLICY(sinfo_flags)) {
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 			error = EINVAL;
 			goto out_unlocked;
 		}
@@ -12622,33 +12538,27 @@ sctp_lower_sosend(struct socket *so,
 		sinfo_flags |= SCTP_EOF;
 	}
 	if (sinfo_flags & SCTP_SENDALL) {
-		/* its a sendall */
 		error = sctp_sendall(inp, uio, top, srcv);
 		top = NULL;
 		goto out_unlocked;
 	}
 	if ((sinfo_flags & SCTP_ADDR_OVER) && (addr == NULL)) {
-		SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 		error = EINVAL;
 		goto out_unlocked;
 	}
-	/* now we must find the assoc */
+	/* Now we must find the association. */
+	SCTP_INP_RLOCK(inp);
 	if ((inp->sctp_flags & SCTP_PCB_FLAGS_CONNECTED) ||
 	    (inp->sctp_flags & SCTP_PCB_FLAGS_IN_TCPPOOL)) {
-		SCTP_INP_RLOCK(inp);
 		stcb = LIST_FIRST(&inp->sctp_asoc_list);
 		if (stcb != NULL) {
 			SCTP_TCB_LOCK(stcb);
 			hold_tcblock = true;
-			if (stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) {
-				SCTP_INP_RUNLOCK(inp);
-				error = ENOTCONN;
-				goto out_unlocked;
-			}
 		}
 		SCTP_INP_RUNLOCK(inp);
 	} else if (sinfo_assoc_id > SCTP_ALL_ASSOC) {
-		stcb = sctp_findassociation_ep_asocid(inp, sinfo_assoc_id, 1);
+		stcb = sctp_findasoc_ep_asocid_locked(inp, sinfo_assoc_id, 1);
+		SCTP_INP_RUNLOCK(inp);
 		if (stcb != NULL) {
 			SCTP_TCB_LOCK_ASSERT(stcb);
 			hold_tcblock = true;
@@ -12659,9 +12569,8 @@ sctp_lower_sosend(struct socket *so,
 		 * increment it, and if we don't find a tcb
 		 * decrement it.
 		 */
-		SCTP_INP_WLOCK(inp);
 		SCTP_INP_INCR_REF(inp);
-		SCTP_INP_WUNLOCK(inp);
+		SCTP_INP_RUNLOCK(inp);
 		stcb = sctp_findassociation_ep_addr(&t_inp, addr, &net, NULL, NULL);
 		if (stcb == NULL) {
 			SCTP_INP_WLOCK(inp);
@@ -12671,7 +12580,10 @@ sctp_lower_sosend(struct socket *so,
 			SCTP_TCB_LOCK_ASSERT(stcb);
 			hold_tcblock = true;
 		}
+	} else {
+		SCTP_INP_RUNLOCK(inp);
 	}
+
 #ifdef INVARIANTS
 	if (stcb != NULL) {
 		SCTP_TCB_LOCK_ASSERT(stcb);
@@ -12686,14 +12598,11 @@ sctp_lower_sosend(struct socket *so,
 		create_lock_applied = true;
 		if ((inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_GONE) ||
 		    (inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_ALLGONE)) {
-			/* Should I really unlock ? */
-			SCTP_LTRACE_ERR_RET(NULL, NULL, NULL, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 			error = EINVAL;
 			goto out_unlocked;
 		}
 		if (((inp->sctp_flags & SCTP_PCB_FLAGS_BOUND_V6) == 0) &&
 		    (addr->sa_family == AF_INET6)) {
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 			error = EINVAL;
 			goto out_unlocked;
 		}
@@ -12717,18 +12626,16 @@ sctp_lower_sosend(struct socket *so,
 			SCTP_ASOC_CREATE_UNLOCK(inp);
 			create_lock_applied = false;
 		}
-		if (error) {
+		if (error != 0) {
 			goto out_unlocked;
 		}
 		if (t_inp != inp) {
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, ENOTCONN);
 			error = ENOTCONN;
 			goto out_unlocked;
 		}
 	}
 	if (stcb == NULL) {
 		if (addr == NULL) {
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, ENOENT);
 			error = ENOENT;
 			goto out_unlocked;
 		} else {
@@ -12741,7 +12648,6 @@ sctp_lower_sosend(struct socket *so,
 				 * User asks to abort a non-existent assoc,
 				 * or EOF a non-existent assoc with no data
 				 */
-				SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, ENOENT);
 				error = ENOENT;
 				goto out_unlocked;
 			}
@@ -12754,7 +12660,8 @@ sctp_lower_sosend(struct socket *so,
 			    p,
 			    SCTP_INITIALIZE_AUTH_PARAMS);
 			if (stcb == NULL) {
-				/* Error is setup for us in the call */
+				/* error is setup for us in the call. */
+				KASSERT(error != 0, ("error is 0 although stcb is NULL"));
 				goto out_unlocked;
 			}
 			SCTP_TCB_LOCK_ASSERT(stcb);
@@ -12774,6 +12681,8 @@ sctp_lower_sosend(struct socket *so,
 					    SCTP_FROM_SCTP_OUTPUT + SCTP_LOC_6);
 					hold_tcblock = false;
 					stcb = NULL;
+					KASSERT(error != 0,
+					    ("error is 0 although sctp_process_cmsgs_for_init() indicated an error"));
 					goto out_unlocked;
 				}
 			}
@@ -12793,8 +12702,16 @@ sctp_lower_sosend(struct socket *so,
 	KASSERT(hold_tcblock, ("hold_tcblock is false"));
 	SCTP_TCB_LOCK_ASSERT(stcb);
 	asoc = &stcb->asoc;
-	KASSERT((asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) == 0,
-	    ("Association about to be freed"));
+	if ((asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) ||
+	    (asoc->state & SCTP_STATE_WAS_ABORTED)) {
+		if (asoc->state & SCTP_STATE_WAS_ABORTED) {
+			/* XXX: Could also be ECONNABORTED, not enough info. */
+			error = ECONNRESET;
+		} else {
+			error = ENOTCONN;
+		}
+		goto out_unlocked;
+	}
 	/* Keep the stcb from being freed under our feet. */
 	atomic_add_int(&asoc->refcnt, 1);
 	free_cnt_applied = true;
@@ -12816,7 +12733,6 @@ sctp_lower_sosend(struct socket *so,
 			net = NULL;
 		if ((net == NULL) ||
 		    ((port != 0) && (port != stcb->rport))) {
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 			error = EINVAL;
 			goto out_unlocked;
 		}
@@ -12831,7 +12747,6 @@ sctp_lower_sosend(struct socket *so,
 
 	if (sctp_is_feature_on(inp, SCTP_PCB_FLAGS_NO_FRAGMENT)) {
 		if (sndlen > (ssize_t)asoc->smallest_mtu) {
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EMSGSIZE);
 			error = EMSGSIZE;
 			goto out_unlocked;
 		}
@@ -12839,7 +12754,7 @@ sctp_lower_sosend(struct socket *so,
 	if (SCTP_SO_IS_NBIO(so)
 	    || (flags & (MSG_NBIO | MSG_DONTWAIT)) != 0
 	    ) {
-		non_blocking = 1;
+		non_blocking = true;
 	}
 	/* would we block? */
 	if (non_blocking) {
@@ -12853,11 +12768,12 @@ sctp_lower_sosend(struct socket *so,
 		}
 		if ((SCTP_SB_LIMIT_SND(so) < (amount + inqueue_bytes + asoc->sb_send_resv)) ||
 		    (asoc->chunks_on_out_queue >= SCTP_BASE_SYSCTL(sctp_max_chunks_on_queue))) {
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EWOULDBLOCK);
-			if (sndlen > (ssize_t)SCTP_SB_LIMIT_SND(so))
+			if ((sndlen > (ssize_t)SCTP_SB_LIMIT_SND(so)) &&
+			    (user_marks_eor == 0)) {
 				error = EMSGSIZE;
-			else
+			} else {
 				error = EWOULDBLOCK;
+			}
 			goto out_unlocked;
 		}
 		asoc->sb_send_resv += (uint32_t)sndlen;
@@ -12865,15 +12781,9 @@ sctp_lower_sosend(struct socket *so,
 		atomic_add_int(&asoc->sb_send_resv, (int)sndlen);
 	}
 	local_soresv = sndlen;
-	if (asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) {
-		SCTP_LTRACE_ERR_RET(NULL, stcb, NULL, SCTP_FROM_SCTP_OUTPUT, ECONNRESET);
-		error = ECONNRESET;
-		goto out_unlocked;
-	}
 	/* Is the stream no. valid? */
 	if (srcv->sinfo_stream >= asoc->streamoutcnt) {
 		/* Invalid stream number */
-		SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 		error = EINVAL;
 		goto out_unlocked;
 	}
@@ -12887,27 +12797,18 @@ sctp_lower_sosend(struct socket *so,
 		} else {
 			error = EINVAL;
 		}
-		SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, error);
 		goto out_unlocked;
 	}
 	if ((SCTP_GET_STATE(stcb) == SCTP_STATE_COOKIE_WAIT) ||
 	    (SCTP_GET_STATE(stcb) == SCTP_STATE_COOKIE_ECHOED)) {
 		queue_only = 1;
 	}
-	/* we are now done with all control */
-	if (control) {
-		sctp_m_freem(control);
-		control = NULL;
-	}
 	if ((SCTP_GET_STATE(stcb) == SCTP_STATE_SHUTDOWN_SENT) ||
 	    (SCTP_GET_STATE(stcb) == SCTP_STATE_SHUTDOWN_RECEIVED) ||
 	    (SCTP_GET_STATE(stcb) == SCTP_STATE_SHUTDOWN_ACK_SENT) ||
 	    (asoc->state & SCTP_STATE_SHUTDOWN_PENDING)) {
-		if (sinfo_flags & SCTP_ABORT) {
-			;
-		} else {
-			SCTP_LTRACE_ERR_RET(NULL, stcb, NULL, SCTP_FROM_SCTP_OUTPUT, ECONNRESET);
-			error = ECONNRESET;
+		if ((sinfo_flags & SCTP_ABORT) == 0) {
+			error = EPIPE;
 			goto out_unlocked;
 		}
 	}
@@ -12921,6 +12822,8 @@ sctp_lower_sosend(struct socket *so,
 	SCTP_TCB_LOCK_ASSERT(stcb);
 	KASSERT((asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) == 0,
 	    ("Association about to be freed"));
+	KASSERT((asoc->state & SCTP_STATE_WAS_ABORTED) == 0,
+	    ("Association was aborted"));
 
 	/* Are we aborting? */
 	if (sinfo_flags & SCTP_ABORT) {
@@ -12931,12 +12834,11 @@ sctp_lower_sosend(struct socket *so,
 		SCTP_STAT_INCR(sctps_sends_with_abort);
 		if ((SCTP_GET_STATE(stcb) == SCTP_STATE_COOKIE_WAIT) ||
 		    (SCTP_GET_STATE(stcb) == SCTP_STATE_COOKIE_ECHOED)) {
-			/* It has to be up before we abort */
-			/* how big is the user initiated abort? */
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
+			/* It has to be up before we abort. */
 			error = EINVAL;
 			goto out;
 		}
+		/* How big is the user initiated abort? */
 		if (top != NULL) {
 			struct mbuf *cntm;
 
@@ -12951,17 +12853,14 @@ sctp_lower_sosend(struct socket *so,
 			tot_out = sndlen;
 			tot_demand = (tot_out + sizeof(struct sctp_paramhdr));
 			if (tot_demand > SCTP_DEFAULT_ADD_MORE) {
-				/* To big */
-				SCTP_LTRACE_ERR_RET(NULL, stcb, net, SCTP_FROM_SCTP_OUTPUT, EMSGSIZE);
 				error = EMSGSIZE;
-				goto out;
+				goto out_unlocked;
 			}
 			mm = sctp_get_mbuf_for_msg((unsigned int)tot_demand, 0, M_NOWAIT, 1, MT_DATA);
 		}
 		if (mm == NULL) {
-			SCTP_LTRACE_ERR_RET(NULL, stcb, net, SCTP_FROM_SCTP_OUTPUT, ENOMEM);
 			error = ENOMEM;
-			goto out;
+			goto out_unlocked;
 		}
 		max_out = asoc->smallest_mtu - sizeof(struct sctp_paramhdr);
 		max_out -= sizeof(struct sctp_abort_msg);
@@ -12979,8 +12878,18 @@ sctp_lower_sosend(struct socket *so,
 			error = uiomove((caddr_t)ph, (int)tot_out, uio);
 			SCTP_TCB_LOCK(stcb);
 			hold_tcblock = true;
-			if (asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) {
+			if ((asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) ||
+			    (asoc->state & SCTP_STATE_WAS_ABORTED)) {
 				sctp_m_freem(mm);
+				if (asoc->state & SCTP_STATE_WAS_ABORTED) {
+					/*
+					 * XXX: Could also be ECONNABORTED,
+					 * not enough info.
+					 */
+					error = ECONNRESET;
+				} else {
+					error = ENOTCONN;
+				}
 				goto out_unlocked;
 			}
 			if (error != 0) {
@@ -12991,6 +12900,7 @@ sctp_lower_sosend(struct socket *so,
 				 */
 				sctp_m_freem(mm);
 				mm = NULL;
+				error = 0;
 			}
 		} else {
 			if (sndlen != 0) {
@@ -13020,6 +12930,8 @@ sctp_lower_sosend(struct socket *so,
 	SCTP_TCB_LOCK_ASSERT(stcb);
 	KASSERT((asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) == 0,
 	    ("Association about to be freed"));
+	KASSERT((asoc->state & SCTP_STATE_WAS_ABORTED) == 0,
+	    ("Association was aborted"));
 
 	/* Calculate the maximum we can send */
 	inqueue_bytes = asoc->total_output_queue_size - (asoc->chunks_on_out_queue * SCTP_DATA_CHUNK_OVERHEAD(stcb));
@@ -13031,22 +12943,20 @@ sctp_lower_sosend(struct socket *so,
 	/* Unless E_EOR mode is on, we must make a send FIT in one call. */
 	if ((user_marks_eor == 0) &&
 	    (sndlen > (ssize_t)SCTP_SB_LIMIT_SND(stcb->sctp_socket))) {
-		/* It will NEVER fit */
-		SCTP_LTRACE_ERR_RET(NULL, stcb, net, SCTP_FROM_SCTP_OUTPUT, EMSGSIZE);
+		/* It will NEVER fit. */
 		error = EMSGSIZE;
 		goto out_unlocked;
 	}
-	if ((uio == NULL) && user_marks_eor) {
+	if ((uio == NULL) && (user_marks_eor != 0)) {
 		/*-
 		 * We do not support eeor mode for
 		 * sending with mbuf chains (like sendfile).
 		 */
-		SCTP_LTRACE_ERR_RET(NULL, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 		error = EINVAL;
 		goto out_unlocked;
 	}
 
-	if (user_marks_eor) {
+	if (user_marks_eor != 0) {
 		local_add_more = (ssize_t)min(SCTP_SB_LIMIT_SND(so), SCTP_BASE_SYSCTL(sctp_add_more_threshold));
 	} else {
 		/*-
@@ -13058,13 +12968,12 @@ sctp_lower_sosend(struct socket *so,
 	if (non_blocking) {
 		goto skip_preblock;
 	}
-	if (((max_len <= local_add_more) &&
-	    ((ssize_t)SCTP_SB_LIMIT_SND(so) >= local_add_more)) ||
+	if (((max_len <= local_add_more) && ((ssize_t)SCTP_SB_LIMIT_SND(so) >= local_add_more)) ||
 	    (max_len == 0) ||
 	    ((asoc->chunks_on_out_queue + asoc->stream_queue_cnt) >= SCTP_BASE_SYSCTL(sctp_max_chunks_on_queue))) {
-		/* No room right now ! */
-		SOCKBUF_LOCK(&so->so_snd);
+		/* No room right now! */
 		inqueue_bytes = asoc->total_output_queue_size - (asoc->chunks_on_out_queue * SCTP_DATA_CHUNK_OVERHEAD(stcb));
+		SOCKBUF_LOCK(&so->so_snd);
 		while ((SCTP_SB_LIMIT_SND(so) < (inqueue_bytes + local_add_more)) ||
 		    ((asoc->stream_queue_cnt + asoc->chunks_on_out_queue) >= SCTP_BASE_SYSCTL(sctp_max_chunks_on_queue))) {
 			SCTPDBG(SCTP_DEBUG_OUTPUT1, "pre_block limit:%u <(inq:%d + %zd) || (%d+%d > %d)\n",
@@ -13082,29 +12991,41 @@ sctp_lower_sosend(struct socket *so,
 			SCTP_TCB_UNLOCK(stcb);
 			hold_tcblock = false;
 			error = sbwait(&so->so_snd);
-			SCTP_TCB_LOCK(stcb);
-			hold_tcblock = true;
-			if (asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) {
-				SOCKBUF_UNLOCK(&so->so_snd);
-				goto out_unlocked;
-			}
-			stcb->block_entry = NULL;
 			if (error || so->so_error || be.error) {
 				if (error == 0) {
-					if (so->so_error)
+					if (so->so_error != 0) {
 						error = so->so_error;
-					if (be.error) {
+					}
+					if (be.error != 0) {
 						error = be.error;
 					}
 				}
 				SOCKBUF_UNLOCK(&so->so_snd);
 				goto out_unlocked;
 			}
+			SOCKBUF_UNLOCK(&so->so_snd);
+			SCTP_TCB_LOCK(stcb);
+			hold_tcblock = true;
+			if ((asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) ||
+			    (asoc->state & SCTP_STATE_WAS_ABORTED)) {
+				if (asoc->state & SCTP_STATE_WAS_ABORTED) {
+					/*
+					 * XXX: Could also be ECONNABORTED,
+					 * not enough info.
+					 */
+					error = ECONNRESET;
+				} else {
+					error = ENOTCONN;
+				}
+				goto out_unlocked;
+			}
+			stcb->block_entry = NULL;
 			if (SCTP_BASE_SYSCTL(sctp_logging_level) & SCTP_BLK_LOGGING_ENABLE) {
 				sctp_log_block(SCTP_BLOCK_LOG_OUTOF_BLK,
 				    asoc, asoc->total_output_queue_size);
 			}
 			inqueue_bytes = asoc->total_output_queue_size - (asoc->chunks_on_out_queue * SCTP_DATA_CHUNK_OVERHEAD(stcb));
+			SOCKBUF_LOCK(&so->so_snd);
 		}
 		if (SCTP_SB_LIMIT_SND(so) > inqueue_bytes) {
 			max_len = SCTP_SB_LIMIT_SND(so) - inqueue_bytes;
@@ -13120,6 +13041,8 @@ skip_preblock:
 	SCTP_TCB_LOCK_ASSERT(stcb);
 	KASSERT((asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) == 0,
 	    ("Association about to be freed"));
+	KASSERT((asoc->state & SCTP_STATE_WAS_ABORTED) == 0,
+	    ("Association was aborted"));
 
 	/*
 	 * sndlen covers for mbuf case uio_resid covers for the non-mbuf
@@ -13127,42 +13050,48 @@ skip_preblock:
 	 */
 	if (sndlen == 0) {
 		if (sinfo_flags & SCTP_EOF) {
-			got_all_of_the_send = 1;
+			got_all_of_the_send = true;
 			goto dataless_eof;
 		} else {
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 			error = EINVAL;
 			goto out;
 		}
 	}
+
 	if (top == NULL) {
 		struct sctp_stream_queue_pending *sp;
 		struct sctp_stream_out *strm;
 		uint32_t sndout;
 
-		/*
-		 * XXX: This will change soon, when the TCP send lock is
-		 * retired.
-		 */
-		SCTP_TCB_UNLOCK(stcb);
-		hold_tcblock = false;
-		SCTP_TCB_SEND_LOCK(stcb);
 		if ((asoc->stream_locked) &&
 		    (asoc->stream_locked_on != srcv->sinfo_stream)) {
-			SCTP_TCB_SEND_UNLOCK(stcb);
-			SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL);
 			error = EINVAL;
 			goto out;
 		}
 		strm = &asoc->strmout[srcv->sinfo_stream];
 		if (strm->last_msg_incomplete == 0) {
 	do_a_copy_in:
-			SCTP_TCB_SEND_UNLOCK(stcb);
+			SCTP_TCB_UNLOCK(stcb);
+			hold_tcblock = false;
 			sp = sctp_copy_it_in(stcb, asoc, srcv, uio, net, max_len, user_marks_eor, &error);
-			if (error) {
+			SCTP_TCB_LOCK(stcb);
+			hold_tcblock = true;
+			if ((asoc->state & SCTP_STATE_ABOUT_TO_BE_FREED) ||
+			    (asoc->state & SCTP_STATE_WAS_ABORTED)) {
+				if (asoc->state & SCTP_STATE_WAS_ABORTED) {
+					/*
+					 * XXX: Could also be ECONNABORTED,
+					 * not enough info.
+					 */
+					error = ECONNRESET;
+				} else {
+					error = ENOTCONN;
+				}
+				goto out;
+			}
+			if (error != 0) {
 				goto out;
 			}
-			SCTP_TCB_SEND_LOCK(stcb);
 			/* The out streams might be reallocated. */
 			strm = &asoc->strmout[srcv->sinfo_stream];
 			if (sp->msg_is_complete) {
@@ -13200,74 +13129,79 @@ skip_preblock:
 #endif
 				goto do_a_copy_in;
*** 1041 LINES SKIPPED ***



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