[PATCH net] tcp: reset late connection after listening socket close

Asbjørn Sloth Tønnesen <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
In commit c82199061009 ("task_work: remove fifo ordering guarantee")
Eric removed the ordering guarantee, thereby changing it from a
guaranteed FIFO to currently LIFO ordering, in an effort to reduce
jitter.

This allows for a TCP handshake to complete, after the listening socket
has been closed, and after inet_csk_listen_stop() has been run.

In that case the client sees the connection as ESTABLISHED, however in
tcp_v{4,6}_syn_recv_sock() the call to __inet_inherit_port() returns
-ENOENT, and the new connection is dropped silently by put_and_exit.

A client may therefore hang indefinitely on a blocking read() if the
used data communication protocol is initiated by the server, like SMTP
and the reporter[1]'s MariaDB protocol both are.

Had the new connection been processed before the listening socket was
closed, it would have been in the accept queue, and been notified when
inet_csk_listen_stop() was run, and the client would have got a reset.

This patch adds a check on the state of the listening socket, and
resets the new connection, before discarding it with put_and_exit.
The check is placed just before it would fail in __inet_inherit_port(),
and just after TCP MD5 and AO options have been copied to newsk.

The blamed commit was identified by testing on ancient Debian stable
releases to get a rough scope of where to look, then identifying in
which release between v3.16 and v4.19 it broke, and finally bisecting
v4.2..v4.3 on a fresh Debian then-stable (jessie) VM with tooling from
that era, and confirmed by reverting it from v4.3, v5.10.y and net.

Reproducer:
  https://files.fiberby.net/ast/2026/kernel/socket_teardown_test.c

Reported-by: Kristian Nielsen <[email protected]>
Link: https://lore.kernel.org/[email protected] # [1]
Fixes: c82199061009 ("task_work: remove fifo ordering guarantee")
Cc: <[email protected]>
Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
---

I'm not submitting a selftest at this time, as I have only found an
efficient way to often detect the issue, not disprove it, and I would
need more test data from different systems, before I can reliably
disprove it with a low runtime budget, without getting false negatives.

The check could also be "sk->sk_state != TCP_LISTEN".  I have only
seen TCP_LISTEN and TCP_CLOSE at this location during my testing.

IMHO socket migration is out of scope for this patch, see commit
54b92e841937 ("tcp: Migrate TCP_ESTABLISHED/TCP_SYN_RECV sockets in
accept queues."), as it would complicate backporting to stable.

I have not yet tested that MD5/AO works, I guess that would involve
extending my reproducer to place client and server in distinct netns.

 net/ipv4/tcp_ipv4.c | 5 +++++
 net/ipv6/tcp_ipv6.c | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index b8887cdd66c57..45d9a7e40e951 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1756,6 +1756,11 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
 		goto put_and_exit; /* OOM, release back memory */
 #endif
 
+	if (sk->sk_state == TCP_CLOSE) {
+		tcp_v4_send_reset(newsk, skb, SK_RST_REASON_TCP_STATE);
+		goto put_and_exit;
+	}
+
 	if (__inet_inherit_port(sk, newsk) < 0)
 		goto put_and_exit;
 	*own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash),
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 9e9155b1b3aa7..c7aa3c6b1c314 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1512,6 +1512,11 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
 		goto put_and_exit; /* OOM */
 #endif
 
+	if (sk->sk_state == TCP_CLOSE) {
+		tcp_v6_send_reset(newsk, skb, SK_RST_REASON_TCP_STATE);
+		goto put_and_exit;
+	}
+
 	if (__inet_inherit_port(sk, newsk) < 0)
 		goto put_and_exit;
 	*own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash),

base-commit: 594d905195024b228c962627ae5ae7c17bd582a4
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.