From 56cba4038add12f6239424c0712aec3dec008405 Mon Sep 17 00:00:00 2001 From: rwatson Date: Mon, 3 Apr 2006 14:07:50 +0000 Subject: 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 --- sys/netinet/tcp_timewait.c | 147 +++++++++++++++++++++++++-------------------- 1 file changed, 81 insertions(+), 66 deletions(-) (limited to 'sys/netinet/tcp_timewait.c') 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); } -- cgit v1.1