From owner-freebsd-net@FreeBSD.ORG Wed Jun 17 13:19:32 2015 Return-Path: Delivered-To: freebsd-net@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2F644108 for ; Wed, 17 Jun 2015 13:19:32 +0000 (UTC) (envelope-from jean-francois.hren@stormshield.eu) Received: from work.netasq.com (gwlille.netasq.com [91.212.116.1]) by mx1.freebsd.org (Postfix) with ESMTP id E7423D01 for ; Wed, 17 Jun 2015 13:19:31 +0000 (UTC) (envelope-from jean-francois.hren@stormshield.eu) Received: from work.netasq.com (localhost.localdomain [127.0.0.1]) by work.netasq.com (Postfix) with ESMTP id 5C5452705D09; Wed, 17 Jun 2015 15:10:30 +0200 (CEST) Received: from localhost (localhost.localdomain [127.0.0.1]) by work.netasq.com (Postfix) with ESMTP id 1480A2705B10; Wed, 17 Jun 2015 15:10:30 +0200 (CEST) Received: from work.netasq.com ([127.0.0.1]) by localhost (work.netasq.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 3rzFI_XwOuQS; Wed, 17 Jun 2015 15:10:30 +0200 (CEST) Received: from work.netasq.com (localhost.localdomain [127.0.0.1]) by work.netasq.com (Postfix) with ESMTP id DFE7C270593A; Wed, 17 Jun 2015 15:10:29 +0200 (CEST) Date: Wed, 17 Jun 2015 15:10:29 +0200 (CEST) From: Jean-Francois HREN To: freebsd-net@freebsd.org Cc: Damien DEVILLE , Fabien Thomas Message-ID: <1176517609.2815392.1434546629748.JavaMail.zimbra@stormshield.eu> In-Reply-To: <1180135344.2814172.1434546351593.JavaMail.zimbra@stormshield.eu> Subject: Sequence number handling issue with TCP data and FIN flag with a transient error MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Thread-Topic: Sequence number handling issue with TCP data and FIN flag with a transient error Thread-Index: uMiNnxyH3izaz1y7npjyh1qFf+PBog== X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Jun 2015 13:19:32 -0000 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