linux-kernel - Re: [PATCH net] tcp: Fix error ts_recent time during three-way handshake

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL+tcoCZQZWdTBNM5o2PEpzEnmgfZFFps1WuB9D75p2=Gkbf2Q@mail.gmail.com>
Date: Tue, 18 Feb 2025 20:02:02 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Wang Hai <wanghai38@...wei.com>
Cc: edumazet@...gle.com, ncardwell@...gle.com, kuniyu@...zon.com, 
	davem@...emloft.net, dsahern@...nel.org, kuba@...nel.org, pabeni@...hat.com, 
	horms@...nel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] tcp: Fix error ts_recent time during three-way handshake

Hi Wang,

On Tue, Feb 18, 2025 at 7:02 PM Wang Hai <wanghai38@...wei.com> wrote:
>
> If two ack packets from a connection enter tcp_check_req at the same time
> through different cpu, it may happen that req->ts_recent is updated with
> with a more recent time and the skb with an older time creates a new sock,
> which will cause the tcp_validate_incoming check to fail.
>
> cpu1                                cpu2
> tcp_check_req
>                                     tcp_check_req
> req->ts_recent = tmp_opt.rcv_tsval = t1
>                                     req->ts_recent = tmp_opt.rcv_tsval = t2
>
> 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) // failed
>
> In tcp_check_req, restore ts_recent to this skb's to fix this bug.

It's known that it's possible to receive two packets in two different
cpus like this case nearly at the same time. I'm curious if the socket
was running on the loopback?

Even if the above check that you commented in tcp_paws_check() fails,
how about the rest of the checks in tcp_validate_incoming()? In your
test, I doubt if really that check failed which finally caused the skb
from CPU2 to be discarded. Let me put it this way, is the paws_win
check the root cause? What is the drop reason for
tcp_validate_incoming()?

Thanks,
Jason

>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Wang Hai <wanghai38@...wei.com>
> ---
>  net/ipv4/tcp_minisocks.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index b089b08e9617..0208455f9eb8 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -878,6 +878,10 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
>         sock_rps_save_rxhash(child, skb);
>         tcp_synack_rtt_meas(child, req);
>         *req_stolen = !own_req;
> +       if (own_req && tcp_sk(child)->rx_opt.tstamp_ok &&
> +           unlikely(tcp_sk(child)->rx_opt.ts_recent != tmp_opt.rcv_tsval))
> +               tcp_sk(child)->rx_opt.ts_recent = tmp_opt.rcv_tsval;
> +
>         return inet_csk_complete_hashdance(sk, child, req, own_req);
>
>  listen_overflow:
> --
> 2.17.1
>
>