Sequence number handling issue with TCP data and FIN flag with a transient error

[Jean-Francois HREN]

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