Acquire the send socket buffer lock around tcp_output() activities

reaching into the socket buffer.  This prevents a number of potential
races, including dereferencing of sb_mb while unlocked leading to
a NULL pointer deref (how I found it).  Potentially this might also
explain other "odd" TCP behavior on SMP boxes (although  haven't
seen it reported).

RELENG_5 candidate.

1 files changed, 14 insertions, 2 deletions

diff --git a/sys/netinet/tcp_output.c b/sys/netinet/tcp_output.c
index 18da2cd..15bf0c0 100644
--- a/sys/netinet/tcp_output.c
+++ b/sys/netinet/tcp_output.c
@@ -252,6 +252,7 @@ after_sack_rexmit:
 	if (tp->t_flags & TF_NEEDSYN)
 		flags |= TH_SYN;
 
+	SOCKBUF_LOCK(&so->so_snd);
 	/*
 	 * If in persist timeout with window of 0, send 1 byte.
 	 * Otherwise, if window is small but nonzero
@@ -332,7 +333,7 @@ after_sack_rexmit:
 			tcp_hc_gettao(&tp->t_inpcb->inp_inc, &tao);
 		if (len > 0 && tp->t_state == TCPS_SYN_SENT &&
 		     tao.tao_ccsent == 0)
-			return 0;
+			goto just_return;
 	}
 
 	/*
@@ -477,7 +478,7 @@ after_sack_rexmit:
 	    !callout_active(tp->tt_persist)) {
 		callout_reset(tp->tt_rexmt, tp->t_rxtcur,
 			      tcp_timer_rexmt, tp);
-		return (0);
+		goto just_return;
 	} 
 	/*
 	 * TCP window updates are not reliable, rather a polling protocol
@@ -510,9 +511,12 @@ after_sack_rexmit:
 	/*
 	 * No reason to send a segment, just return.
 	 */
+just_return:
+	SOCKBUF_UNLOCK(&so->so_snd);
 	return (0);
 
 send:
+	SOCKBUF_LOCK_ASSERT(&so->so_snd);
 	/*
 	 * Before ESTABLISHED, force sending of initial options
 	 * unless TCP set not to do any options.
@@ -762,6 +766,7 @@ send:
 #ifdef notyet
 		if ((m = m_copypack(so->so_snd.sb_mb, off,
 		    (int)len, max_linkhdr + hdrlen)) == 0) {
+			SOCKBUF_UNLOCK(&so->so_snd);
 			error = ENOBUFS;
 			goto out;
 		}
@@ -773,6 +778,7 @@ send:
 #else
 		MGETHDR(m, M_DONTWAIT, MT_HEADER);
 		if (m == NULL) {
+			SOCKBUF_UNLOCK(&so->so_snd);
 			error = ENOBUFS;
 			goto out;
 		}
@@ -780,6 +786,7 @@ send:
 		if (MHLEN < hdrlen + max_linkhdr) {
 			MCLGET(m, M_DONTWAIT);
 			if ((m->m_flags & M_EXT) == 0) {
+				SOCKBUF_UNLOCK(&so->so_snd);
 				m_freem(m);
 				error = ENOBUFS;
 				goto out;
@@ -795,6 +802,7 @@ send:
 		} else {
 			m->m_next = m_copy(so->so_snd.sb_mb, off, (int) len);
 			if (m->m_next == 0) {
+				SOCKBUF_UNLOCK(&so->so_snd);
 				(void) m_free(m);
 				error = ENOBUFS;
 				goto out;
@@ -809,7 +817,9 @@ send:
 		 */
 		if (off + len == so->so_snd.sb_cc)
 			flags |= TH_PUSH;
+		SOCKBUF_UNLOCK(&so->so_snd);
 	} else {
+		SOCKBUF_UNLOCK(&so->so_snd);
 		if (tp->t_flags & TF_ACKNOW)
 			tcpstat.tcps_sndacks++;
 		else if (flags & (TH_SYN|TH_FIN|TH_RST))
@@ -833,6 +843,7 @@ send:
 		m->m_data += max_linkhdr;
 		m->m_len = hdrlen;
 	}
+	SOCKBUF_UNLOCK_ASSERT(&so->so_snd);
 	m->m_pkthdr.rcvif = (struct ifnet *)0;
 #ifdef MAC
 	mac_create_mbuf_from_inpcb(tp->t_inpcb, m);
@@ -1117,6 +1128,7 @@ timer:
 		}
 
 out:
+		SOCKBUF_UNLOCK_ASSERT(&so->so_snd);	/* Check gotos. */
 		if (error == ENOBUFS) {
 	                if (!callout_active(tp->tt_rexmt) &&
 			    !callout_active(tp->tt_persist))