Date: Fri, 19 Jun 2009 22:19:43 +0200 From: Andre Oppermann <andre@freebsd.org> To: Harti Brandt <harti@freebsd.org> Cc: FreeBSD/net@dlr.de, freebsd-net@freebsd.org Subject: Re: TCP bug? Message-ID: <4A3BF2DF.6080603@freebsd.org> In-Reply-To: <20090619191756.R581@beagle.kn.op.dlr.de> References: <20090619191756.R581@beagle.kn.op.dlr.de>
next in thread | previous in thread | raw e-mail | index | archive | help
Harti Brandt wrote:
> Hi all,
>
> one of my TCP test cases breaks in what one could call an edge case:
>
> When the TCP is in SYN-SENT state (the user has called connect()) and
> the peer answers with an almost-lamp test packet which has SYN, FIN, ACK
> and data larger than the window, TCP ACKs a window full of data, drops
> the rest, but processes the FIN - it goes into CLOSE_WAIT. This looks
> wrong to me. When dropping the data that is outside the window, it
> should also drop the FIN.
Indeed.
> The problem seems to be very old - I found it alread in rev. 1.1 of
> tcp_input.c. In -CURRENT it is on line 2590: when the sequence number of
> the incoming segment is the next expected one, the reassembly queue is
> empty and we are in an established state, the segment data is added to
> the socket buffer and all TCP header flags are cleared except for
> TH_FIN. Unfortunately here the original header flags are taken instead
> of the cached version in thflags. Earlier in the processing the
> out-of-window data and the FIN in thflags were chopped off and now
> TH_FIN reappears.
>
> The fix should be easy: instead of using the original flag byte to get
> the FIN use the cached copy.
...
> I wonder, though, why the code is as it is, i.e. why it takes the
> original FIN flag. Any idea?
I guess there are two, possibly interrelated, explanations for that
bug:
1) there are two code paths in TCP input processing depending on the
current state of the connection. The header prediction path and the
"slow" path. It's not always easy to keep them in sync. Sometimes
adjustments or additions are only made to one of them.
2) in old T/TCP (RFC1644) which we supported in our TCP code the SYN/FIN
combination was a valid one, though not directly intended for SYN/ACK/FIN.
It's easily imaginable that a bug or unforeseen interaction could creep in there.
Your proposed patch does exactly the right thing by using the cached copy.
I don't see any reason why thflags should be derived from th->th_flags way
down here again.
--
Andre
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4A3BF2DF.6080603>