Commit 646d2ac7 authored by Jakub Kicinski's avatar Jakub Kicinski

Merge branch 'tcp-make-simultaneous-connect-rfc-compliant'

Kuniyuki Iwashima says:

====================
tcp: Make simultaneous connect() RFC-compliant.

Patch 1 fixes an issue that BPF TCP option parser is triggered for ACK
instead of SYN+ACK in the case of simultaneous connect().

Patch 2 removes an wrong assumption in tcp_ao/self-connnect tests.

v2: https://lore.kernel.org/netdev/20240708180852.92919-1-kuniyu@amazon.com/
v1: https://lore.kernel.org/netdev/20240704035703.95065-1-kuniyu@amazon.com/
====================

Link: https://patch.msgid.link/20240710171246.87533-1-kuniyu@amazon.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 42ffe242 b3bb4d23
...@@ -5998,6 +5998,11 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb, ...@@ -5998,6 +5998,11 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
* RFC 5961 4.2 : Send a challenge ack * RFC 5961 4.2 : Send a challenge ack
*/ */
if (th->syn) { if (th->syn) {
if (sk->sk_state == TCP_SYN_RECV && sk->sk_socket && th->ack &&
TCP_SKB_CB(skb)->seq + 1 == TCP_SKB_CB(skb)->end_seq &&
TCP_SKB_CB(skb)->seq + 1 == tp->rcv_nxt &&
TCP_SKB_CB(skb)->ack_seq == tp->snd_nxt)
goto pass;
syn_challenge: syn_challenge:
if (syn_inerr) if (syn_inerr)
TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS); TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
...@@ -6007,6 +6012,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb, ...@@ -6007,6 +6012,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
goto discard; goto discard;
} }
pass:
bpf_skops_parse_hdr(sk, skb); bpf_skops_parse_hdr(sk, skb);
return true; return true;
...@@ -6813,6 +6819,9 @@ tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb) ...@@ -6813,6 +6819,9 @@ tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
tcp_fast_path_on(tp); tcp_fast_path_on(tp);
if (sk->sk_shutdown & SEND_SHUTDOWN) if (sk->sk_shutdown & SEND_SHUTDOWN)
tcp_shutdown(sk, SEND_SHUTDOWN); tcp_shutdown(sk, SEND_SHUTDOWN);
if (sk->sk_socket)
goto consume;
break; break;
case TCP_FIN_WAIT1: { case TCP_FIN_WAIT1: {
......
...@@ -30,8 +30,6 @@ static void setup_lo_intf(const char *lo_intf) ...@@ -30,8 +30,6 @@ static void setup_lo_intf(const char *lo_intf)
static void tcp_self_connect(const char *tst, unsigned int port, static void tcp_self_connect(const char *tst, unsigned int port,
bool different_keyids, bool check_restore) bool different_keyids, bool check_restore)
{ {
uint64_t before_challenge_ack, after_challenge_ack;
uint64_t before_syn_challenge, after_syn_challenge;
struct tcp_ao_counters before_ao, after_ao; struct tcp_ao_counters before_ao, after_ao;
uint64_t before_aogood, after_aogood; uint64_t before_aogood, after_aogood;
struct netstat *ns_before, *ns_after; struct netstat *ns_before, *ns_after;
...@@ -62,8 +60,6 @@ static void tcp_self_connect(const char *tst, unsigned int port, ...@@ -62,8 +60,6 @@ static void tcp_self_connect(const char *tst, unsigned int port,
ns_before = netstat_read(); ns_before = netstat_read();
before_aogood = netstat_get(ns_before, "TCPAOGood", NULL); before_aogood = netstat_get(ns_before, "TCPAOGood", NULL);
before_challenge_ack = netstat_get(ns_before, "TCPChallengeACK", NULL);
before_syn_challenge = netstat_get(ns_before, "TCPSYNChallenge", NULL);
if (test_get_tcp_ao_counters(sk, &before_ao)) if (test_get_tcp_ao_counters(sk, &before_ao))
test_error("test_get_tcp_ao_counters()"); test_error("test_get_tcp_ao_counters()");
...@@ -82,8 +78,6 @@ static void tcp_self_connect(const char *tst, unsigned int port, ...@@ -82,8 +78,6 @@ static void tcp_self_connect(const char *tst, unsigned int port,
ns_after = netstat_read(); ns_after = netstat_read();
after_aogood = netstat_get(ns_after, "TCPAOGood", NULL); after_aogood = netstat_get(ns_after, "TCPAOGood", NULL);
after_challenge_ack = netstat_get(ns_after, "TCPChallengeACK", NULL);
after_syn_challenge = netstat_get(ns_after, "TCPSYNChallenge", NULL);
if (test_get_tcp_ao_counters(sk, &after_ao)) if (test_get_tcp_ao_counters(sk, &after_ao))
test_error("test_get_tcp_ao_counters()"); test_error("test_get_tcp_ao_counters()");
if (!check_restore) { if (!check_restore) {
...@@ -98,18 +92,6 @@ static void tcp_self_connect(const char *tst, unsigned int port, ...@@ -98,18 +92,6 @@ static void tcp_self_connect(const char *tst, unsigned int port,
close(sk); close(sk);
return; return;
} }
if (after_challenge_ack <= before_challenge_ack ||
after_syn_challenge <= before_syn_challenge) {
/*
* It's also meant to test simultaneous open, so check
* these counters as well.
*/
test_fail("%s: Didn't challenge SYN or ACK: %zu <= %zu OR %zu <= %zu",
tst, after_challenge_ack, before_challenge_ack,
after_syn_challenge, before_syn_challenge);
close(sk);
return;
}
if (test_tcp_ao_counters_cmp(tst, &before_ao, &after_ao, TEST_CNT_GOOD)) { if (test_tcp_ao_counters_cmp(tst, &before_ao, &after_ao, TEST_CNT_GOOD)) {
close(sk); close(sk);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment