Date: Thu, 21 May 2009 10:21:07 -0400 From: George Neville-Neil <gnn@neville-neil.com> To: Zachary Loafman <zachary.loafman@isilon.com> Cc: net@freebsd.org Subject: Re: [PATCH] SYN issue Message-ID: <43B043BD-4D7B-414B-91AB-3C8DBA0EC078@neville-neil.com> In-Reply-To: <20090519211346.GC675@isilon.com> References: <20090519211346.GC675@isilon.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On May 19, 2009, at 17:13 , Zachary Loafman wrote:
> net@ -
>
> A short patch attached that requires 3 paragraphs of explanation.
>
> We found an issue in TCP when the a client connects to our server,
> establishes a connection, reboots and chooses the same source port to
> re-establish the connection. This isn't hard from other vendors'
> clients. On Solaris, the same NFS mount order at boot time will
> frequently result in source port re-use for the NFS connections. In
> this case, the customer was seeing mounts hang until the keepalive on
> our side would kick the established connection.
>
> The problem in the code is probably best explained using the patch
> itself:
>
> ---
> Index: sys/netinet/tcp_input.c
> ===================================================================
> --- sys/netinet/tcp_input.c (revision xxxx)
> +++ sys/netinet/tcp_input.c (working copy)
> @@ -1818,7 +1818,11 @@ tcp_do_segment(struct mbuf *m, struct tc
>
> todrop = tp->rcv_nxt - th->th_seq;
> if (todrop > 0) {
> - if (thflags & TH_SYN) {
> + /*
> + * If this is a duplicate SYN for our current
> connection,
> + * advance over it and pretend and it's not a SYN.
> + */
> + if (thflags & TH_SYN && th->th_seq == tp->irs) {
> thflags &= ~TH_SYN;
> th->th_seq++;
> if (th->th_urp > 1)
> ---
>
> The problem is that when our TCP stack gets a SYN packet for a
> connection that's already in ESTABLISHED state, it runs through the
> above code. The above code is basically noticing that the packet is
> coming in left of the receive window and then saying "Ah, a SYN! This
> must be a duplicate SYN for our existing connect." After that, it just
> turns off SYN and treats it as a normal packet (after advancing past
> the SYN seq number). The code is broken, though: the only condition
> under which this is a duplicate SYN is if the th_seq matches the irs,
> the initial receive sequence.
>
> After correcting the above, any SYN that doesn't exactly match the
> initial sequence number results in a RST|ACK response and the
> ESTABLISHED connection being dropped. Before this change, this is also
> what happened if a SYN arrived within or past the window, so I'm
> basically making the before-window behavior match the other
> behavior. I tested this using telnet to establish a TCP connection and
> raw packet injection to throw SYNs at it.
>
> Comments?
>
Yes, nice catch.
Best,
George
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?43B043BD-4D7B-414B-91AB-3C8DBA0EC078>