Re: [PATCH net-next] tcp: tighten the FIN exception in tcp_sequence()

[Simon Baatz]

Hi Eric,

On Fri, Jun 12, 2026 at 08:47:51PM -0700, Eric Dumazet wrote:
> On Fri, Jun 12, 2026 at 4:40???PM Simon Baatz <gmbnomis@xxxxxxxxx> wrote:
> >
> > Hi Jakub,
> >
> > On Fri, Jun 12, 2026 at 03:43:55PM -0700, Jakub Kicinski wrote:
> > > On Wed, 10 Jun 2026 00:09:24 +0200 Simon Baatz via B4 Relay wrote:
> > > > From: Simon Baatz <gmbnomis@xxxxxxxxx>
> > > >
> > > > Commit 1e3bb184e941 ("tcp: re-enable acceptance of FIN packets when
> > > > RWIN is 0") added a special case in tcp_sequence() to mirror the FIN
> > > > exception in tcp_data_queue(), which accepts bare in-order FINs even
> > > > when the advertised window is zero. That behavior is not
> > > > RFC-compliant, but was introduced in commit 2bd99aef1b19 ("tcp: accept
> > > > bare FIN packets under memory pressure") to break tight FIN/ACK loops
> > > > caused by broken clients.
> > > >
> > > > However, the condition added by commit 1e3bb184e941 ("tcp: re-enable
> > > > acceptance of FIN packets when RWIN is 0") is broader than required
> > > > and allows other non-compliant packets as well.
> > > >
> > > > Tighten the tcp_sequence() FIN exception to only allow packets where
> > > > the packet is a bare in-order FIN and only the FIN flag extends beyond
> > > > tcp_max_receive_window(). In particular, this exception is only
> > > > reachable if tcp_max_receive_window() is zero. Otherwise the packet is
> > > > already accepted by the normal sequence check.
> > > >
> > > > The existing packetdrill test tcp_rcv_zero_wnd_fin.pkt exercises this
> > > > behavior already and does not need to be changed.
> > > >
> > > > Signed-off-by: Simon Baatz <gmbnomis@xxxxxxxxx>
> > >
> > > This is odd. You are sending this patch which shares a lot of
> > > similarities with Eric's patch:
> > > https://lore.kernel.org/all/20260608151452.706822-1-edumazet@xxxxxxxxxx/
> > >
> > > Why are you submitting your own patch instead of discussing it further
> > > with Eric and letting him send v2?
> >
> > That's what I understood from Eric's reply to my comments.  He asked
> > for an alternative, so I sent this as a concrete sugggestion.
> 
> Yes this is fine, please next time includ a link to the 'other patch'
> since this discussion
> was started by someone :)

Hmm, in hindsight this is quite obvious...
 
> About your patch, I thought that it would be fine to allow a remote
> peer to add a FIN
> to a payload packet of N bytes even if RWIN == N

I just realized that RFC 9293 has an "or" in:

  RCV.NXT =< SEG.SEQ < RCV.NXT+RCV.WND

  or

  RCV.NXT =< SEG.SEQ+SEG.LEN-1 < RCV.NXT+RCV.WND

(SEG.LEN includes SYN/FIN)

Linux TCP is stricter here effectively requiring both conditions
since commit 9ca48d6 ("tcp: do not accept packets beyond window"). 
Given that, it's fine to accept packets with an "additional FIN" that
fulfill the first condition.

> It seems the bug of your other stack is that the FIN can be sent with
> no payload,
> and this is a less broader case.
> 
> FIN storage (a bit) is there, we can generalize the acceptance of FIN for free ?

Agreed. Let's drop this patch and generalize the FIN handling as you suggest.

- Simon

-- 
Simon Baatz <gmbnomis@xxxxxxxxx>