Re: [PATCH 2/4] tcp: Completely ignore data segment in CLOSE-WAIT state, log a message

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3030 bytes --]

On Thu, Oct 02, 2025 at 02:06:44AM +0200, Stefano Brivio wrote:
> According to RFC 9293 we should ignore data (note: not data segments)
> in CLOSE-WAIT state (indicated by TAP_FIN_RCVD), see 3.10.7.4
> "Other states":
> 
>   [...]
> 
>   Seventh, process the segment text:
> 
>   [...]
> 
>   CLOSE-WAIT STATE
> 
>   This should not occur since a FIN has been received from the remote
>   side. Ignore the segment text.
> 
> and we almost do that, except that we would look at the data length
> to decide whether it's a request for fast re-transmission, so fix
> that, and while at it, log a message, so that cases such as the
> following one are more apparent in debug logs:
> 
> 28692   0.009758 88.198.0.164 → 93.235.151.95 54 TCP 55414 → 47080 [FIN, ACK] Seq=121441 Ack=141 Win=65536 Len=0
> 
> we should ignore this FIN flag, because we didn't accept data up
> to this sequence (see next segment), but we don't do it, so, here:
> 
> 28693   0.000036 93.235.151.95 → 88.198.0.164 54 TCP 47080 → 55414 [ACK] Seq=141 Ack=90722 Win=32128 Len=0
> 28694   0.034597 93.235.151.95 → 88.198.0.164 54 TCP 47080 → 55414 [FIN, ACK] Seq=141 Ack=90722 Win=121216 Len=0
> 28695   0.000019 88.198.0.164 → 93.235.151.95 54 TCP 55414 → 47080 [ACK] Seq=121442 Ack=142 Win=65536 Len=0
> 28696   0.162968 88.198.0.164 → 93.235.151.95 30773 TCP [TCP Retransmission] 55414 → 47080 [FIN, PSH, ACK] Seq=90722 Ack=142 Win=65536 Len=30719 [TCP segment of a reassembled PDU]
> 
> we are erroneously in CLOSE-WAIT (TAP_FIN_RCVD) state, and this
> segment would look pretty strange there.
> 
> This specific case is fixed by the next patch, so it should never
> happen again.
> 
> Link: https://archives.passt.top/passt-dev/20250910115726.432bbb8d@elisabeth/
> Link: https://bugs.passt.top/show_bug.cgi?id=126
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tcp.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 48b1ef2..3f7dc82 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2130,9 +2130,15 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
>  
>  	/* Established connections not accepting data from tap */
>  	if (conn->events & TAP_FIN_RCVD) {
> +		size_t dlen;
>  		bool retr;
>  
> -		retr = th->ack && !tcp_packet_data_len(th, l4len) && !th->fin &&
> +		if ((dlen = tcp_packet_data_len(th, l4len))) {
> +			flow_dbg(conn, "data segment in CLOSE-WAIT (%zu B)",
> +				 dlen);
> +		}
> +
> +		retr = th->ack && !th->fin &&
>  		       ntohl(th->ack_seq) == conn->seq_ack_from_tap &&
>  		       ntohs(th->window) == conn->wnd_from_tap;
>  
> -- 
> 2.43.0
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]