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 up= on a potential bug in netinet/tcp_output.c If an error occurs while processing a TCP segment with some data and the FI= N 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=3Dm= arkup#l1360 and https://svnweb.freebsd.org/base/head/sys/netinet/tcp_output.c?view=3Dma= rkup#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=3Drevision&revision=3D249372 ) now a= llows 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 @@ =09=09=09=09tp->sackhint.sack_bytes_rexmit -=3D len; =09=09=09=09KASSERT(tp->sackhint.sack_bytes_rexmit >=3D 0, =09=09=09=09 ("sackhint bytes rtx >=3D 0")); -=09=09=09} else -=09=09=09=09tp->snd_nxt -=3D len; +=09=09=09} =09=09} =09=09SOCKBUF_UNLOCK_ASSERT(&so->so_snd);=09/* Check gotos. */ =09=09switch (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 @@ =09=09=09=09tp->sackhint.sack_bytes_rexmit -=3D len; =09=09=09=09KASSERT(tp->sackhint.sack_bytes_rexmit >=3D 0, =09=09=09=09 ("sackhint bytes rtx >=3D 0")); -=09=09=09} else +=09=09=09} else { =09=09=09=09tp->snd_nxt -=3D len; +=09=09=09=09if (flags & TH_FIN) +=09=09=09=09=09tp->snd_nxt--; +=09=09=09} =09=09} =09=09SOCKBUF_UNLOCK_ASSERT(&so->so_snd);=09/* Check gotos. */ =09=09switch (error) { Jean-Fran=C3=A7ois Hren ASQ Team Member http://www.stormshield.eu STORMSHIELD Parc Scientifique de la Haute Borne Parc Horizon - B=C3=A2timent 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>