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

[PATCH net v2] 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 significantly increases the probability for a race to occur
between a TCP handshake and the closing of the listening socket.

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.

The call to __inet_inherit_port() returns -ENOENT because
inet_csk(sk)->icsk_bind_hash is NULL, after inet_put_port() has been
called by tcp_set_state(sk, TCP_CLOSE).

This patch adds a check on the return value of the __inet_inherit_port()
call, and jumps to a new label, where it resets the new connection,
before proceeding with the put_and_exit label.

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.

Additionally the race can be reproduced back to v3.6, by backporting
the blamed commit. Beyond v3.6 there are too many conflicts.

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

While the blamed commit properly only made the race observable, then
the commit that made the race possible is less realistic to track down,
and I don't think is worth the time to continue to the search for it
somewhere before v3.6.

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.

Changelog:
v2:
  - Use return from __inet_inherit_port() to trigger send_reply()
  - Use req->rsk_ops->send_reset.
  - Clarity commit message, and update to reflect the changes.
  (Thanks Kuniyuki)
v1: https://lore.kernel.org/20260807194513.1263310-1-ast@fiberby.net

 net/ipv4/tcp_ipv4.c | 8 +++++++-
 net/ipv6/tcp_ipv6.c | 8 +++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index b8887cdd66c5..e0fba4579a57 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1690,6 +1690,7 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
 	int l3index;
 #endif
 	struct ip_options_rcu *inet_opt;
+	int ret;
 
 	if (sk_acceptq_is_full(sk))
 		goto exit_overflow;
@@ -1756,7 +1757,10 @@ 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 (__inet_inherit_port(sk, newsk) < 0)
+	ret = __inet_inherit_port(sk, newsk);
+	if (ret == -ENOENT)
+		goto send_reset_and_exit;
+	else if (ret < 0)
 		goto put_and_exit;
 	*own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash),
 				       &found_dup_sk);
@@ -1784,6 +1788,8 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
 exit:
 	tcp_listendrop(sk);
 	return NULL;
+send_reset_and_exit:
+	req->rsk_ops->send_reset(newsk, skb, SK_RST_REASON_TCP_STATE);
 put_and_exit:
 	newinet->inet_opt = NULL;
 	inet_csk_prepare_forced_close(newsk);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 9e9155b1b3aa..021c93de2273 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1400,6 +1400,7 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
 	int l3index;
 #endif
 	struct flowi6 fl6;
+	int ret;
 
 	if (skb->protocol == htons(ETH_P_IP))
 		return tcp_v4_syn_recv_sock(sk, skb, req, dst,
@@ -1512,7 +1513,10 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
 		goto put_and_exit; /* OOM */
 #endif
 
-	if (__inet_inherit_port(sk, newsk) < 0)
+	ret = __inet_inherit_port(sk, newsk);
+	if (ret == -ENOENT)
+		goto send_reset_and_exit;
+	else if (ret < 0)
 		goto put_and_exit;
 	*own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash),
 				       &found_dup_sk);
@@ -1547,6 +1551,8 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
 exit:
 	tcp_listendrop(sk);
 	return NULL;
+send_reset_and_exit:
+	req->rsk_ops->send_reset(newsk, skb, SK_RST_REASON_TCP_STATE);
 put_and_exit:
 	inet_csk_prepare_forced_close(newsk);
 	tcp_done(newsk);

base-commit: dd057113ac7ba5bdd2aed3d9405305911152f911
-- 
2.55.0

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

[Kuniyuki Iwashima]

On Mon, Aug 10, 2026 at 1:57 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.
>
> This significantly increases the probability for a race to occur
> between a TCP handshake and the closing of the listening socket.

I looked into the repro, and the description above is too specific
to your reproducer working on localhost.

In your repro, a server creates a listening socket, accept()s
some child connections to get multiple file descriptors, and
exit()s the process, which close()s file descriptors from listener
(smaller fd) to child fds.

The last fput() queues each fd to the delayed work, which is
currently executed in LIFO ordering.

The child fd sends RST to the client and later the listener is
TCP_CLOSE'd.

On the client side, it tries to detect the server's exit() by checking
RST on the established connections (child fds on listener), and then
creates a new connection, which creates a reqsk with the alive listener
but the last ACK will hit TCP_CLOSE'd listener.

I checked the last ACK generated by the repro fails at
__inet_inherit_port() OR inet_csk_reqsk_queue_add().

That's why reverting c82199061009 prevents reproduction because
it's too late to create a new connection when the client notices the
listener's exit() via RST of child connections; new connection's SYN
always gets RST with the revert.

So, I would drop the description above and simply explain it like

"When __inet_inherit_port() returns -ENOENT, the new connection
 is dropped silently."


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

or inet_csk_reqsk_queue_add() should have sent RST.

> and been notified when
> inet_csk_listen_stop() was run, and the client would have got a reset.
>
> The call to __inet_inherit_port() returns -ENOENT because
> inet_csk(sk)->icsk_bind_hash is NULL, after inet_put_port() has been
> called by tcp_set_state(sk, TCP_CLOSE).
>
> This patch adds a check on the return value of the __inet_inherit_port()
> call, and jumps to a new label, where it resets the new connection,
> before proceeding with the put_and_exit label.
>
> The blamed commit was identified by testing on ancient Debian stable

This commit should be more appropriate since __inet_inherit_port()
did not fail (except for OOM) before this commit:

Fixes: c2f34a65a61c ("tcp/dccp: fix potential NULL deref in
__inet_inherit_port()")


> 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.
>
> Additionally the race can be reproduced back to v3.6, by backporting
> the blamed commit. Beyond v3.6 there are too many conflicts.
>
> 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>
> ---
>
> While the blamed commit properly only made the race observable, then
> the commit that made the race possible is less realistic to track down,
> and I don't think is worth the time to continue to the search for it
> somewhere before v3.6.
>
> 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.
>
> Changelog:
> v2:
>   - Use return from __inet_inherit_port() to trigger send_reply()
>   - Use req->rsk_ops->send_reset.
>   - Clarity commit message, and update to reflect the changes.
>   (Thanks Kuniyuki)
> v1: https://lore.kernel.org/20260807194513.1263310-1-ast@fiberby.net
>
>  net/ipv4/tcp_ipv4.c | 8 +++++++-
>  net/ipv6/tcp_ipv6.c | 8 +++++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index b8887cdd66c5..e0fba4579a57 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1690,6 +1690,7 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
>         int l3index;
>  #endif
>         struct ip_options_rcu *inet_opt;
> +       int ret;
>
>         if (sk_acceptq_is_full(sk))
>                 goto exit_overflow;
> @@ -1756,7 +1757,10 @@ 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 (__inet_inherit_port(sk, newsk) < 0)
> +       ret = __inet_inherit_port(sk, newsk);
> +       if (ret == -ENOENT)
> +               goto send_reset_and_exit;
> +       else if (ret < 0)
>                 goto put_and_exit;

nit: Please move the -ENOENT branch inside (ret < 0)

if (unlikely(ret < 0) {
        if (ret == -ENOENT)
                goto send_reset_and_exit;
        goto put_and_exit;
}


>         *own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash),
>                                        &found_dup_sk);
> @@ -1784,6 +1788,8 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
>  exit:
>         tcp_listendrop(sk);
>         return NULL;
> +send_reset_and_exit:
> +       req->rsk_ops->send_reset(newsk, skb, SK_RST_REASON_TCP_STATE);

Sorry, I think this can be direct call, tcp_v4_send_reset().

Also please pass the listener instead of newsk, it's more
like a call in tcp_check_req().

Same remarks for IPv6.

Thanks

>  put_and_exit:
>         newinet->inet_opt = NULL;
>         inet_csk_prepare_forced_close(newsk);
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 9e9155b1b3aa..021c93de2273 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1400,6 +1400,7 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
>         int l3index;
>  #endif
>         struct flowi6 fl6;
> +       int ret;
>
>         if (skb->protocol == htons(ETH_P_IP))
>                 return tcp_v4_syn_recv_sock(sk, skb, req, dst,
> @@ -1512,7 +1513,10 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
>                 goto put_and_exit; /* OOM */
>  #endif
>
> -       if (__inet_inherit_port(sk, newsk) < 0)
> +       ret = __inet_inherit_port(sk, newsk);
> +       if (ret == -ENOENT)
> +               goto send_reset_and_exit;
> +       else if (ret < 0)
>                 goto put_and_exit;
>         *own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash),
>                                        &found_dup_sk);
> @@ -1547,6 +1551,8 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
>  exit:
>         tcp_listendrop(sk);
>         return NULL;
> +send_reset_and_exit:
> +       req->rsk_ops->send_reset(newsk, skb, SK_RST_REASON_TCP_STATE);
>  put_and_exit:
>         inet_csk_prepare_forced_close(newsk);
>         tcp_done(newsk);
>
> base-commit: dd057113ac7ba5bdd2aed3d9405305911152f911
> --
> 2.55.0
>

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

[Asbjørn Sloth Tønnesen]

On 8/11/26 6:37 AM, Kuniyuki Iwashima wrote:
> On Mon, Aug 10, 2026 at 1:57 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.
>>
>> This significantly increases the probability for a race to occur
>> between a TCP handshake and the closing of the listening socket.
> 
> I looked into the repro, and the description above is too specific
> to your reproducer working on localhost.
> 
> In your repro, a server creates a listening socket, accept()s
> some child connections to get multiple file descriptors, and
> exit()s the process, which close()s file descriptors from listener
> (smaller fd) to child fds.
> 
> The last fput() queues each fd to the delayed work, which is
> currently executed in LIFO ordering.
> 
> The child fd sends RST to the client and later the listener is
> TCP_CLOSE'd.
> 
> On the client side, it tries to detect the server's exit() by checking
> RST on the established connections (child fds on listener), and then
> creates a new connection, which creates a reqsk with the alive listener
> but the last ACK will hit TCP_CLOSE'd listener.
> 
> I checked the last ACK generated by the repro fails at
> __inet_inherit_port() OR inet_csk_reqsk_queue_add().
> 
> That's why reverting c82199061009 prevents reproduction because
> it's too late to create a new connection when the client notices the
> listener's exit() via RST of child connections; new connection's SYN
> always gets RST with the revert.
> 
> So, I would drop the description above and simply explain it like
> 
> "When __inet_inherit_port() returns -ENOENT, the new connection
>   is dropped silently."
> 
> 
>> 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,
> 
> or inet_csk_reqsk_queue_add() should have sent RST.
> 
>> and been notified when
>> inet_csk_listen_stop() was run, and the client would have got a reset.
>>
>> The call to __inet_inherit_port() returns -ENOENT because
>> inet_csk(sk)->icsk_bind_hash is NULL, after inet_put_port() has been
>> called by tcp_set_state(sk, TCP_CLOSE).
>>
>> This patch adds a check on the return value of the __inet_inherit_port()
>> call, and jumps to a new label, where it resets the new connection,
>> before proceeding with the put_and_exit label.
>>
>> The blamed commit was identified by testing on ancient Debian stable
> 
> This commit should be more appropriate since __inet_inherit_port()
> did not fail (except for OOM) before this commit:
> 
> Fixes: c2f34a65a61c ("tcp/dccp: fix potential NULL deref in
> __inet_inherit_port()")

Thank you, for you detailed analysis. I didn't find this because I didn't
notice the error condition changing over time, and this being a v4.4 commit
while I was looking for earlier commits.


>> 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.
>>
>> Additionally the race can be reproduced back to v3.6, by backporting
>> the blamed commit. Beyond v3.6 there are too many conflicts.
>>
>> 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>
>> ---
>>
>> While the blamed commit properly only made the race observable, then
>> the commit that made the race possible is less realistic to track down,
>> and I don't think is worth the time to continue to the search for it
>> somewhere before v3.6.
>>
>> 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.
>>
>> Changelog:
>> v2:
>>    - Use return from __inet_inherit_port() to trigger send_reply()
>>    - Use req->rsk_ops->send_reset.
>>    - Clarity commit message, and update to reflect the changes.
>>    (Thanks Kuniyuki)
>> v1: https://lore.kernel.org/20260807194513.1263310-1-ast@fiberby.net
>>
>>   net/ipv4/tcp_ipv4.c | 8 +++++++-
>>   net/ipv6/tcp_ipv6.c | 8 +++++++-
>>   2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>> index b8887cdd66c5..e0fba4579a57 100644
>> --- a/net/ipv4/tcp_ipv4.c
>> +++ b/net/ipv4/tcp_ipv4.c
>> @@ -1690,6 +1690,7 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
>>          int l3index;
>>   #endif
>>          struct ip_options_rcu *inet_opt;
>> +       int ret;
>>
>>          if (sk_acceptq_is_full(sk))
>>                  goto exit_overflow;
>> @@ -1756,7 +1757,10 @@ 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 (__inet_inherit_port(sk, newsk) < 0)
>> +       ret = __inet_inherit_port(sk, newsk);
>> +       if (ret == -ENOENT)
>> +               goto send_reset_and_exit;
>> +       else if (ret < 0)
>>                  goto put_and_exit;
> 
> nit: Please move the -ENOENT branch inside (ret < 0)
> 
> if (unlikely(ret < 0) {
>          if (ret == -ENOENT)
>                  goto send_reset_and_exit;
>          goto put_and_exit;
> }
> 
> 
>>          *own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash),
>>                                         &found_dup_sk);
>> @@ -1784,6 +1788,8 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
>>   exit:
>>          tcp_listendrop(sk);
>>          return NULL;
>> +send_reset_and_exit:
>> +       req->rsk_ops->send_reset(newsk, skb, SK_RST_REASON_TCP_STATE);
> 
> Sorry, I think this can be direct call, tcp_v4_send_reset().
> 
> Also please pass the listener instead of newsk, it's more
> like a call in tcp_check_req().
> 
> Same remarks for IPv6.

Thank you for your very detailed and insightful review.

I have v3 ready, and will post it when the 24 hours are up.

-- 
pw-bot: cr