Re: [PATCH net v2] tcp: Defer ts_recent changes until req is owned

[Eric Dumazet]

On Sun, Feb 23, 2025 at 8:36 AM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
>
> On Sat, Feb 22, 2025 at 11:41 AM Wang Hai <wanghai38@xxxxxxxxxx> wrote:
> >
> > The same 5-tuple packet may be processed by different CPUSs, so two
> > CPUs may receive different ack packets at the same time when the
> > state is TCP_NEW_SYN_RECV.
> >
> > In that case, req->ts_recent in tcp_check_req may be changed concurrently,
> > which will probably cause the newsk's ts_recent to be incorrectly large.
> > So that tcp_validate_incoming will fail.
> >
> > cpu1                                    cpu2
> > tcp_check_req
> >                                         tcp_check_req
> >  req->ts_recent = rcv_tsval = t1
> >                                          req->ts_recent = rcv_tsval = t2
> >
> >  syn_recv_sock
> >   newsk->ts_recent = req->ts_recent = t2 // t1 < t2
> > tcp_child_process
> >  tcp_rcv_state_process
> >   tcp_validate_incoming
> >    tcp_paws_check
> >     if ((s32)(rx_opt->ts_recent - rx_opt->rcv_tsval) <= paws_win)
> >         // t2 - t1 > paws_win, failed
> >
> > In tcp_check_req, Defer ts_recent changes to this skb's to fix this bug.
>
> I think this sentence is a bit misleading.
>
> What your patch does is to no longer change req->ts_recent,
> but conditionally update tcp_sk(child)->rx_opt.ts_recent

Also please change the patch title.

The fix is about not changing req->ts_recent at all.