Date: Wed, 17 Jun 2015 15:10:29 +0200 (CEST) From: Jean-Francois HREN <jean-francois.hren@stormshield.eu> To: freebsd-net@freebsd.org Cc: Damien DEVILLE <damien.deville@stormshield.eu>, Fabien Thomas <fabien.thomas@stormshield.eu> Subject: Sequence number handling issue with TCP data and FIN flag with a transient error Message-ID: <1176517609.2815392.1434546629748.JavaMail.zimbra@stormshield.eu> In-Reply-To: <1180135344.2814172.1434546351593.JavaMail.zimbra@stormshield.eu>
next in thread | previous in thread | raw e-mail | index | archive | help
Hello, while investigating a freeze on a modified FreeBSD 9.3 I stumbled upon a potential bug in netinet/tcp_output.c If an error occurs while processing a TCP segment with some data and the FIN flag, the back out of the sequence number advance does not take into account the increase by 1 due to the FIN flag (see https://svnweb.freebsd.org/base/head/sys/netinet/tcp_output.c?view=markup#l1360 and https://svnweb.freebsd.org/base/head/sys/netinet/tcp_output.c?view=markup#l1439 ). In the case of a transient error, this leads to a retransmitted TCP segment with a shifted by 1 sequence number and a missing first byte in the TCP payload. In FreeBSD 9.3, it happens only when an error occurs in netinet/ip_output.c::ip_output() or netinet6/ip6_output::ip6_output() but in head, R249372 ( https://svnweb.freebsd.org/base?view=revision&revision=249372 ) now allows the same behaviour if an ENOBUFS error occurs in netinet/tcp_output.c Tentative solutions would be either to remove the back out of the sequence number advance completely and to treat transient error cases like real lost packets --- netinet/tcp_output.c +++ netinet/tcp_output.c @@ -1435,8 +1435,7 @@ tp->sackhint.sack_bytes_rexmit -= len; KASSERT(tp->sackhint.sack_bytes_rexmit >= 0, ("sackhint bytes rtx >= 0")); - } else - tp->snd_nxt -= len; + } } SOCKBUF_UNLOCK_ASSERT(&so->so_snd); /* Check gotos. */ switch (error) { or to decrease the sequence number advance by 1 if a FIN flag was sent. --- netinet/tcp_output.c +++ netinet/tcp_output.c @@ -1435,8 +1435,11 @@ tp->sackhint.sack_bytes_rexmit -= len; KASSERT(tp->sackhint.sack_bytes_rexmit >= 0, ("sackhint bytes rtx >= 0")); - } else + } else { tp->snd_nxt -= len; + if (flags & TH_FIN) + tp->snd_nxt--; + } } SOCKBUF_UNLOCK_ASSERT(&so->so_snd); /* Check gotos. */ switch (error) { Jean-François Hren ASQ Team Member http://www.stormshield.eu STORMSHIELD Parc Scientifique de la Haute Borne Parc Horizon - Bâtiment 6 Avenue de l'Horizon 59650 Villeneuve d'Ascq France
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1176517609.2815392.1434546629748.JavaMail.zimbra>