Netdev List
 help / color / mirror / Atom feed
* [PATCH v1 net 0/3] tcp: Small fixes for DEFER_CONNECT.
@ 2026-11-07 23:56 Kuniyuki Iwashima
  2026-11-07 23:56 ` [PATCH v1 net 1/3] tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort() Kuniyuki Iwashima
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-07 23:56 UTC (permalink / raw)
  To: Eric Dumazet, Neal Cardwell, David S . Miller, Jakub Kicinski,
	Paolo Abeni
  Cc: Yuchung Cheng, Wei Wang, Matthieu Baerts, Mat Martineau,
	Geliang Tang, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev

Chris Mason reported a bug that TFO client socket can accept
segments even before it sends SYN.

He provided a patch too, but it needed some adjustments (Patch 2).

Patch 1 is a prereq fix for Patch 2, and Patch 3 is packetdrill
test for the reported bug.


Chris Mason (1):
  tcp: Drop any segments if DEFER_CONNECT is set.

Kuniyuki Iwashima (2):
  tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort().
  selftest: packetdrill: Add test for DEFER_CONNECT.

 include/net/inet_common.h                     |  2 +-
 net/ipv4/af_inet.c                            | 10 +--
 net/ipv4/tcp.c                                |  9 ++-
 net/ipv4/tcp_input.c                          |  9 +++
 net/ipv4/tcp_ipv4.c                           |  2 +-
 net/ipv6/tcp_ipv6.c                           |  2 +-
 net/mptcp/protocol.c                          |  5 +-
 .../tcp_fastopen_client_defer-connect.pkt     | 61 +++++++++++++++++++
 8 files changed, 89 insertions(+), 11 deletions(-)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_fastopen_client_defer-connect.pkt

-- 
2.56.0.455.g6c5a503b38-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v1 net 1/3] tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort().
  2026-11-07 23:56 [PATCH v1 net 0/3] tcp: Small fixes for DEFER_CONNECT Kuniyuki Iwashima
@ 2026-11-07 23:56 ` Kuniyuki Iwashima
  2026-11-10 18:46   ` Matthieu Baerts
  2026-11-07 23:56 ` [PATCH v1 net 2/3] tcp: Drop any segments if DEFER_CONNECT is set Kuniyuki Iwashima
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-07 23:56 UTC (permalink / raw)
  To: Eric Dumazet, Neal Cardwell, David S . Miller, Jakub Kicinski,
	Paolo Abeni
  Cc: Yuchung Cheng, Wei Wang, Matthieu Baerts, Mat Martineau,
	Geliang Tang, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev

When TFO is enabled, tcp_v[46]_connect() sets sk->sk_state to
TCP_SYN_SENT and puts the socket into ehash without calling
tcp_connect().

Then, tcp_fastopen_defer_connect() sets DEFER_CONNECT, and
the first write()/sendmsg() will call tcp_connect() and send
SYN with data if possible.

If tcp_abort() kills a socket before the first write()/sendmsg(),
the unhashed socket can still call it and send SYN in TCP_CLOSE.

Also, DEFER_CONNECT is cleared after __inet_stream_connect().

So, blocking write()/sendmsg() can release lock_sock() in
inet_wait_for_connect() while DEFER_CONNECT is set, and
tcp_poll() could set EPOLLOUT even after SYN is sent.

Let's clear DEFER_CONNECT in tcp_abort() and right before
tcp_connect() in tcp_sendmsg_fastopen().

Note that the last argument of __inet_stream_connect() is
changed from is_sendmsg to defer_connect as DEFER_CONNECT
is already cleared when __inet_stream_connect() is called.

This change is also needed for the next patch.

Fixes: 19f6d3f3c842 ("net/tcp-fastopen: Add new API support")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/inet_common.h |  2 +-
 net/ipv4/af_inet.c        | 10 ++++++----
 net/ipv4/tcp.c            |  9 +++++++--
 net/mptcp/protocol.c      |  5 +++--
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/include/net/inet_common.h b/include/net/inet_common.h
index 3d747896be30..0f088a213c38 100644
--- a/include/net/inet_common.h
+++ b/include/net/inet_common.h
@@ -25,7 +25,7 @@ int inet_release(struct socket *sock);
 int inet_stream_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
 			int addr_len, int flags);
 int __inet_stream_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
-			  int addr_len, int flags, int is_sendmsg);
+			  int addr_len, int flags, bool defer_connect);
 int inet_dgram_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
 		       int addr_len, int flags);
 int inet_accept(struct socket *sock, struct socket *newsock,
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 7677b0343abd..8822bae439f8 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -629,7 +629,7 @@ static long inet_wait_for_connect(struct sock *sk, long timeo, int writebias)
  *	TCP 'magic' in here.
  */
 int __inet_stream_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
-			  int addr_len, int flags, int is_sendmsg)
+			  int addr_len, int flags, bool defer_connect)
 {
 	struct sock *sk = sock->sk;
 	int err;
@@ -664,8 +664,10 @@ int __inet_stream_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
 		err = -EISCONN;
 		goto out;
 	case SS_CONNECTING:
-		if (inet_test_bit(DEFER_CONNECT, sk))
-			err = is_sendmsg ? -EINPROGRESS : -EISCONN;
+		if (defer_connect)
+			err = -EINPROGRESS;
+		else if (inet_test_bit(DEFER_CONNECT, sk))
+			err = -EISCONN;
 		else
 			err = -EALREADY;
 		/* Fall out of switch with err, set for this state */
@@ -752,7 +754,7 @@ int inet_stream_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
 	int err;
 
 	lock_sock(sock->sk);
-	err = __inet_stream_connect(sock, uaddr, addr_len, flags, 0);
+	err = __inet_stream_connect(sock, uaddr, addr_len, flags, false);
 	release_sock(sock->sk);
 	return err;
 }
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 876f3e5f34c0..39ccbfcef136 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1057,6 +1057,7 @@ int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct inet_sock *inet = inet_sk(sk);
 	struct sockaddr *uaddr = msg->msg_name;
+	bool defer_connect = false;
 	int err, flags;
 
 	if (!(READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_fastopen) &
@@ -1076,6 +1077,9 @@ int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
 	tp->fastopen_req->uarg = uarg;
 
 	if (inet_test_bit(DEFER_CONNECT, sk)) {
+		inet_clear_bit(DEFER_CONNECT, sk);
+		defer_connect = true;
+
 		err = tcp_connect(sk);
 		/* Same failure procedure as in tcp_v4/6_connect */
 		if (err) {
@@ -1086,14 +1090,13 @@ int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
 	}
 	flags = (msg->msg_flags & MSG_DONTWAIT) ? O_NONBLOCK : 0;
 	err = __inet_stream_connect(sk->sk_socket, (struct sockaddr_unsized *)uaddr,
-				    msg->msg_namelen, flags, 1);
+				    msg->msg_namelen, flags, defer_connect);
 	/* fastopen_req could already be freed in __inet_stream_connect
 	 * if the connection times out or gets rst
 	 */
 	if (tp->fastopen_req) {
 		*copied = tp->fastopen_req->copied;
 		tcp_free_fastopen_req(tp);
-		inet_clear_bit(DEFER_CONNECT, sk);
 	}
 	return err;
 }
@@ -5148,6 +5151,8 @@ int tcp_abort(struct sock *sk, int err)
 		/* Don't race with userspace socket closes such as tcp_close. */
 		lock_sock(sk);
 
+	inet_clear_bit(DEFER_CONNECT, sk);
+
 	/* Avoid closing the same socket twice. */
 	if (sk->sk_state == TCP_CLOSE) {
 		if (!has_current_bpf_ctx())
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7a22e9388e0e..8968db58083e 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1920,10 +1920,12 @@ static int mptcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
 	msg->msg_flags = saved_flags;
 	release_sock(ssk);
 
+	inet_clear_bit(DEFER_CONNECT, sk);
+
 	/* do the blocking bits of inet_stream_connect outside the ssk socket lock */
 	if (ret == -EINPROGRESS && !(msg->msg_flags & MSG_DONTWAIT)) {
 		ret = __inet_stream_connect(sk->sk_socket, msg->msg_name,
-					    msg->msg_namelen, msg->msg_flags, 1);
+					    msg->msg_namelen, msg->msg_flags, true);
 
 		/* Keep the same behaviour of plain TCP: zero the copied bytes in
 		 * case of any error, except timeout or signal
@@ -1941,7 +1943,6 @@ static int mptcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
 			sk->sk_socket->state = SS_UNCONNECTED;
 		}
 	}
-	inet_clear_bit(DEFER_CONNECT, sk);
 
 	return ret;
 }
-- 
2.56.0.455.g6c5a503b38-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1 net 2/3] tcp: Drop any segments if DEFER_CONNECT is set.
  2026-11-07 23:56 [PATCH v1 net 0/3] tcp: Small fixes for DEFER_CONNECT Kuniyuki Iwashima
  2026-11-07 23:56 ` [PATCH v1 net 1/3] tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort() Kuniyuki Iwashima
@ 2026-11-07 23:56 ` Kuniyuki Iwashima
  2026-11-07 23:56 ` [PATCH v1 net 3/3] selftest: packetdrill: Add test for DEFER_CONNECT Kuniyuki Iwashima
  2026-11-11 22:12 ` [PATCH v1 net 0/3] tcp: Small fixes " patchwork-bot+netdevbpf
  3 siblings, 0 replies; 8+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-07 23:56 UTC (permalink / raw)
  To: Eric Dumazet, Neal Cardwell, David S . Miller, Jakub Kicinski,
	Paolo Abeni
  Cc: Yuchung Cheng, Wei Wang, Matthieu Baerts, Mat Martineau,
	Geliang Tang, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev, Chris Mason

From: Chris Mason <mason@kernel.org>

TCP Fast Open lets a client send data in its SYN. With
TCP_FASTOPEN_CONNECT and a cached Fast Open cookie, which the server
sent in the SYN-ACK of an earlier connection, the connection is set up
in two steps:

- connect() sends nothing, so that the SYN can carry the data of the
  first write(). It only sets DEFER_CONNECT and inserts the socket into
  the hash table in SYN_SENT.
- The first write() calls tcp_connect(), which runs tcp_connect_init()
  and sends the SYN.

A socket must not accept any segment before it has sent its SYN. But
nothing on the receive path checks DEFER_CONNECT. So until the write(),
the half-initialised socket processes segments like any socket in
SYN_SENT, and a forged SYN is accepted.

Similarly, ICMP with seq matching ISN is accepted in the state.
Before commit 57889a8f6d49 ("tcp: initialize send sequence numbers
before deferring TFO"), even seq 0 message was accepted.

Drop any segments if DEFER_CONNECT is set.

Fixes: 19f6d3f3c842 ("net/tcp-fastopen: Add new API support")
Assisted-by: LLM
Signed-off-by: Chris Mason <mason@kernel.org>
[kuniyu: revised DEFER_CONNECT tests and the commit message]
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/tcp_input.c | 9 +++++++++
 net/ipv4/tcp_ipv4.c  | 2 +-
 net/ipv6/tcp_ipv6.c  | 2 +-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 3fd0e215324d..70326323411c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -7252,6 +7252,15 @@ tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 		goto discard;
 
 	case TCP_SYN_SENT:
+		/* We sent nothing yet, so no segment belongs to this connection.
+		 * Send no RST: it would be built from this socket, whose TCP-AO
+		 * keys are not set up either.
+		 */
+		if (unlikely(inet_test_bit(DEFER_CONNECT, sk))) {
+			SKB_DR_SET(reason, NOT_SPECIFIED);
+			goto discard;
+		}
+
 		tp->rx_opt.saw_tstamp = 0;
 		tcp_mstamp_refresh(tp);
 		queued = tcp_rcv_synsent_state_process(sk, skb, th);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 0dc3b087665d..861d669a3f1f 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -541,7 +541,7 @@ int tcp_v4_err(struct sk_buff *skb, u32 info)
 		if (!(type == ICMP_DEST_UNREACH && code == ICMP_FRAG_NEEDED))
 			__NET_INC_STATS(net, LINUX_MIB_LOCKDROPPEDICMPS);
 	}
-	if (sk->sk_state == TCP_CLOSE)
+	if (sk->sk_state == TCP_CLOSE || inet_test_bit(DEFER_CONNECT, sk))
 		goto out;
 
 	if (static_branch_unlikely(&ip4_min_ttl)) {
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index b1dec5d86d63..5c7ed38f1f56 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -449,7 +449,7 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	if (sock_owned_by_user(sk) && type != ICMPV6_PKT_TOOBIG)
 		__NET_INC_STATS(net, LINUX_MIB_LOCKDROPPEDICMPS);
 
-	if (sk->sk_state == TCP_CLOSE)
+	if (sk->sk_state == TCP_CLOSE || inet_test_bit(DEFER_CONNECT, sk))
 		goto out;
 
 	if (static_branch_unlikely(&ip6_min_hopcount)) {
-- 
2.56.0.455.g6c5a503b38-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1 net 3/3] selftest: packetdrill: Add test for DEFER_CONNECT.
  2026-11-07 23:56 [PATCH v1 net 0/3] tcp: Small fixes for DEFER_CONNECT Kuniyuki Iwashima
  2026-11-07 23:56 ` [PATCH v1 net 1/3] tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort() Kuniyuki Iwashima
  2026-11-07 23:56 ` [PATCH v1 net 2/3] tcp: Drop any segments if DEFER_CONNECT is set Kuniyuki Iwashima
@ 2026-11-07 23:56 ` Kuniyuki Iwashima
  2026-11-11 22:12 ` [PATCH v1 net 0/3] tcp: Small fixes " patchwork-bot+netdevbpf
  3 siblings, 0 replies; 8+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-07 23:56 UTC (permalink / raw)
  To: Eric Dumazet, Neal Cardwell, David S . Miller, Jakub Kicinski,
	Paolo Abeni
  Cc: Yuchung Cheng, Wei Wang, Matthieu Baerts, Mat Martineau,
	Geliang Tang, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev

tcp_fastopen_client_defer-connect.pkt does the following:

  1. create a socket
  2. connect() + connect(AF_UNSPEC) to let packetdrill
     remember 4-tuple
  3. setsockopt(TCP_FASTOPEN_CONNECT) + connect()
  4. inject a forged SYN
  5. check sk->sk_state
  6. verify sendto() sends SYN

Without the previous patch, incoming simultaneous SYN is
accepted even before outgoing SYN is sent.

Before:

  [   27.941419] TCP: tcp_send_synack: wrong queue state
  tcp_fastopen_client_defer-connect.pkt:35: error handling packet:
    live packet field tcp_seq: expected: 501 (0x1f5) vs actual: 0 (0x0)
  script packet:  0.004460 . 501:501(0) ack 1
  actual packet:  0.004450 . 0:0(0) ack 4294961618 win 1050
  not ok 1 ipv4

Or if we remove Test 1, we can see ICMP is accepted:

  tcp_fastopen_client_defer-connect.pkt:33: runtime error in sendto
    call: Expected result 500 but got -1 with errno 103 (Software caused connection abort)
  not ok 1 ipv4

After:

  ok 1 ipv4
  ok 2 ipv6
  ok 3 ipv4-mapped-ipv6
  # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 .../tcp_fastopen_client_defer-connect.pkt     | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_fastopen_client_defer-connect.pkt

diff --git a/tools/testing/selftests/net/packetdrill/tcp_fastopen_client_defer-connect.pkt b/tools/testing/selftests/net/packetdrill/tcp_fastopen_client_defer-connect.pkt
new file mode 100644
index 000000000000..e90df498d935
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_fastopen_client_defer-connect.pkt
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Test that a TCP_FASTOPEN_CONNECT socket in DEFER_CONNECT state
+// ignores inbound TCP segments and ICMP errors before SYN is sent.
+//
+`./defaults.sh
+ ./set_sysctls.py /proc/sys/net/ipv4/tcp_timestamps=0`
+
+// Cache warmup: send a Fast Open cookie request
+    0 socket(..., SOCK_STREAM|SOCK_NONBLOCK, IPPROTO_TCP) = 3
+   +0 sendto(3, ..., 0, MSG_FASTOPEN, ..., ...) = -1 EINPROGRESS (Operation is now in progress)
+   +0 > S 0:0(0) <mss 1460,nop,nop,sackOK,nop,wscale 8,FO,nop,nop>
+   +0 < S. 123:123(0) ack 1 win 14600 <mss 1460,nop,nop,sackOK,nop,wscale 6,FO abcd1234,nop,nop>
+   +0 > . 1:1(0) ack 1
+   +0 close(3) = 0
+   +0 > F. 1:1(0) ack 1
+   +0 < F. 1:1(0) ack 2 win 92
+   +0 > . 2:2(0) ack 2
+
+// Test 1: Forged SYN during DEFER_CONNECT
+// connect() + connect(AF_UNSPEC) to let packetdrill remember
+// IP, ports, ISN to inject a forged SYN
+   +0 socket(..., SOCK_STREAM|SOCK_NONBLOCK, IPPROTO_TCP) = 4
+   +0 connect(4, ..., ...) = -1 EINPROGRESS (Operation is now in progress)
+   +0 > S 0:0(0) <mss 1460,nop,nop,sackOK,nop,wscale 8>
+   +0 connect(4, AF_UNSPEC, ...) = 0
+
+// set DEFER_CONNECT bit
+   +0 bind(4, ..., ...) = 0
+   +0 setsockopt(4, SOL_TCP, TCP_FASTOPEN_CONNECT, [1], 4) = 0
+   +0 connect(4, ..., ...) = 0
+
+// Forged SYN should not be accepted
+   +0 < 8080 > 8080 S 0:0(0) win 14600 <mss 1460,nop,nop,sackOK,nop,wscale 6>
+   +0 %{ assert tcpi_state == TCP_SYN_SENT, tcpi_state }%
+
+   +0 sendto(4, ..., 500, 0, ..., ...) = 500
+   +0 > S 0:500(500) <mss 1460,nop,nop,sackOK,nop,wscale 8,FO abcd1234,nop,nop>
+   +0 < S. 5678:5678(0) ack 501 win 14600 <mss 1460,nop,nop,sackOK,nop,wscale 6>
+   +0 > . 501:501(0) ack 1
+   +0 %{ assert tcpi_state == TCP_ESTABLISHED, tcpi_state }%
+   +0 close(4) = 0
+   +0 > F. 501:501(0) ack 1
+   +0 < R. 1:1(0) ack 502 win 0
+
+// Test 2: Forged ICMP error during DEFER_CONNECT
+   +0 socket(..., SOCK_STREAM|SOCK_NONBLOCK, IPPROTO_TCP) = 5
+   +0 setsockopt(5, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+   +0 bind(5, ..., ...) = 0
+   +0 connect(5, ..., ...) = -1 EINPROGRESS (Operation is now in progress)
+   +0 > S 0:0(0) <mss 1460,nop,nop,sackOK,nop,wscale 8>
+   +0 connect(5, AF_UNSPEC, ...) = 0
+   +0 setsockopt(5, SOL_TCP, TCP_FASTOPEN_CONNECT, [1], 4) = 0
+   +0 connect(5, ..., ...) = 0
+
+   +0 < icmp unreachable [3:3(0)]
+   +0 %{ assert tcpi_state == TCP_SYN_SENT, tcpi_state }%
+   +0 sendto(5, ..., 500, 0, ..., ...) = 500
+   +0 close(5) = 0
+
+`/tmp/sysctl_restore_${PPID}.sh`
-- 
2.56.0.455.g6c5a503b38-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 net 1/3] tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort().
  2026-11-07 23:56 ` [PATCH v1 net 1/3] tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort() Kuniyuki Iwashima
@ 2026-11-10 18:46   ` Matthieu Baerts
  2026-11-10 19:12     ` Kuniyuki Iwashima
  0 siblings, 1 reply; 8+ messages in thread
From: Matthieu Baerts @ 2026-11-10 18:46 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Eric Dumazet, Neal Cardwell, David S . Miller, Jakub Kicinski,
	Paolo Abeni, Yuchung Cheng, Wei Wang, Mat Martineau, Geliang Tang,
	Simon Horman, Kuniyuki Iwashima, netdev

Hi Kuniyuki,

On 08/11/2026 00:56, Kuniyuki Iwashima wrote:
> When TFO is enabled, tcp_v[46]_connect() sets sk->sk_state to
> TCP_SYN_SENT and puts the socket into ehash without calling
> tcp_connect().
> 
> Then, tcp_fastopen_defer_connect() sets DEFER_CONNECT, and
> the first write()/sendmsg() will call tcp_connect() and send
> SYN with data if possible.
> 
> If tcp_abort() kills a socket before the first write()/sendmsg(),
> the unhashed socket can still call it and send SYN in TCP_CLOSE.
> 
> Also, DEFER_CONNECT is cleared after __inet_stream_connect().
> 
> So, blocking write()/sendmsg() can release lock_sock() in
> inet_wait_for_connect() while DEFER_CONNECT is set, and
> tcp_poll() could set EPOLLOUT even after SYN is sent.
> 
> Let's clear DEFER_CONNECT in tcp_abort() and right before
> tcp_connect() in tcp_sendmsg_fastopen().

Thank you for the fix!

It looks like we can still have the race with MPTCP:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261107235720.3129212-1-kuniyu%40google.com

But that's not blocking this series here, so for the MPTCP part:

Acked-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>


Just to avoid a deadlock, are you planning to fix the issue on MPTCP
side as well? If not, I can look at that later on when I can.

(Perhaps by checking if the msk and ssk's DEFER_CONNECT flag is no
longer aligned in mptcp_sendmsg_fastopen(), under the ssk lock? If not,
clear the msk one, report the error, release the lock and return? BTW,
it looks like we also forgot to clear the DEFER_CONNECT flag in
mptcp_disconnect() as well...)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 net 1/3] tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort().
  2026-11-10 18:46   ` Matthieu Baerts
@ 2026-11-10 19:12     ` Kuniyuki Iwashima
  2026-11-10 19:14       ` Matthieu Baerts
  0 siblings, 1 reply; 8+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-10 19:12 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: Eric Dumazet, Neal Cardwell, David S . Miller, Jakub Kicinski,
	Paolo Abeni, Yuchung Cheng, Wei Wang, Mat Martineau, Geliang Tang,
	Simon Horman, Kuniyuki Iwashima, netdev

On Tue, Nov 10, 2026 at 10:46 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi Kuniyuki,
>
> On 08/11/2026 00:56, Kuniyuki Iwashima wrote:
> > When TFO is enabled, tcp_v[46]_connect() sets sk->sk_state to
> > TCP_SYN_SENT and puts the socket into ehash without calling
> > tcp_connect().
> >
> > Then, tcp_fastopen_defer_connect() sets DEFER_CONNECT, and
> > the first write()/sendmsg() will call tcp_connect() and send
> > SYN with data if possible.
> >
> > If tcp_abort() kills a socket before the first write()/sendmsg(),
> > the unhashed socket can still call it and send SYN in TCP_CLOSE.
> >
> > Also, DEFER_CONNECT is cleared after __inet_stream_connect().
> >
> > So, blocking write()/sendmsg() can release lock_sock() in
> > inet_wait_for_connect() while DEFER_CONNECT is set, and
> > tcp_poll() could set EPOLLOUT even after SYN is sent.
> >
> > Let's clear DEFER_CONNECT in tcp_abort() and right before
> > tcp_connect() in tcp_sendmsg_fastopen().
>
> Thank you for the fix!
>
> It looks like we can still have the race with MPTCP:
>
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261107235720.3129212-1-kuniyu%40google.com
>
> But that's not blocking this series here, so for the MPTCP part:
>
> Acked-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Thanks Matt !

>
>
> Just to avoid a deadlock, are you planning to fix the issue on MPTCP
> side as well? If not, I can look at that later on when I can.

I would appreciate it if you could look into it.

>
> (Perhaps by checking if the msk and ssk's DEFER_CONNECT flag is no
> longer aligned in mptcp_sendmsg_fastopen(), under the ssk lock? If not,
> clear the msk one, report the error, release the lock and return? BTW,
> it looks like we also forgot to clear the DEFER_CONNECT flag in
> mptcp_disconnect() as well...)

Sounds good.

Thanks !

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 net 1/3] tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort().
  2026-11-10 19:12     ` Kuniyuki Iwashima
@ 2026-11-10 19:14       ` Matthieu Baerts
  0 siblings, 0 replies; 8+ messages in thread
From: Matthieu Baerts @ 2026-11-10 19:14 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Eric Dumazet, Neal Cardwell, David S . Miller, Jakub Kicinski,
	Paolo Abeni, Yuchung Cheng, Wei Wang, Mat Martineau, Geliang Tang,
	Simon Horman, Kuniyuki Iwashima, netdev

On 10/11/2026 20:12, Kuniyuki Iwashima wrote:
> On Tue, Nov 10, 2026 at 10:46 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>>
>> Hi Kuniyuki,
>>
>> On 08/11/2026 00:56, Kuniyuki Iwashima wrote:
>>> When TFO is enabled, tcp_v[46]_connect() sets sk->sk_state to
>>> TCP_SYN_SENT and puts the socket into ehash without calling
>>> tcp_connect().
>>>
>>> Then, tcp_fastopen_defer_connect() sets DEFER_CONNECT, and
>>> the first write()/sendmsg() will call tcp_connect() and send
>>> SYN with data if possible.
>>>
>>> If tcp_abort() kills a socket before the first write()/sendmsg(),
>>> the unhashed socket can still call it and send SYN in TCP_CLOSE.
>>>
>>> Also, DEFER_CONNECT is cleared after __inet_stream_connect().
>>>
>>> So, blocking write()/sendmsg() can release lock_sock() in
>>> inet_wait_for_connect() while DEFER_CONNECT is set, and
>>> tcp_poll() could set EPOLLOUT even after SYN is sent.
>>>
>>> Let's clear DEFER_CONNECT in tcp_abort() and right before
>>> tcp_connect() in tcp_sendmsg_fastopen().
>>
>> Thank you for the fix!
>>
>> It looks like we can still have the race with MPTCP:
>>
>> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261107235720.3129212-1-kuniyu%40google.com
>>
>> But that's not blocking this series here, so for the MPTCP part:
>>
>> Acked-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> Thanks Matt !
> 
>>
>>
>> Just to avoid a deadlock, are you planning to fix the issue on MPTCP
>> side as well? If not, I can look at that later on when I can.
> 
> I would appreciate it if you could look into it.

Sure, I will wait for this series to be applied first.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 net 0/3] tcp: Small fixes for DEFER_CONNECT.
  2026-11-07 23:56 [PATCH v1 net 0/3] tcp: Small fixes for DEFER_CONNECT Kuniyuki Iwashima
                   ` (2 preceding siblings ...)
  2026-11-07 23:56 ` [PATCH v1 net 3/3] selftest: packetdrill: Add test for DEFER_CONNECT Kuniyuki Iwashima
@ 2026-11-11 22:12 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-11-11 22:12 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: edumazet, ncardwell, davem, kuba, pabeni, ycheng, weiwan, matttbe,
	martineau, geliang, horms, kuni1840, netdev

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat,  7 Nov 2026 23:56:07 +0000 you wrote:
> Chris Mason reported a bug that TFO client socket can accept
> segments even before it sends SYN.
> 
> He provided a patch too, but it needed some adjustments (Patch 2).
> 
> Patch 1 is a prereq fix for Patch 2, and Patch 3 is packetdrill
> test for the reported bug.
> 
> [...]

Here is the summary with links:
  - [v1,net,1/3] tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort().
    https://git.kernel.org/netdev/net/c/93723db43e51
  - [v1,net,2/3] tcp: Drop any segments if DEFER_CONNECT is set.
    https://git.kernel.org/netdev/net/c/44809e491779
  - [v1,net,3/3] selftest: packetdrill: Add test for DEFER_CONNECT.
    https://git.kernel.org/netdev/net/c/e6f5a8d08005

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-11-11 22:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-07 23:56 [PATCH v1 net 0/3] tcp: Small fixes for DEFER_CONNECT Kuniyuki Iwashima
2026-11-07 23:56 ` [PATCH v1 net 1/3] tcp: Clear DEFER_CONNECT before tcp_connect() and in tcp_abort() Kuniyuki Iwashima
2026-11-10 18:46   ` Matthieu Baerts
2026-11-10 19:12     ` Kuniyuki Iwashima
2026-11-10 19:14       ` Matthieu Baerts
2026-11-07 23:56 ` [PATCH v1 net 2/3] tcp: Drop any segments if DEFER_CONNECT is set Kuniyuki Iwashima
2026-11-07 23:56 ` [PATCH v1 net 3/3] selftest: packetdrill: Add test for DEFER_CONNECT Kuniyuki Iwashima
2026-11-11 22:12 ` [PATCH v1 net 0/3] tcp: Small fixes " patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox