* [PATCH net v3] tcp: reset late connection after listening socket close
@ 2026-08-11 21:09 Asbjørn Sloth Tønnesen
2026-08-12 3:16 ` Kuniyuki Iwashima
2026-08-17 19:55 ` Jakub Kicinski
0 siblings, 2 replies; 3+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2026-08-11 21:09 UTC (permalink / raw)
To: Eric Dumazet, Neal Cardwell, Kuniyuki Iwashima
Cc: Asbjørn Sloth Tønnesen, David S. Miller, Jakub Kicinski,
Paolo Abeni, Simon Horman, netdev, linux-kernel, Kristian Nielsen,
stable
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 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 either have been added to the accept queue, or
inet_csk_reqsk_queue_add() should have sent RST.
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 -ENOENT handling to both __inet_inherit_port() call
sites, and ensures that RST is sent before the connection is dropped.
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: c2f34a65a61c ("tcp/dccp: fix potential NULL deref in __inet_inherit_port()")
Cc: <stable@vger.kernel.org>
Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
Changelog:
v3:
- Rewrite commit message around fixing commit c2f34a65a61c.
- Call tcp_v{4,6}_send_reset() directly again (but with sk, not newsk).
- Nest the two return value checks, and wrap in unlikely().
(Thanks again Kuniyuki)
v2: https://lore.kernel.org/20260810205642.1611338-1-ast@fiberby.net
- 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 | 9 ++++++++-
net/ipv6/tcp_ipv6.c | 9 ++++++++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index b8887cdd66c5..9a14c2e56ec3 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,8 +1757,12 @@ 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 (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);
if (likely(*own_req)) {
@@ -1784,6 +1789,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:
+ tcp_v4_send_reset(sk, 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..ecb0b405703c 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,8 +1513,12 @@ 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 (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);
if (*own_req) {
@@ -1547,6 +1552,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:
+ tcp_v6_send_reset(sk, skb, SK_RST_REASON_TCP_STATE);
put_and_exit:
inet_csk_prepare_forced_close(newsk);
tcp_done(newsk);
base-commit: cba9ccb47e9fa4cc77692fb896cc5ab57a667882
--
2.55.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net v3] tcp: reset late connection after listening socket close
2026-08-11 21:09 [PATCH net v3] tcp: reset late connection after listening socket close Asbjørn Sloth Tønnesen
@ 2026-08-12 3:16 ` Kuniyuki Iwashima
2026-08-17 19:55 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Kuniyuki Iwashima @ 2026-08-12 3:16 UTC (permalink / raw)
To: Asbjørn Sloth Tønnesen
Cc: Eric Dumazet, Neal Cardwell, David S. Miller, Jakub Kicinski,
Paolo Abeni, Simon Horman, netdev, linux-kernel, Kristian Nielsen,
stable
On Tue, Aug 11, 2026 at 2:10 PM Asbjørn Sloth Tønnesen <ast@fiberby.net> wrote:
>
> 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 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 either have been added to the accept queue, or
> inet_csk_reqsk_queue_add() should have sent RST.
>
> 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 -ENOENT handling to both __inet_inherit_port() call
> sites, and ensures that RST is sent before the connection is dropped.
>
> 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: c2f34a65a61c ("tcp/dccp: fix potential NULL deref in __inet_inherit_port()")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Thanks !
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net v3] tcp: reset late connection after listening socket close
2026-08-11 21:09 [PATCH net v3] tcp: reset late connection after listening socket close Asbjørn Sloth Tønnesen
2026-08-12 3:16 ` Kuniyuki Iwashima
@ 2026-08-17 19:55 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-08-17 19:55 UTC (permalink / raw)
To: ast
Cc: Jakub Kicinski, edumazet, ncardwell, kuniyu, davem, pabeni, horms,
netdev, linux-kernel, knielsen, stable
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
tcp: reset late connection after listening socket close
When __inet_inherit_port() returns -ENOENT because the listener already
released its bound port, the child socket was dropped silently and the peer
could hang forever. This patch adds an -ENOENT case to both
tcp_v{4,6}_syn_recv_sock() that sends an RST with the listening socket
before dropping the child.
Link: https://lore.kernel.org/87sf0ldk41.fsf@urd.knielsen-hq.org # [1]
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index b8887cdd66c5..9a14c2e56ec3 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
[ ... ]
> @@ -1756,8 +1757,12 @@ 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 (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);
> if (likely(*own_req)) {
[ ... ]
> @@ -1784,6 +1789,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:
> + tcp_v4_send_reset(sk, skb, SK_RST_REASON_TCP_STATE);
[Severity: High]
Can this oops when the listener has TCP-AO keys installed?
As the commit message explains, this label is reached because
tcp_set_state(sk, TCP_CLOSE) already ran inet_put_port(), so sk->sk_state
is normally TCP_CLOSE here, and the socket was never established.
tcp_v4_send_reset() then feeds that socket to the AO code:
net/ipv4/tcp_ipv4.c:tcp_v4_send_reset() {
...
if (aoh && tcp_v4_ao_sign_reset(sk, skb, aoh, &arg, &rep.th, rep.opt))
return;
...
}
and tcp_ao_prepare_reset() only special-cases listen/request sockets:
net/ipv4/tcp_ao.c:tcp_ao_prepare_reset() {
...
if ((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_NEW_SYN_RECV)) {
...
} else {
*key = tcp_ao_established_key(sk, ao_info, aoh->rnext_keyid, -1);
if (!*key)
return -ENOENT;
*traffic_key = snd_other_key(*key);
rnext_key = READ_ONCE(ao_info->rnext_key);
*keyid = rnext_key->rcvid;
...
}
TCP_CLOSE is not in that mask, so the else branch runs on a socket whose
ao_info->rnext_key was never set. tcp_ao_add_cmd() deliberately skips it
for listen/close sockets:
net/ipv4/tcp_ao.c:tcp_ao_add_cmd() {
...
if (!((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))) {
tcp_ao_cache_traffic_keys(sk, ao_info, key);
if (first) {
ao_info->current_key = key;
ao_info->rnext_key = key;
}
}
...
}
and include/net/tcp_ao.h documents "Never dereference any of them from
listen sockets". Does rnext_key->rcvid then dereference NULL in softirq
context?
If rnext_key happens to be non-NULL, *traffic_key = snd_other_key(*key)
returns traffic key bytes that were never computed, since
tcp_ao_cache_traffic_keys() is skipped for LISTEN/CLOSE and
tcp_ao_key_alloc() uses sock_kmalloc() without zeroing. Would the RST be
MACed with uninitialized memory and dropped by the peer?
In the narrow window where the listener is still TCP_LISTEN, the LISTEN
branch derives the key with sisn = th->seq and disn = 0, which only matches
a SYN, not the final handshake ACK that got us here. Does that leave the
RST with an invalid MAC (or no RST at all when tcp_ao_do_lookup() fails and
tcp_v4_ao_sign_reset() returns early), so the hang this patch fixes remains
for TCP-AO connections while the child is still destroyed?
[Severity: Medium]
Is this path also reachable from TCP Fast Open, and if so, is the RST
correct there?
tcp_fastopen_create_child() calls into this function with the TFO SYN skb:
net/ipv4/tcp_fastopen.c:tcp_fastopen_create_child() {
child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL,
NULL, &own_req, NULL);
if (!child)
return NULL;
...
}
For that skb th->ack is 0, so tcp_v4_send_reset() emits RST+ACK
acknowledging the SYN. The NULL return does not abort the handshake:
tcp_try_fastopen() returns NULL and tcp_conn_request() continues in the
else branch:
net/ipv4/tcp_input.c:tcp_conn_request() {
} else {
tcp_rsk(req)->tfo_listener = false;
if (!want_cookie &&
unlikely(!inet_csk_reqsk_queue_hash_add(sk, req)))
...
af_ops->send_synack(sk, dst, &fl, req, &foc,
...
}
Does that mean the same receive pass sends an RST and then a SYN-ACK for
the same 4-tuple, and leaves a request sock hashed on a listener that is
being torn down? There is currently no way for this label to tell the
caller that the flow was already reset, so the fast open fallback cannot be
suppressed.
> 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..ecb0b405703c 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
[ ... ]
> @@ -1547,6 +1552,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:
> + tcp_v6_send_reset(sk, skb, SK_RST_REASON_TCP_STATE);
> put_and_exit:
> inet_csk_prepare_forced_close(newsk);
> tcp_done(newsk);
Both questions above apply here too: tcp_v6_send_reset() calls
tcp_ao_prepare_reset() with the same closed listener, and
tcp_fastopen_create_child() reaches tcp_v6_syn_recv_sock() through
icsk_af_ops->syn_recv_sock() as well.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-17 19:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11 21:09 [PATCH net v3] tcp: reset late connection after listening socket close Asbjørn Sloth Tønnesen
2026-08-12 3:16 ` Kuniyuki Iwashima
2026-08-17 19:55 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.