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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANn89i+sG1eHW+EAfcy+5_XTAF5VuQuNc+vjg_m=iqZ=ZPFE5A@mail.gmail.com>
Date: Mon, 24 Feb 2025 09:59:10 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Wang Hai <wanghai38@...wei.com>
Cc: kerneljasonxing@...il.com, ncardwell@...gle.com, kuniyu@...zon.com, 
	davem@...emloft.net, dsahern@...nel.org, kuba@...nel.org, pabeni@...hat.com, 
	horms@...nel.org, zhangchangzhong@...wei.com, liujian56@...wei.com, 
	yuehaibing@...wei.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] tcp: Defer ts_recent changes until req is owned

On Sun, Feb 23, 2025 at 8:36 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Sat, Feb 22, 2025 at 11:41 AM Wang Hai <wanghai38@...wei.com> 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.