Re: [PATCH net] tcp: Fix error ts_recent time during three-way handshake
From: Wang Hai
Date: Tue Feb 18 2025 - 21:09:42 EST
On 2025/2/18 20:02, Jason Xing wrote:
Hi Wang,
On Tue, Feb 18, 2025 at 7:02 PM Wang Hai <wanghai38@xxxxxxxxxx> 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?
Hi Jason,
Yeah, it's running in 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()?
The skb from cpu1 is a valid skb, so it should have passed the
tcp_validate_incoming check, but the current concurrency issue
prevented it from passing the check.
5951 static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
5952 const struct tcphdr *th, int
syn_inerr)
5953 {
5954 struct tcp_sock *tp = tcp_sk(sk);
5955 SKB_DR(reason);
5956
5957 /* RFC1323: H1. Apply PAWS check first. */
5958 if (!tcp_fast_parse_options(sock_net(sk), skb, th, tp) ||
5959 !tp->rx_opt.saw_tstamp ||
5960 tcp_paws_check(&tp->rx_opt, TCP_PAWS_WINDOW)) // failed
5961 goto step1;
5962
5963 reason = tcp_disordered_ack_check(sk, skb);
5964 if (!reason) // failed
5965 goto step1;
5966 /* Reset is accepted even if it did not pass PAWS. */
5967 if (th->rst) // failed
5968 goto step1;
5969 if (unlikely(th->syn)) // failed
5970 goto syn_challenge;
5971
5972 /* Old ACK are common, increment PAWS_OLD_ACK
5973 * and do not send a dupack.
5974 */
5975 if (reason == SKB_DROP_REASON_TCP_RFC7323_PAWS_ACK) {
5976 NET_INC_STATS(sock_net(sk), LINUX_MIB_PAWS_OLD_ACK);
5977 goto discard;
5978 }
5979 NET_INC_STATS(sock_net(sk), LINUX_MIB_PAWSESTABREJECTED);
5980 if (!tcp_oow_rate_limited(sock_net(sk), skb,
5981 LINUX_MIB_TCPACKSKIPPEDPAWS,
5982 &tp->last_oow_ack_time))
5983 tcp_send_dupack(sk, skb);
5984 goto discard; // Drop the skb from here.
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()?
The values of ts_recent and rcv_tsval are compared in tcp_paws_check,
where ts_recent comes from req->ts_recent = t2 and rcv_tsval is the
current cpu1 skb time. ts_recent - rcv_tsval may be greater than 1
and here will fail here. Tcp_paws_check fails causing
tcp_validate_incoming to fail too.
Thanks,
Jason
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Wang Hai <wanghai38@xxxxxxxxxx>
---
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