In TCP notify routines, check inpcb for INP_TIMEWAIT and INP_DROPPED.

The INP_DROPPED check replaces the current NULL checks; the INP_TIMEWAIT
checks appear to have always been required, but not been there, which
is/was a bug.  This avoids unconditionally casting of in_ppcb to a tcpcb,
when it may be a twtcb, which may have resulted in obscure ICMP-related
panics in earlier releases.

MFC after:	3 months

1 files changed, 81 insertions, 66 deletions

diff --git a/sys/netinet/tcp_timewait.c b/sys/netinet/tcp_timewait.c
index 055b0e0..66e438d 100644
--- a/sys/netinet/tcp_timewait.c
+++ b/sys/netinet/tcp_timewait.c
@@ -852,7 +852,13 @@ tcp_notify(struct inpcb *inp, int error)
 
 	INP_INFO_WLOCK_ASSERT(&tcbinfo);
 	INP_LOCK_ASSERT(inp);
+
+	if ((inp->inp_vflag & INP_TIMEWAIT) ||
+	    (inp->inp_vflag & INP_DROPPED))
+		return (inp);
+
 	tp = intotcpcb(inp);
+	KASSERT(tp != NULL, ("tcp_notify: tp == NULL"));
 
 	/*
 	 * Ignore some errors if we are hooked up.
@@ -1166,7 +1172,9 @@ tcp_ctlinput(int cmd, struct sockaddr *sa, void *vip)
 		    ip->ip_src, th->th_sport, 0, NULL);
 		if (inp != NULL)  {
 			INP_LOCK(inp);
-			if (inp->inp_socket != NULL) {
+			if (!(inp->inp_vflag & INP_TIMEWAIT) &&
+			    !(inp->inp_vflag & INP_DROPPED) &&
+			    !(inp->inp_socket == NULL)) {
 				icmp_tcp_seq = htonl(th->th_seq);
 				tp = intotcpcb(inp);
 				if (SEQ_GEQ(icmp_tcp_seq, tp->snd_una) &&
@@ -1434,16 +1442,20 @@ tcp_drop_syn_sent(struct inpcb *inp, int errno)
 
 	INP_INFO_WLOCK_ASSERT(&tcbinfo);
 	INP_LOCK_ASSERT(inp);
+
+	if ((inp->inp_vflag & INP_TIMEWAIT) ||
+	    (inp->inp_vflag & INP_DROPPED))
+		return (inp);
+
 	tp = intotcpcb(inp);
+	if (tp->t_state != TCPS_SYN_SENT)
+		return (inp);
 
-	if (tp != NULL && tp->t_state == TCPS_SYN_SENT) {
-		tp = tcp_drop(tp, errno);
-		if (tp != NULL)
-			return (inp);
-		else
-			return (NULL);
-	}
-	return (inp);
+	tp = tcp_drop(tp, errno);
+	if (tp != NULL)
+		return (inp);
+	else
+		return (NULL);
 }
 
 /*
@@ -1465,81 +1477,84 @@ tcp_mtudisc(struct inpcb *inp, int errno)
 #endif /* INET6 */
 
 	INP_LOCK_ASSERT(inp);
+	if ((inp->inp_vflag & INP_TIMEWAIT) ||
+	    (inp->inp_vflag & INP_DROPPED))
+		return (inp);
+
 	tp = intotcpcb(inp);
-	if (tp != NULL) {
+	KASSERT(tp != NULL, ("tcp_mtudisc: tp == NULL"));
+
 #ifdef INET6
-		isipv6 = (tp->t_inpcb->inp_vflag & INP_IPV6) != 0;
+	isipv6 = (tp->t_inpcb->inp_vflag & INP_IPV6) != 0;
 #endif
-		maxmtu = tcp_hc_getmtu(&inp->inp_inc); /* IPv4 and IPv6 */
-		romtu =
+	maxmtu = tcp_hc_getmtu(&inp->inp_inc); /* IPv4 and IPv6 */
+	romtu =
 #ifdef INET6
-		    isipv6 ? tcp_maxmtu6(&inp->inp_inc) :
+	    isipv6 ? tcp_maxmtu6(&inp->inp_inc) :
 #endif /* INET6 */
-		    tcp_maxmtu(&inp->inp_inc);
-		if (!maxmtu)
-			maxmtu = romtu;
-		else
-			maxmtu = min(maxmtu, romtu);
-		if (!maxmtu) {
-			tp->t_maxopd = tp->t_maxseg =
+	    tcp_maxmtu(&inp->inp_inc);
+	if (!maxmtu)
+		maxmtu = romtu;
+	else
+		maxmtu = min(maxmtu, romtu);
+	if (!maxmtu) {
+		tp->t_maxopd = tp->t_maxseg =
 #ifdef INET6
-				isipv6 ? tcp_v6mssdflt :
+			isipv6 ? tcp_v6mssdflt :
 #endif /* INET6 */
-				tcp_mssdflt;
-			return (inp);
-		}
-		mss = maxmtu -
+			tcp_mssdflt;
+		return (inp);
+	}
+	mss = maxmtu -
 #ifdef INET6
-			(isipv6 ?
-			 sizeof(struct ip6_hdr) + sizeof(struct tcphdr) :
+		(isipv6 ? sizeof(struct ip6_hdr) + sizeof(struct tcphdr) :
 #endif /* INET6 */
-			 sizeof(struct tcpiphdr)
+		 sizeof(struct tcpiphdr)
 #ifdef INET6
-			 )
+		 )
 #endif /* INET6 */
-			;
+		;
 
-		/*
-		 * XXX - The above conditional probably violates the TCP
-		 * spec.  The problem is that, since we don't know the
-		 * other end's MSS, we are supposed to use a conservative
-		 * default.  But, if we do that, then MTU discovery will
-		 * never actually take place, because the conservative
-		 * default is much less than the MTUs typically seen
-		 * on the Internet today.  For the moment, we'll sweep
-		 * this under the carpet.
-		 *
-		 * The conservative default might not actually be a problem
-		 * if the only case this occurs is when sending an initial
-		 * SYN with options and data to a host we've never talked
-		 * to before.  Then, they will reply with an MSS value which
-		 * will get recorded and the new parameters should get
-		 * recomputed.  For Further Study.
-		 */
-		if (tp->t_maxopd <= mss)
-			return (inp);
-		tp->t_maxopd = mss;
+	/*
+	 * XXX - The above conditional probably violates the TCP
+	 * spec.  The problem is that, since we don't know the
+	 * other end's MSS, we are supposed to use a conservative
+	 * default.  But, if we do that, then MTU discovery will
+	 * never actually take place, because the conservative
+	 * default is much less than the MTUs typically seen
+	 * on the Internet today.  For the moment, we'll sweep
+	 * this under the carpet.
+	 *
+	 * The conservative default might not actually be a problem
+	 * if the only case this occurs is when sending an initial
+	 * SYN with options and data to a host we've never talked
+	 * to before.  Then, they will reply with an MSS value which
+	 * will get recorded and the new parameters should get
+	 * recomputed.  For Further Study.
+	 */
+	if (tp->t_maxopd <= mss)
+		return (inp);
+	tp->t_maxopd = mss;
 
-		if ((tp->t_flags & (TF_REQ_TSTMP|TF_NOOPT)) == TF_REQ_TSTMP &&
-		    (tp->t_flags & TF_RCVD_TSTMP) == TF_RCVD_TSTMP)
-			mss -= TCPOLEN_TSTAMP_APPA;
+	if ((tp->t_flags & (TF_REQ_TSTMP|TF_NOOPT)) == TF_REQ_TSTMP &&
+	    (tp->t_flags & TF_RCVD_TSTMP) == TF_RCVD_TSTMP)
+		mss -= TCPOLEN_TSTAMP_APPA;
 #if	(MCLBYTES & (MCLBYTES - 1)) == 0
-		if (mss > MCLBYTES)
-			mss &= ~(MCLBYTES-1);
+	if (mss > MCLBYTES)
+		mss &= ~(MCLBYTES-1);
 #else
-		if (mss > MCLBYTES)
-			mss = mss / MCLBYTES * MCLBYTES;
+	if (mss > MCLBYTES)
+		mss = mss / MCLBYTES * MCLBYTES;
 #endif
-		if (so->so_snd.sb_hiwat < mss)
-			mss = so->so_snd.sb_hiwat;
+	if (so->so_snd.sb_hiwat < mss)
+		mss = so->so_snd.sb_hiwat;
 
-		tp->t_maxseg = mss;
+	tp->t_maxseg = mss;
 
-		tcpstat.tcps_mturesent++;
-		tp->t_rtttime = 0;
-		tp->snd_nxt = tp->snd_una;
-		tcp_output(tp);
-	}
+	tcpstat.tcps_mturesent++;
+	tp->t_rtttime = 0;
+	tp->snd_nxt = tp->snd_una;
+	tcp_output(tp);
 	return (inp);
 }