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

[Asbjørn Sloth Tønnesen]

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 <knielsen@xxxxxxxxxxxxxxx>
Link: https://lore.kernel.org/87sf0ldk41.fsf@xxxxxxxxxxxxxxxxxxx # [1]
Fixes: c82199061009 ("task_work: remove fifo ordering guarantee")
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Asbjørn Sloth Tønnesen <ast@xxxxxxxxxxx>
---

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