Re: [PATCH net 1/2] tcp: re-enable acceptance of FIN packets when RWIN is 0

[Simon Baatz <gmbnomis@gmail.com>]

Hi Eric,

On Mon, Feb 23, 2026 at 09:36:38AM +0100, Eric Dumazet wrote:
> On Mon, Feb 23, 2026 at 9:01???AM Eric Dumazet <edumazet@google.com> wrote:
> ...
> > >
> > > Fixes: 9ca48d616ed7 ("tcp: do not accept packets beyond window")
> >
> > OK, but this commit is fine ? It seems the issue is coming from buggy peers ?

That "Fixes" tag is a technicality, I did not want to imply that the
commit is broken: 9ca48d616ed7 (which is RFC compliant) turns the
workaround (which isn't RFC compliant) 2bd99aef1b19 ("tcp:
accept bare FIN packets under memory pressure") into dead code.

If we still need that workaround, technically, this is a regression
caused by 9ca48d616ed7.  If not, I am perfectly happy to propose a
commit that removes the dead code.

> > Eventually the receive queue would be drained by the application, the
> > peer would retransmit
> > this FIN, and it would be accepted.

If I understand the problem correctly, the workaround was introduced
to break a FIN/ACK loop because the broken peer (macOS) did not use
exponential backoff in that scenario.  So yes, it would be accepted,
but until then we and the peer would ping-pong FIN/ACKs.

But as said, if we don't need the workaround I can submit a patch to
remove it.

> ...
> > > +       reason = tcp_sequence(sk, TCP_SKB_CB(skb)->seq,
> > > +                             TCP_SKB_CB(skb)->end_seq - th->fin);
> >
> > I don't think this is the right fix. Basically it says that FIN do not
> > count, but TCP RFC says otherwise.

We can be more specific and only allow that if we have a zero window. 
(I thought I would not hurt much to accept that FIN which does not
take up real space)

> > It also adds code in TCP fast path.

Hmm, the call site for tcp_validate_incoming() in
tcp_rcv_established() is:

	/*
	 *	Standard slow path.
	 */
validate:
	if (!tcp_validate_incoming(sk, skb, th, 1))
		return;


Am I missing something?

 
> We can keep fast path unchanged with this variant.
> 
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index e7b41abb82aad33d8cab4fcfa989cc4771149b41..156c92450f3ed00357aff2ef3e586b83f3cecb5e
> 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4858,15 +4858,24 @@ static enum skb_drop_reason
> tcp_disordered_ack_check(const struct sock *sk,
>   */
> 
>  static enum skb_drop_reason tcp_sequence(const struct sock *sk,
> -                                        u32 seq, u32 end_seq)
> +                                        u32 seq, u32 end_seq,
> +                                        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 (after(end_seq, tp->rcv_nxt + tcp_receive_window(tp))) {
> -               if (after(seq, tp->rcv_nxt + tcp_receive_window(tp)))
> +       seq_limit = tp->rcv_nxt + tcp_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, seq_limit))
> +                       return SKB_NOT_DROPPED_YET;
> +
> +               if (after(seq, seq_limit))
>                         return SKB_DROP_REASON_TCP_INVALID_SEQUENCE;
> 
>                 /* Only accept this packet if receive queue is empty. */
> @@ -6379,7 +6388,8 @@ static bool tcp_validate_incoming(struct sock
> *sk, struct sk_buff *skb,
> 
>  step1:
>         /* Step 1: check sequence number */
> -       reason = tcp_sequence(sk, TCP_SKB_CB(skb)->seq,
> TCP_SKB_CB(skb)->end_seq);
> +       reason = tcp_sequence(sk, TCP_SKB_CB(skb)->seq,
> +                             TCP_SKB_CB(skb)->end_seq, th);
>         if (reason) {
>                 /* RFC793, page 37: "In all states except SYN-SENT, all reset
>                  * (RST) segments are validated by checking their SEQ-fields."

Sure, I can do that.  Do you want me to add the
tcp_receive_window(tp) == 0 check to narrow it down further?

(And a dumb question: As you are the author of that code, how do I
attribute that commit when submitting a v2?)