All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] tcp: avoid double destroy in tcp_abort() of an unaccepted child
@ 2026-10-28 16:08 Eric Dumazet
  2026-10-28 16:11 ` netdev-bot+sinfo
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Eric Dumazet @ 2026-10-28 16:08 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
	Neal Cardwell
  Cc: Simon Horman, netdev, Kyle Zeng, Eric Dumazet

From: Kyle Zeng <kylebot@openai.com>

tcp_abort() checks for TCP_CLOSE while only owning the socket lock,
then grabs the BH lock before calling tcp_done_with_error().

For a child still sitting in a listener accept queue (which can be
reached via SOCK_DESTROY or bpf_sock_destroy()), inet_csk_listen_stop()
can run in this window: it only holds the child BH lock, and calls
inet_child_forget(), which disconnects, orphans and destroys the child.

tcp_abort() then calls tcp_done_with_error() -> tcp_done(), which sees
SOCK_DEAD and calls inet_csk_destroy_sock() a second time, leading to
a double sock_put() and a use-after-free.

Recheck TCP_CLOSE after acquiring the BH lock, unless we just closed
a listener ourselves, in which case tcp_done_with_error() must still
run to report the error.

Note that the WARN_ON(sock_owned_by_user(child)) in
inet_csk_listen_stop() can still trigger in this window.

Fixes: bac76cf89816 ("tcp: fix forever orphan socket caused by tcp_abort")
Signed-off-by: Kyle Zeng <kylebot@openai.com>
[edumazet: recheck TCP_CLOSE under the BH lock instead of moving the test, smaller diff]
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/ipv4/tcp.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 1f56d8c0c5fe947072eb5ce50217b13fd33ff74d..6a9566f179e02c3a2e3c91f4ffa96a85190c861d 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -5128,6 +5128,8 @@ void tcp_done(struct sock *sk)
 int tcp_abort(struct sock *sk, int err)
 {
 	int state = inet_sk_state_load(sk);
+	bool listener = false;
+	int ret = 0;
 
 	if (state == TCP_NEW_SYN_RECV) {
 		struct request_sock *req = inet_reqsk(sk);
@@ -5162,21 +5164,31 @@ int tcp_abort(struct sock *sk, int err)
 	if (sk->sk_state == TCP_LISTEN) {
 		tcp_set_state(sk, TCP_CLOSE);
 		inet_csk_listen_stop(sk);
+		listener = true;
 	}
 
 	/* Don't race with BH socket closes such as inet_csk_listen_stop. */
 	local_bh_disable();
 	bh_lock_sock(sk);
 
+	/* An unaccepted child might have been closed by
+	 * inet_csk_listen_stop() while we were not holding its BH lock.
+	 */
+	if (!listener && sk->sk_state == TCP_CLOSE) {
+		ret = -ENOENT;
+		goto out;
+	}
+
 	if (tcp_need_reset(sk->sk_state))
 		tcp_send_active_reset(sk, SK_RST_REASON_TCP_STATE);
 	tcp_done_with_error(sk, err);
 
+out:
 	bh_unlock_sock(sk);
 	local_bh_enable();
 	if (!has_current_bpf_ctx())
 		release_sock(sk);
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(tcp_abort);
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net] tcp: avoid double destroy in tcp_abort() of an unaccepted child
  2026-10-28 16:08 [PATCH net] tcp: avoid double destroy in tcp_abort() of an unaccepted child Eric Dumazet
@ 2026-10-28 16:11 ` netdev-bot+sinfo
  2026-10-28 19:54 ` Kuniyuki Iwashima
  2026-10-29 16:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: netdev-bot+sinfo @ 2026-10-28 16:11 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
	Neal Cardwell, Simon Horman, netdev, Kyle Zeng

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] tcp: avoid double destroy in tcp_abort() of an unaccepted child
  2026-10-28 16:08 [PATCH net] tcp: avoid double destroy in tcp_abort() of an unaccepted child Eric Dumazet
  2026-10-28 16:11 ` netdev-bot+sinfo
@ 2026-10-28 19:54 ` Kuniyuki Iwashima
  2026-10-29 16:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-28 19:54 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Neal Cardwell,
	Simon Horman, netdev, Kyle Zeng

On Wed, 28 Oct 2026 17:08:47 +0100, Eric Dumazet wrote:
> From: Kyle Zeng <kylebot@openai.com>
> 
> tcp_abort() checks for TCP_CLOSE while only owning the socket lock,
> then grabs the BH lock before calling tcp_done_with_error().
> 
> For a child still sitting in a listener accept queue (which can be
> reached via SOCK_DESTROY or bpf_sock_destroy()), inet_csk_listen_stop()
> can run in this window: it only holds the child BH lock, and calls
> inet_child_forget(), which disconnects, orphans and destroys the child.
> 
> tcp_abort() then calls tcp_done_with_error() -> tcp_done(), which sees
> SOCK_DEAD and calls inet_csk_destroy_sock() a second time, leading to
> a double sock_put() and a use-after-free.
> 
> Recheck TCP_CLOSE after acquiring the BH lock, unless we just closed
> a listener ourselves, in which case tcp_done_with_error() must still
> run to report the error.
> 
> Note that the WARN_ON(sock_owned_by_user(child)) in
> inet_csk_listen_stop() can still trigger in this window.
> 
> Fixes: bac76cf89816 ("tcp: fix forever orphan socket caused by tcp_abort")
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> [edumazet: recheck TCP_CLOSE under the BH lock instead of moving the test, smaller diff]
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] tcp: avoid double destroy in tcp_abort() of an unaccepted child
  2026-10-28 16:08 [PATCH net] tcp: avoid double destroy in tcp_abort() of an unaccepted child Eric Dumazet
  2026-10-28 16:11 ` netdev-bot+sinfo
  2026-10-28 19:54 ` Kuniyuki Iwashima
@ 2026-10-29 16:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-29 16:20 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: davem, kuba, pabeni, kuniyu, ncardwell, horms, netdev, kylebot

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 28 Oct 2026 17:08:47 +0100 you wrote:
> From: Kyle Zeng <kylebot@openai.com>
> 
> tcp_abort() checks for TCP_CLOSE while only owning the socket lock,
> then grabs the BH lock before calling tcp_done_with_error().
> 
> For a child still sitting in a listener accept queue (which can be
> reached via SOCK_DESTROY or bpf_sock_destroy()), inet_csk_listen_stop()
> can run in this window: it only holds the child BH lock, and calls
> inet_child_forget(), which disconnects, orphans and destroys the child.
> 
> [...]

Here is the summary with links:
  - [net] tcp: avoid double destroy in tcp_abort() of an unaccepted child
    https://git.kernel.org/netdev/net/c/dbd62ad55a4c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-29 16:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 16:08 [PATCH net] tcp: avoid double destroy in tcp_abort() of an unaccepted child Eric Dumazet
2026-10-28 16:11 ` netdev-bot+sinfo
2026-10-28 19:54 ` Kuniyuki Iwashima
2026-10-29 16:20 ` patchwork-bot+netdevbpf

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.