[PATCH net-next] tcp: refine tcp_sequence() for the FIN exception

[PATCH net-next] tcp: refine tcp_sequence() for the FIN exception

[Eric Dumazet]

Commit 0e24d17bd966 ("tcp: implement RFC 7323 window retraction
receiver requirements") removed the special FIN case that
was added in commit 1e3bb184e941 ("tcp: re-enable acceptance of
FIN packets when RWIN is 0").

If a peer sends a segment containing data and a FIN flag before
it learns about our window retraction and has a buggy TCP stack,
it might place the FIN one byte beyond what it thinks is the
right edge of the window (i.e., max_window_edge + 1).

The data portion (end_seq - th->fin) will end exactly at max_window_edge.
In this case, we will drop the packet if our receive queue is not empty,
even though the data was sent within the window we previously allowed.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Simon Baatz <gmbnomis@gmail.com>
---
 net/ipv4/tcp_input.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index ab7a4e5435a8a2cbb532d42c54af76d8541c903b..8560a9c6d38207c098d673497caf2c7652c36f5c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4812,18 +4812,20 @@ static enum skb_drop_reason tcp_sequence(const struct sock *sk,
 					 const struct tcphdr *th)
 {
 	const struct tcp_sock *tp = tcp_sk(sk);
+	u32 seq_limit;
 
 	if (before(end_seq, tp->rcv_wup))
 		return SKB_DROP_REASON_TCP_OLD_SEQUENCE;
 
-	if (unlikely(after(end_seq, tp->rcv_nxt + tcp_max_receive_window(tp)))) {
+	seq_limit = tp->rcv_nxt + tcp_max_receive_window(tp);
+	if (unlikely(after(end_seq, seq_limit))) {
 		/* Some stacks are known to handle FIN incorrectly; allow the
 		 * FIN to extend beyond the window and check it in detail later.
 		 */
-		if (!after(end_seq - th->fin, tp->rcv_nxt + tcp_receive_window(tp)))
+		if (!after(end_seq - th->fin, seq_limit))
 			return SKB_NOT_DROPPED_YET;
 
-		if (after(seq, tp->rcv_nxt + tcp_max_receive_window(tp)))
+		if (after(seq, seq_limit))
 			return SKB_DROP_REASON_TCP_INVALID_SEQUENCE;
 
 		/* Only accept this packet if receive queue is empty. */
-- 
2.54.0.1032.g2f8565e1d1-goog

Re: [PATCH net-next] tcp: refine tcp_sequence() for the FIN exception

[Neal Cardwell]

On Mon, Jun 8, 2026 at 11:14 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Commit 0e24d17bd966 ("tcp: implement RFC 7323 window retraction
> receiver requirements") removed the special FIN case that
> was added in commit 1e3bb184e941 ("tcp: re-enable acceptance of
> FIN packets when RWIN is 0").
>
> If a peer sends a segment containing data and a FIN flag before
> it learns about our window retraction and has a buggy TCP stack,
> it might place the FIN one byte beyond what it thinks is the
> right edge of the window (i.e., max_window_edge + 1).
>
> The data portion (end_seq - th->fin) will end exactly at max_window_edge.
> In this case, we will drop the packet if our receive queue is not empty,
> even though the data was sent within the window we previously allowed.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Simon Baatz <gmbnomis@gmail.com>
> ---

Reviewed-by: Neal Cardwell <ncardwell@google.com>

Thanks, Eric!

neal

Re: [PATCH net-next] tcp: refine tcp_sequence() for the FIN exception

[Kuniyuki Iwashima]

On Mon, Jun 8, 2026 at 8:14 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Commit 0e24d17bd966 ("tcp: implement RFC 7323 window retraction
> receiver requirements") removed the special FIN case that
> was added in commit 1e3bb184e941 ("tcp: re-enable acceptance of
> FIN packets when RWIN is 0").
>
> If a peer sends a segment containing data and a FIN flag before
> it learns about our window retraction and has a buggy TCP stack,
> it might place the FIN one byte beyond what it thinks is the
> right edge of the window (i.e., max_window_edge + 1).
>
> The data portion (end_seq - th->fin) will end exactly at max_window_edge.
> In this case, we will drop the packet if our receive queue is not empty,
> even though the data was sent within the window we previously allowed.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net-next] tcp: refine tcp_sequence() for the FIN exception

[Simon Baatz]

Hi Eric,

On Mon, Jun 08, 2026 at 03:14:52PM +0000, Eric Dumazet wrote:
> Commit 0e24d17bd966 ("tcp: implement RFC 7323 window retraction
> receiver requirements") removed the special FIN case that
> was added in commit 1e3bb184e941 ("tcp: re-enable acceptance of
> FIN packets when RWIN is 0").

Commit 0e24d17bd966 did not remove the special handling; it is still
present and covered by the test "tcp_rcv_zero_wnd_fin.pkt".
 
> If a peer sends a segment containing data and a FIN flag before
> it learns about our window retraction and has a buggy TCP stack,
> it might place the FIN one byte beyond what it thinks is the
> right edge of the window (i.e., max_window_edge + 1).

The FIN exception in tcp_data_queue() is not a generic allowance for
incorrect FIN handling.  It is much more specific and only applies
when:

1. the packet is in-sequence 
2. RWIN == 0
3. the packet is a bare FIN
 
> The data portion (end_seq - th->fin) will end exactly at max_window_edge.
> In this case, we will drop the packet if our receive queue is not empty,
> even though the data was sent within the window we previously allowed.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Simon Baatz <gmbnomis@gmail.com>
> ---
>  net/ipv4/tcp_input.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index ab7a4e5435a8a2cbb532d42c54af76d8541c903b..8560a9c6d38207c098d673497caf2c7652c36f5c 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4812,18 +4812,20 @@ static enum skb_drop_reason tcp_sequence(const struct sock *sk,
>  					 const struct tcphdr *th)
>  {
>  	const struct tcp_sock *tp = tcp_sk(sk);
> +	u32 seq_limit;
>  
>  	if (before(end_seq, tp->rcv_wup))
>  		return SKB_DROP_REASON_TCP_OLD_SEQUENCE;
>  
> -	if (unlikely(after(end_seq, tp->rcv_nxt + tcp_max_receive_window(tp)))) {
> +	seq_limit = tp->rcv_nxt + tcp_max_receive_window(tp);
> +	if (unlikely(after(end_seq, seq_limit))) {
>  		/* Some stacks are known to handle FIN incorrectly; allow the
>  		 * FIN to extend beyond the window and check it in detail later.
>  		 */
> -		if (!after(end_seq - th->fin, tp->rcv_nxt + tcp_receive_window(tp)))
> +		if (!after(end_seq - th->fin, seq_limit))
>  			return SKB_NOT_DROPPED_YET;

It is not clear which additional case this change is intended to
allow.  Are you sure such a packet would not be rejected by later
checks in the data path?

(For the existing FIN exception, the previous condition also seems
broader than necessary. Actually, it should be sufficient to use
"!after(end_seq - th->fin, tp->rcv_nxt)")

>  
> -		if (after(seq, tp->rcv_nxt + tcp_max_receive_window(tp)))
> +		if (after(seq, seq_limit))
>  			return SKB_DROP_REASON_TCP_INVALID_SEQUENCE;
>  
>  		/* Only accept this packet if receive queue is empty. */
> -- 
> 2.54.0.1032.g2f8565e1d1-goog
> 
-- 
Simon Baatz <gmbnomis@gmail.com>

Re: [PATCH net-next] tcp: refine tcp_sequence() for the FIN exception

[Eric Dumazet]

On Mon, Jun 8, 2026 at 3:12 PM Simon Baatz <gmbnomis@gmail.com> wrote:
>
> Hi Eric,
>
> On Mon, Jun 08, 2026 at 03:14:52PM +0000, Eric Dumazet wrote:
> > Commit 0e24d17bd966 ("tcp: implement RFC 7323 window retraction
> > receiver requirements") removed the special FIN case that
> > was added in commit 1e3bb184e941 ("tcp: re-enable acceptance of
> > FIN packets when RWIN is 0").
>
> Commit 0e24d17bd966 did not remove the special handling; it is still
> present and covered by the test "tcp_rcv_zero_wnd_fin.pkt".
>
> > If a peer sends a segment containing data and a FIN flag before
> > it learns about our window retraction and has a buggy TCP stack,
> > it might place the FIN one byte beyond what it thinks is the
> > right edge of the window (i.e., max_window_edge + 1).
>
> The FIN exception in tcp_data_queue() is not a generic allowance for
> incorrect FIN handling.  It is much more specific and only applies
> when:
>
> 1. the packet is in-sequence
> 2. RWIN == 0
> 3. the packet is a bare FIN
>
> > The data portion (end_seq - th->fin) will end exactly at max_window_edge.
> > In this case, we will drop the packet if our receive queue is not empty,
> > even though the data was sent within the window we previously allowed.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Simon Baatz <gmbnomis@gmail.com>
> > ---
> >  net/ipv4/tcp_input.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index ab7a4e5435a8a2cbb532d42c54af76d8541c903b..8560a9c6d38207c098d673497caf2c7652c36f5c 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -4812,18 +4812,20 @@ static enum skb_drop_reason tcp_sequence(const struct sock *sk,
> >                                        const struct tcphdr *th)
> >  {
> >       const struct tcp_sock *tp = tcp_sk(sk);
> > +     u32 seq_limit;
> >
> >       if (before(end_seq, tp->rcv_wup))
> >               return SKB_DROP_REASON_TCP_OLD_SEQUENCE;
> >
> > -     if (unlikely(after(end_seq, tp->rcv_nxt + tcp_max_receive_window(tp)))) {
> > +     seq_limit = tp->rcv_nxt + tcp_max_receive_window(tp);
> > +     if (unlikely(after(end_seq, seq_limit))) {
> >               /* Some stacks are known to handle FIN incorrectly; allow the
> >                * FIN to extend beyond the window and check it in detail later.
> >                */
> > -             if (!after(end_seq - th->fin, tp->rcv_nxt + tcp_receive_window(tp)))
> > +             if (!after(end_seq - th->fin, seq_limit))
> >                       return SKB_NOT_DROPPED_YET;
>
> It is not clear which additional case this change is intended to
> allow.  Are you sure such a packet would not be rejected by later
> checks in the data path?
>
> (For the existing FIN exception, the previous condition also seems
> broader than necessary. Actually, it should be sufficient to use
> "!after(end_seq - th->fin, tp->rcv_nxt)")

It is possible our internal sashiko instance got this wrong.

<quote>
If tcp_max_receive_window() is greater than tcp_receive_window()
(i.e. the window was shrunk), and end_seq is after tcp_max_receive_window(),
 then end_seq - th->fin will always be after tcp_receive_window().
Does this mean the FIN workaround is disabled when the window has been shrunk?
 Should this use tcp_max_receive_window() instead?
</quote>

Can you suggest an alternative? Why using two confusing variants of
what should be the same stuff?

Other points from sashilko:

> @@ -7185,6 +7183,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
>  */
>  WRITE_ONCE(tp->rcv_nxt, TCP_SKB_CB(skb)->seq + 1);
>  tp->rcv_wup = TCP_SKB_CB(skb)->seq + 1;
> + tp->rcv_mwnd_seq = tp->rcv_wup + tp->rcv_wnd;
>
>  /* RFC1323: The window in SYN & SYN/ACK segments is
>  * never scaled.

Since the SYN packet only advertises min(tp->rcv_wnd, 65535U), does setting
rcv_mwnd_seq to tp->rcv_wup + tp->rcv_wnd here track a much larger window than
was actually advertised if tp->rcv_wnd is greater than 65535? If window scaling
is not negotiated, this maximum window sequence number will remain inflated.
Should this be clamped to 65535?

[ ... ]

> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -612,6 +612,7 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
>  newtp->window_clamp = req->rsk_window_clamp;
>  newtp->rcv_ssthresh = req->rsk_rcv_wnd;
>  newtp->rcv_wnd = req->rsk_rcv_wnd;
> + newtp->rcv_mwnd_seq = newtp->rcv_wup + req->rsk_rcv_wnd;
>  newtp->rx_opt.wscale_ok = ireq->wscale_ok;

Similarly here, the SYNACK only advertises min(req->rsk_rcv_wnd, 65535U). Does
setting rcv_mwnd_seq using the unclamped req->rsk_rcv_wnd track a larger window
than was ever advertised?


Neal, what do you think?

Re: [PATCH net-next] tcp: refine tcp_sequence() for the FIN exception

[Simon Baatz]

On Mon, Jun 08, 2026 at 05:45:30PM -0700, Eric Dumazet wrote:
> On Mon, Jun 8, 2026 at 3:12???PM Simon Baatz <gmbnomis@gmail.com> wrote:
> >
> > Hi Eric,
> >
> > On Mon, Jun 08, 2026 at 03:14:52PM +0000, Eric Dumazet wrote:
> > > Commit 0e24d17bd966 ("tcp: implement RFC 7323 window retraction
> > > receiver requirements") removed the special FIN case that
> > > was added in commit 1e3bb184e941 ("tcp: re-enable acceptance of
> > > FIN packets when RWIN is 0").
> >
> > Commit 0e24d17bd966 did not remove the special handling; it is still
> > present and covered by the test "tcp_rcv_zero_wnd_fin.pkt".
> >
> > > If a peer sends a segment containing data and a FIN flag before
> > > it learns about our window retraction and has a buggy TCP stack,
> > > it might place the FIN one byte beyond what it thinks is the
> > > right edge of the window (i.e., max_window_edge + 1).
> >
> > The FIN exception in tcp_data_queue() is not a generic allowance for
> > incorrect FIN handling.  It is much more specific and only applies
> > when:
> >
> > 1. the packet is in-sequence
> > 2. RWIN == 0
> > 3. the packet is a bare FIN
> >
> > > The data portion (end_seq - th->fin) will end exactly at max_window_edge.
> > > In this case, we will drop the packet if our receive queue is not empty,
> > > even though the data was sent within the window we previously allowed.
> > >
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > Cc: Simon Baatz <gmbnomis@gmail.com>
> > > ---
> > >  net/ipv4/tcp_input.c | 8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > > index ab7a4e5435a8a2cbb532d42c54af76d8541c903b..8560a9c6d38207c098d673497caf2c7652c36f5c 100644
> > > --- a/net/ipv4/tcp_input.c
> > > +++ b/net/ipv4/tcp_input.c
> > > @@ -4812,18 +4812,20 @@ static enum skb_drop_reason tcp_sequence(const struct sock *sk,
> > >                                        const struct tcphdr *th)
> > >  {
> > >       const struct tcp_sock *tp = tcp_sk(sk);
> > > +     u32 seq_limit;
> > >
> > >       if (before(end_seq, tp->rcv_wup))
> > >               return SKB_DROP_REASON_TCP_OLD_SEQUENCE;
> > >
> > > -     if (unlikely(after(end_seq, tp->rcv_nxt + tcp_max_receive_window(tp)))) {
> > > +     seq_limit = tp->rcv_nxt + tcp_max_receive_window(tp);
> > > +     if (unlikely(after(end_seq, seq_limit))) {
> > >               /* Some stacks are known to handle FIN incorrectly; allow the
> > >                * FIN to extend beyond the window and check it in detail later.
> > >                */
> > > -             if (!after(end_seq - th->fin, tp->rcv_nxt + tcp_receive_window(tp)))
> > > +             if (!after(end_seq - th->fin, seq_limit))
> > >                       return SKB_NOT_DROPPED_YET;
> >
> > It is not clear which additional case this change is intended to
> > allow.  Are you sure such a packet would not be rejected by later
> > checks in the data path?
> >
> > (For the existing FIN exception, the previous condition also seems
> > broader than necessary. Actually, it should be sufficient to use
> > "!after(end_seq - th->fin, tp->rcv_nxt)")
> 
> It is possible our internal sashiko instance got this wrong.
> 
> <quote>
> If tcp_max_receive_window() is greater than tcp_receive_window()
> (i.e. the window was shrunk), and end_seq is after tcp_max_receive_window(),
>  then end_seq - th->fin will always be after tcp_receive_window().
> Does this mean the FIN workaround is disabled when the window has been shrunk?
>  Should this use tcp_max_receive_window() instead?
> </quote>
> 
> Can you suggest an alternative? Why using two confusing variants of
> what should be the same stuff?

Judging from past discussions, I think you prefer us to be strict in
tcp_sequence(), so I think we should replicate the conditions
(in-sequence, RWIN == 0, bare FIN) for the FIN exception in
tcp_data_queue() closely here.  I’ll send a patch that mirrors those
conditions in tcp_sequence().
 
> Other points from sashilko:
> 
> > @@ -7185,6 +7183,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
> >  */
> >  WRITE_ONCE(tp->rcv_nxt, TCP_SKB_CB(skb)->seq + 1);
> >  tp->rcv_wup = TCP_SKB_CB(skb)->seq + 1;
> > + tp->rcv_mwnd_seq = tp->rcv_wup + tp->rcv_wnd;
> >
> >  /* RFC1323: The window in SYN & SYN/ACK segments is
> >  * never scaled.
> 
> Since the SYN packet only advertises min(tp->rcv_wnd, 65535U), does setting
> rcv_mwnd_seq to tp->rcv_wup + tp->rcv_wnd here track a much larger window than
> was actually advertised if tp->rcv_wnd is greater than 65535? If window scaling
> is not negotiated, this maximum window sequence number will remain inflated.
> Should this be clamped to 65535?

We should never violate the tcp_receive_window() <=
tcp_max_receive_window() invariant in my opinion.  The point above
extends to tp->rcv_wnd itself, since that value is used to decide
e.g. when to send ACKs and thus, should also reflect the actual
RWIN.  I proposed to improve this in:

https://lore.kernel.org/netdev/20260408-tcp_rcv_exact_clamp_and_wnd-v1-0-76a6f212e153@gmail.com/

These inflated internal tcp_(max_)receive_window() values do occur in
"real life".  Here is a "ss -mit" output for a connection initiated
by an IoT (WiFi) device (connection is more than 2 days old at this
point):

ESTAB 0      0      192.168.74.95:mqtt  192.168.74.115:62897
   skmem:(r0,rb3538217,t0,tb87040,f0,w0,o0,bl0,d3557) cubic rto:373 rtt:152.029/44.771 ato:40 mss:1460 pmtu:1500 rcvmss:798 advmss:1460 cwnd:2 ssthresh:7 bytes_sent:13991 bytes_retrans:4 bytes_acked:13987 bytes_received:1022266 segs_out:16098 segs_in:21817 data_segs_out:6986 data_segs_in:14168 send 154kbps lastsnd:16433 lastrcv:16434 lastack:16240 pacing_rate 307kbps delivery_rate 3.96Mbps delivered:6985 app_limited busy:952155ms retrans:0/2 rcv_rtt:184612 rcv_space:64858 rcv_ssthresh:1021970 minrtt:14.059 snd_wnd:4993 rcv_wnd:1021440 rehash:2

That device has around 50KB of RAM and, consequently, a very
minimalistic TCP implementation (no TS, no window scaling). So all
calculations/decisions based on tcp_(max_)receive_window() operate on
a value that is off by a factor of 15 from the advertised value.

(Before you ask: The huge rb3538217 is caused by having no TS and the
window-based rcv_rtt estimator not delivering sensible results for
such a messaging-style connection.  The connection started with the
default rb131072.)

> [ ... ]
> 
> > diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> > --- a/net/ipv4/tcp_minisocks.c
> > +++ b/net/ipv4/tcp_minisocks.c
> > @@ -612,6 +612,7 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
> >  newtp->window_clamp = req->rsk_window_clamp;
> >  newtp->rcv_ssthresh = req->rsk_rcv_wnd;
> >  newtp->rcv_wnd = req->rsk_rcv_wnd;
> > + newtp->rcv_mwnd_seq = newtp->rcv_wup + req->rsk_rcv_wnd;
> >  newtp->rx_opt.wscale_ok = ireq->wscale_ok;
> 
> Similarly here, the SYNACK only advertises min(req->rsk_rcv_wnd, 65535U). Does
> setting rcv_mwnd_seq using the unclamped req->rsk_rcv_wnd track a larger window
> than was ever advertised?
> 
> 
> Neal, what do you think?

-- 
Simon Baatz <gmbnomis@gmail.com>