On Fri, Aug 06, 2021 at 05:22:18PM +0200, Alexandr Nedvedicky wrote:
> > Although I did not obverve it, there seems to be the same problem
> > for snd_wl1 and rcv_up.  For rcv_up I copied the comparison with
> > rcv_nxt from urgent processing to keep future urgent data.

Over the weekend I thought about the SEQ_GT(tp->rcv_nxt, tp->rcv_up)
check.  I think FreeBSD is right and we don't need it.

In the regular case we may receive a retransmit of an old packet.
This check preserves the rcv_up from packets that we received earlier
but with higher sequence number.  But in the header prediction code
we know that TAILQ_EMPTY(&tp->t_segq).  So the current packet is
the most recent one and we can blindly take its rcv_nxt as urgent
pointer.

So maybe we want take the simplified diff below.

>     can you also share some details about testing you have done?
>     (tool + command line options)

That is quite tricky.  I do not have a simple test case for that.
One of our OpenBSD based product guarantees unidirectional traffic.
We habe a userland process that receives the data, sends it to
another OpenBSD machine.  There it goes to socket splicing and
finaly it ends in a Linux FTP server.

some magic -> user land sending process --> socket splicing --> Linux FTP

I suspect that with all the machines and processes involved, we
have strange timing and run into the race.

bluhm

Index: netinet/tcp_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.368
diff -u -p -r1.368 tcp_input.c
--- netinet/tcp_input.c 16 Apr 2021 12:08:25 -0000      1.368
+++ netinet/tcp_input.c 9 Aug 2021 09:41:05 -0000
@@ -966,6 +966,8 @@ findpcb:
                                        tp->t_pmtud_mss_acked = acked;
 
                                tp->snd_una = th->th_ack;
+                               /* Pull snd_wl2 up to prevent seq wrap. */
+                               tp->snd_wl2 = th->th_ack;
                                /*
                                 * We want snd_last to track snd_una so
                                 * as to avoid sequence wraparound problems
@@ -1015,6 +1017,9 @@ findpcb:
                                tcp_clean_sackreport(tp);
                        tcpstat_inc(tcps_preddat);
                        tp->rcv_nxt += tlen;
+                       /* Pull snd_wl1 and rcv_up up to prevent seq wrap. */
+                       tp->snd_wl1 = th->th_seq;
+                       tp->rcv_up = tp->rcv_nxt;
                        tcpstat_pkt(tcps_rcvpack, tcps_rcvbyte, tlen);
                        ND6_HINT(tp);
 

Reply via email to