On Mon, Feb 23, 2026 at 6:16 PM Simon Baatz <[email protected]> wrote:
>
> 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 <[email protected]> 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?
The 'slow path' comment does not mean everything from this can be really slow.
'slow path' is quite often used, header prediction is less and less a thing.
>
>
> > 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?
Not sure what you mean.
>
> (And a dumb question: As you are the author of that code, how do I
> attribute that commit when submitting a v2?)
No worries, this is part of the review process, I give you permission to
be the author, even if this happens to be something I wrote.
