svn commit: r368181 - in stable/12/sys/netinet: . tcp_stacks
Kyle Evans
kevans at freebsd.org
Wed Jan 6 15:01:31 UTC 2021
On Mon, Nov 30, 2020 at 3:45 AM Michael Tuexen <tuexen at freebsd.org> wrote:
>
> Author: tuexen
> Date: Mon Nov 30 09:45:44 2020
> New Revision: 368181
> URL: https://svnweb.freebsd.org/changeset/base/368181
>
> Log:
> MFC r367530:
> RFC 7323 specifies that:
> * TCP segments without timestamps should be dropped when support for
> the timestamp option has been negotiated.
> * TCP segments with timestamps should be processed normally if support
> for the timestamp option has not been negotiated.
> This patch enforces the above.
> Manually resolved merge conflicts.
>
> MFC 367891:
> Fix an issue I introuced in r367530: tcp_twcheck() can be called
> with to == NULL for SYN segments. So don't assume tp != NULL.
> Thanks to jhb@ for reporting and suggesting a fix.
>
> MFC r367946:
> Fix two occurences of a typo in a comment introduced in r367530.
> Thanks to lstewart@ for reporting them.
>
Hi Michael,
Dmitri (CC'd) spotted a regression in the golang test suite along
stable/12 and bisected it back to this MFC (reported via
efnet#bsdports). The test puts up a local HTTP server and attempts to
close the read-side while the write-side is still going, hopefully
observing a write failure on the write-side in the process (but it
never does).
I minimized it to this (rough) reproducer, which shows the write side
hanging around in CLOSE_WAIT and successfully writing the msg
repeatedly on recent -CURRENT while 12.2 observes an EPIPE almost
immediately: https://people.freebsd.org/~kevans/tcpr.c
root at viper:~/grep# sockstat -s | grep 8993
root a.out 80831 4 tcp4 127.0.0.1:8993 *:*
LISTEN
root a.out 80831 5 tcp4 127.0.0.1:8993
127.0.0.1:40319 CLOSE_WAIT
root at viper:~/grep#
Thanks,
Kyle Evans
>
> Modified:
> stable/12/sys/netinet/tcp_input.c
> stable/12/sys/netinet/tcp_stacks/rack.c
> stable/12/sys/netinet/tcp_syncache.c
> stable/12/sys/netinet/tcp_timewait.c
> Directory Properties:
> stable/12/ (props changed)
>
> Modified: stable/12/sys/netinet/tcp_input.c
> ==============================================================================
> --- stable/12/sys/netinet/tcp_input.c Mon Nov 30 09:22:33 2020 (r368180)
> +++ stable/12/sys/netinet/tcp_input.c Mon Nov 30 09:45:44 2020 (r368181)
> @@ -975,8 +975,8 @@ findpcb:
> }
> INP_INFO_RLOCK_ASSERT(&V_tcbinfo);
>
> - if (thflags & TH_SYN)
> - tcp_dooptions(&to, optp, optlen, TO_SYN);
> + tcp_dooptions(&to, optp, optlen,
> + (thflags & TH_SYN) ? TO_SYN : 0);
> /*
> * NB: tcp_twcheck unlocks the INP and frees the mbuf.
> */
> @@ -1706,20 +1706,29 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru
> }
>
> /*
> - * If timestamps were negotiated during SYN/ACK they should
> - * appear on every segment during this session and vice versa.
> + * If timestamps were negotiated during SYN/ACK and a
> + * segment without a timestamp is received, silently drop
> + * the segment.
> + * See section 3.2 of RFC 7323.
> */
> if ((tp->t_flags & TF_RCVD_TSTMP) && !(to.to_flags & TOF_TS)) {
> if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
> log(LOG_DEBUG, "%s; %s: Timestamp missing, "
> - "no action\n", s, __func__);
> + "segment silently dropped\n", s, __func__);
> free(s, M_TCPLOG);
> }
> + goto drop;
> }
> + /*
> + * If timestamps were not negotiated during SYN/ACK and a
> + * segment with a timestamp is received, ignore the
> + * timestamp and process the packet normally.
> + * See section 3.2 of RFC 7323.
> + */
> if (!(tp->t_flags & TF_RCVD_TSTMP) && (to.to_flags & TOF_TS)) {
> if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
> log(LOG_DEBUG, "%s; %s: Timestamp not expected, "
> - "no action\n", s, __func__);
> + "segment processed normally\n", s, __func__);
> free(s, M_TCPLOG);
> }
> }
>
> Modified: stable/12/sys/netinet/tcp_stacks/rack.c
> ==============================================================================
> --- stable/12/sys/netinet/tcp_stacks/rack.c Mon Nov 30 09:22:33 2020 (r368180)
> +++ stable/12/sys/netinet/tcp_stacks/rack.c Mon Nov 30 09:45:44 2020 (r368181)
> @@ -6708,7 +6708,27 @@ rack_hpts_do_segment(struct mbuf *m, struct tcphdr *th
> TCP_LOG_EVENT(tp, th, &so->so_rcv, &so->so_snd, TCP_LOG_IN, 0,
> tlen, &log, true);
> }
> +
> /*
> + * Parse options on any incoming segment.
> + */
> + tcp_dooptions(&to, (u_char *)(th + 1),
> + (th->th_off << 2) - sizeof(struct tcphdr),
> + (thflags & TH_SYN) ? TO_SYN : 0);
> +
> + /*
> + * If timestamps were negotiated during SYN/ACK and a
> + * segment without a timestamp is received, silently drop
> + * the segment.
> + * See section 3.2 of RFC 7323.
> + */
> + if ((tp->t_flags & TF_RCVD_TSTMP) && !(to.to_flags & TOF_TS)) {
> + way_out = 5;
> + retval = 0;
> + goto done_with_input;
> + }
> +
> + /*
> * Segment received on connection. Reset idle time and keep-alive
> * timer. XXX: This should be done after segment validation to
> * ignore broken/spoofed segs.
> @@ -6761,12 +6781,6 @@ rack_hpts_do_segment(struct mbuf *m, struct tcphdr *th
> rack_cong_signal(tp, th, CC_ECN);
> }
> }
> - /*
> - * Parse options on any incoming segment.
> - */
> - tcp_dooptions(&to, (u_char *)(th + 1),
> - (th->th_off << 2) - sizeof(struct tcphdr),
> - (thflags & TH_SYN) ? TO_SYN : 0);
>
> /*
> * If echoed timestamp is later than the current time, fall back to
> @@ -6898,6 +6912,7 @@ rack_hpts_do_segment(struct mbuf *m, struct tcphdr *th
> rack_timer_audit(tp, rack, &so->so_snd);
> way_out = 2;
> }
> + done_with_input:
> rack_log_doseg_done(rack, cts, nxt_pkt, did_out, way_out);
> if (did_out)
> rack->r_wanted_output = 0;
>
> Modified: stable/12/sys/netinet/tcp_syncache.c
> ==============================================================================
> --- stable/12/sys/netinet/tcp_syncache.c Mon Nov 30 09:22:33 2020 (r368180)
> +++ stable/12/sys/netinet/tcp_syncache.c Mon Nov 30 09:45:44 2020 (r368181)
> @@ -1142,6 +1142,40 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt
> }
>
> /*
> + * If timestamps were not negotiated during SYN/ACK and a
> + * segment with a timestamp is received, ignore the
> + * timestamp and process the packet normally.
> + * See section 3.2 of RFC 7323.
> + */
> + if (!(sc->sc_flags & SCF_TIMESTAMP) &&
> + (to->to_flags & TOF_TS)) {
> + if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
> + log(LOG_DEBUG, "%s; %s: Timestamp not "
> + "expected, segment processed normally\n",
> + s, __func__);
> + free(s, M_TCPLOG);
> + s = NULL;
> + }
> + }
> +
> + /*
> + * If timestamps were negotiated during SYN/ACK and a
> + * segment without a timestamp is received, silently drop
> + * the segment.
> + * See section 3.2 of RFC 7323.
> + */
> + if ((sc->sc_flags & SCF_TIMESTAMP) &&
> + !(to->to_flags & TOF_TS)) {
> + SCH_UNLOCK(sch);
> + if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
> + log(LOG_DEBUG, "%s; %s: Timestamp missing, "
> + "segment silently dropped\n", s, __func__);
> + free(s, M_TCPLOG);
> + }
> + return (-1); /* Do not send RST */
> + }
> +
> + /*
> * Pull out the entry to unlock the bucket row.
> *
> * NOTE: We must decrease TCPS_SYN_RECEIVED count here, not
> @@ -1184,32 +1218,6 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt
> log(LOG_DEBUG, "%s; %s: SEQ %u != IRS+1 %u, segment "
> "rejected\n", s, __func__, th->th_seq, sc->sc_irs);
> goto failed;
> - }
> -
> - /*
> - * If timestamps were not negotiated during SYN/ACK they
> - * must not appear on any segment during this session.
> - */
> - if (!(sc->sc_flags & SCF_TIMESTAMP) && (to->to_flags & TOF_TS)) {
> - if ((s = tcp_log_addrs(inc, th, NULL, NULL)))
> - log(LOG_DEBUG, "%s; %s: Timestamp not expected, "
> - "segment rejected\n", s, __func__);
> - goto failed;
> - }
> -
> - /*
> - * If timestamps were negotiated during SYN/ACK they should
> - * appear on every segment during this session.
> - * XXXAO: This is only informal as there have been unverified
> - * reports of non-compliants stacks.
> - */
> - if ((sc->sc_flags & SCF_TIMESTAMP) && !(to->to_flags & TOF_TS)) {
> - if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
> - log(LOG_DEBUG, "%s; %s: Timestamp missing, "
> - "no action\n", s, __func__);
> - free(s, M_TCPLOG);
> - s = NULL;
> - }
> }
>
> *lsop = syncache_socket(sc, *lsop, m);
>
> Modified: stable/12/sys/netinet/tcp_timewait.c
> ==============================================================================
> --- stable/12/sys/netinet/tcp_timewait.c Mon Nov 30 09:22:33 2020 (r368180)
> +++ stable/12/sys/netinet/tcp_timewait.c Mon Nov 30 09:45:44 2020 (r368181)
> @@ -373,9 +373,10 @@ tcp_twstart(struct tcpcb *tp)
> /*
> * Returns 1 if the TIME_WAIT state was killed and we should start over,
> * looking for a pcb in the listen state. Returns 0 otherwise.
> + * It be called with to == NULL only for pure SYN-segments.
> */
> int
> -tcp_twcheck(struct inpcb *inp, struct tcpopt *to __unused, struct tcphdr *th,
> +tcp_twcheck(struct inpcb *inp, struct tcpopt *to, struct tcphdr *th,
> struct mbuf *m, int tlen)
> {
> struct tcptw *tw;
> @@ -396,6 +397,8 @@ tcp_twcheck(struct inpcb *inp, struct tcpopt *to __unu
> goto drop;
>
> thflags = th->th_flags;
> + KASSERT(to != NULL || (thflags & (TH_SYN | TH_ACK)) == TH_SYN,
> + ("tcp_twcheck: called without options on a non-SYN segment"));
>
> /*
> * NOTE: for FIN_WAIT_2 (to be added later),
> @@ -443,6 +446,16 @@ tcp_twcheck(struct inpcb *inp, struct tcpopt *to __unu
> */
> if ((thflags & TH_ACK) == 0)
> goto drop;
> +
> + /*
> + * If timestamps were negotiated during SYN/ACK and a
> + * segment without a timestamp is received, silently drop
> + * the segment.
> + * See section 3.2 of RFC 7323.
> + */
> + if (((to->to_flags & TOF_TS) == 0) && (tw->t_recent != 0)) {
> + goto drop;
> + }
>
> /*
> * Reset the 2MSL timer if this is a duplicate FIN.
More information about the svn-src-all
mailing list