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@xxxxxxxxxxx> 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@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;
> +       }

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
>