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

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

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

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

[Kuniyuki Iwashima]

On Fri, Aug 7, 2026 at 12:47 PM Asbjørn Sloth Tønnesen <ast@fiberby.net> wrote:
>
> 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.

Could you elaborate a bit more on how c82199061009 introduced the issue?

I didn't quite see the connection between the first and second paragraphs.

The listener's state could change at any time on SMP.

>
> 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.

Note that if __inet_inherit_port() passes, inet_csk_reqsk_queue_add()
calls inet_child_forget().


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

What happens if the state changes here ?


> +
>         if (__inet_inherit_port(sk, newsk) < 0)
>                 goto put_and_exit;

-ENOENT should be checked to jump to a new label and
call req->rsk_ops->send_reset() there.


>         *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
>

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

[Asbjørn Sloth Tønnesen]

Thank you for your review.

On 8/7/26 9:05 PM, Kuniyuki Iwashima wrote:
> On Fri, Aug 7, 2026 at 12:47 PM Asbjørn Sloth Tønnesen <ast@fiberby.net> wrote:
>> 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.
> 
> Could you elaborate a bit more on how c82199061009 introduced the issue?
> 
> I didn't quite see the connection between the first and second paragraphs.

Sorry, I didn't elaborate more, it was already getting quite long.  I had also
hoped that the bisection had yielded a stronger and clearer connection, or that
this commit had been blamed before.

My understanding is that due to the LIFO (Last In First Out) ordering, the order
and thereby timing sequence is reversed. I haven't looked too deeply into the
specifics here, like which items have been scheduled together etc., as I don't
suggest reverting the blamed commit.  I have rather focused on fixing this TCP
quirk, and make it work with LIFO ordering, and not restoring the FIFO ordering.
I have mainly used netstat counters to debug the path taken by the affected
connections, and identify where and why the connections died.

I was also skeptical of the bisection result.  Did my reproducer have a false
negative?  However by reverting the commit, and thereby re-adding the order
reversal logic I have reliably proved that the blamed commit is deeply connected
to the experienced symptom, on everything I have tested post-v4.3 (the revert has
a trivial conflict on v5.10, but can then cleanly be cherry-picked on the current
net tree).  I have tested with my reproducer on v4.3, v4.4, v5.10.y and net, both
that my reproducer detects the issue in the tagged release, and that the issue is
gone with the revert.  I tested v5.10.y as the original report was against v5.10.


> The listener's state could change at any time on SMP.

True, but it won't schedule work items, and execute them in reverse order, which
IMHO makes this race condition a lot easier to hit with LIFO ordering.


>> 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.
> 
> Note that if __inet_inherit_port() passes, inet_csk_reqsk_queue_add()
> calls inet_child_forget().

__inet_inherit_port() consistently exits early with -ENOENT, because
inet_csk(sk)->icsk_bind_hash is NULL for these ill timed connections.

inet_csk(sk)->icsk_bind_hash is NULL, because inet_put_port() has been
called.

inet_put_port() has been called by tcp_set_state(TCP_CLOSE).

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

[Asbjørn Sloth Tønnesen]

On 8/7/26 9:05 PM, Kuniyuki Iwashima wrote:
> On Fri, Aug 7, 2026 at 12:47 PM Asbjørn Sloth Tønnesen <ast@fiberby.net> wrote:
>> [..]
>> +       if (sk->sk_state == TCP_CLOSE) {
>> +               tcp_v4_send_reset(newsk, skb, SK_RST_REASON_TCP_STATE);
>> +               goto put_and_exit;
>> +       }
> 
> What happens if the state changes here ?
> 
> 
>> +
>>          if (__inet_inherit_port(sk, newsk) < 0)
>>                  goto put_and_exit;
> 
> -ENOENT should be checked to jump to a new label and
> call req->rsk_ops->send_reset() there.

I will do that in v2, thanks.