* Re: [PATCH net] tcp: wait for TIME_WAIT teardown before freeing per-netns ehash
2026-10-28 16:25 [PATCH net] tcp: wait for TIME_WAIT teardown before freeing per-netns ehash Eric Dumazet
@ 2026-10-28 19:45 ` Kuniyuki Iwashima
2026-10-29 6:29 ` Jiayuan Chen
2026-10-29 17:15 ` netdev-bot+sashiko
2 siblings, 0 replies; 4+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-28 19:45 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:25:10 +0100, Eric Dumazet wrote:
> From: Kyle Zeng <kylebot@openai.com>
>
> inet_twsk_kill() removes a TIME_WAIT socket from the established hash
> before it finishes using the ehash lock, hashinfo and bind tables, and
> before it drops the death-row reference. A running timer can therefore
> unhash its socket before inet_twsk_purge() reaches it. The purge cannot
> synchronize with that timer, and tcp_sk_exit_batch() can free the
> private ehash and hashinfo while inet_twsk_kill() is still using them.
>
> An unprivileged reproducer using user and network namespaces, a private
> TCP ehash and a FIN_WAIT2 timer triggers a KASAN slab-use-after-free in
> inet_twsk_kill(). The allocation and free stacks point to
> inet_pernet_hashinfo_alloc() and inet_pernet_hashinfo_free().
>
> Wait for an RCU grace period after purging the whole batch.
> TIME_WAIT removal runs with bottom halves disabled, so this also waits
> for removers that were already invisible to the purge. Keep the wait
> under tcp_exit_batch_mutex, before freeing any private hashinfo.
>
> Fixes: d1e5e6408b30 ("tcp: Introduce optional per-netns ehash.")
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> [edumazet: only call synchronize_net() if one netns in the batch uses a private ehash]
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Thanks !
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] tcp: wait for TIME_WAIT teardown before freeing per-netns ehash
2026-10-28 16:25 [PATCH net] tcp: wait for TIME_WAIT teardown before freeing per-netns ehash Eric Dumazet
2026-10-28 19:45 ` Kuniyuki Iwashima
@ 2026-10-29 6:29 ` Jiayuan Chen
2026-10-29 17:15 ` netdev-bot+sashiko
2 siblings, 0 replies; 4+ messages in thread
From: Jiayuan Chen @ 2026-10-29 6:29 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni,
Kuniyuki Iwashima, Neal Cardwell
Cc: Simon Horman, netdev, Kyle Zeng
On 10/29/26 12:25 AM, Eric Dumazet wrote:
> From: Kyle Zeng <kylebot@openai.com>
>
> inet_twsk_kill() removes a TIME_WAIT socket from the established hash
> before it finishes using the ehash lock, hashinfo and bind tables, and
> before it drops the death-row reference. A running timer can therefore
> unhash its socket before inet_twsk_purge() reaches it. The purge cannot
> synchronize with that timer, and tcp_sk_exit_batch() can free the
> private ehash and hashinfo while inet_twsk_kill() is still using them.
>
> An unprivileged reproducer using user and network namespaces, a private
> TCP ehash and a FIN_WAIT2 timer triggers a KASAN slab-use-after-free in
> inet_twsk_kill(). The allocation and free stacks point to
> inet_pernet_hashinfo_alloc() and inet_pernet_hashinfo_free().
>
> Wait for an RCU grace period after purging the whole batch.
> TIME_WAIT removal runs with bottom halves disabled, so this also waits
> for removers that were already invisible to the purge. Keep the wait
> under tcp_exit_batch_mutex, before freeing any private hashinfo.
>
> Fixes: d1e5e6408b30 ("tcp: Introduce optional per-netns ehash.")
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> [edumazet: only call synchronize_net() if one netns in the batch uses a private ehash]
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---
> net/ipv4/tcp_ipv4.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index ebe5aa70374a3fd9e3305377efebec3004989945..3f8563a36db1e9ea853699ecb547af7ee9491b11 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -3561,6 +3561,17 @@ static void __net_exit tcp_sk_exit_batch(struct list_head *net_exit_list)
>
> tcp_twsk_purge(net_exit_list);
>
> + /* inet_twsk_kill() can unhash a socket before the purge sees it.
> + * Wait for such BH-disabled callers to finish before freeing a
> + * private ehash. This is not needed for the shared tcp_hashinfo.
> + */
> + list_for_each_entry(net, net_exit_list, exit_list) {
> + if (net->ipv4.tcp_death_row.hashinfo->pernet) {
> + synchronize_net();
> + break;
> + }
> + }
Calling synchronize_net() only for the private ehash is nice and cheap
> +
> list_for_each_entry(net, net_exit_list, exit_list) {
> inet_pernet_hashinfo_free(net->ipv4.tcp_death_row.hashinfo);
> WARN_ON_ONCE(!refcount_dec_and_test(&net->ipv4.tcp_death_row.tw_refcount));
But I think this WARN_ON_ONCE can still be triggered, is that intentional?
tcp_sk_exit_batch inet_twsk_kill (timer)
unlink
tcp_twsk_purge() // does not see tw
refcount_dec_and_test() // tw_refcount 2 -> 1, WARN
refcount_dec(&tw->tw_dr->tw_refcount)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] tcp: wait for TIME_WAIT teardown before freeing per-netns ehash
2026-10-28 16:25 [PATCH net] tcp: wait for TIME_WAIT teardown before freeing per-netns ehash Eric Dumazet
2026-10-28 19:45 ` Kuniyuki Iwashima
2026-10-29 6:29 ` Jiayuan Chen
@ 2026-10-29 17:15 ` netdev-bot+sashiko
2 siblings, 0 replies; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-10-29 17:15 UTC (permalink / raw)
To: edumazet; +Cc: davem, kuba, pabeni, kuniyu, ncardwell, horms, netdev, kylebot
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 (1) · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] tcp: missing synchronize_net() in tcp_sk_exit_batch() for shared
ehash
Pre-existing issues:
- [Medium] tcp_sk_exit_batch() (net/ipv4/tcp_ipv4.c) now calls
synchronize_net() only when some netns in the batch has a private…
--
tcp: wait for TIME_WAIT teardown before freeing per-netns ehash
This commit adds a synchronize_net() call in tcp_sk_exit_batch() after
tcp_twsk_purge(). The goal is to wait for inet_twsk_kill() callers that
unhashed their socket before the purge could see it. The wait runs only
when at least one netns in the batch uses a private ehash, before the
private hashinfo is freed.
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index ebe5aa70374a3..3f8563a36db1e 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -3561,6 +3561,17 @@ static void __net_exit tcp_sk_exit_batch(struct list_head *net_exit_list)
>
> tcp_twsk_purge(net_exit_list);
>
> + /* inet_twsk_kill() can unhash a socket before the purge sees it.
> + * Wait for such BH-disabled callers to finish before freeing a
> + * private ehash. This is not needed for the shared tcp_hashinfo.
> + */
> + list_for_each_entry(net, net_exit_list, exit_list) {
> + if (net->ipv4.tcp_death_row.hashinfo->pernet) {
> + synchronize_net();
> + break;
> + }
> + }
> +
> list_for_each_entry(net, net_exit_list, exit_list) {
> inet_pernet_hashinfo_free(net->ipv4.tcp_death_row.hashinfo);
> WARN_ON_ONCE(!refcount_dec_and_test(&net->ipv4.tcp_death_row.tw_refcount));
[Severity: High]
Is the shared tcp_hashinfo case really safe to skip here?
The new comment says the grace period is not needed for the shared
tcp_hashinfo. That holds for the hashinfo memory. inet_twsk_kill()
also touches per-netns state, though, and tcp_sk_exit_batch() checks
that state for every netns in the batch.
inet_twsk_kill() does three things in order:
- unhashes the twsk with sk_nulls_del_node_init_rcu()
- looks up the bhash and bhash2 buckets via twsk_net(tw)
- calls refcount_dec(&tw->tw_dr->tw_refcount), where tw_dr is
&net->ipv4.tcp_death_row
Every TIME_WAIT socket holds a tw_refcount reference, taken in
__inet_twsk_schedule(), whether or not hashinfo->pernet is set.
Consider a batch where every netns uses the default shared ehash
(tcp_child_ehash_entries == 0). Can this still happen?
CPU A
tw_timer_handler()
inet_twsk_kill()
sk_nulls_del_node_init_rcu() /* twsk of dying netns unhashed */
CPU B
tcp_sk_exit_batch()
tcp_twsk_purge()
inet_twsk_purge(&tcp_hashinfo) /* twsk already gone */
/* no pernet hashinfo, synchronize_net() skipped */
WARN_ON_ONCE(!refcount_dec_and_test(
&net->ipv4.tcp_death_row.tw_refcount))
/* 2 -> 1, WARN fires */
CPU A
refcount_dec(&tw->tw_dr->tw_refcount)
/* 1 -> 0, REFCOUNT_DEC_LEAK */
A BH caller can hit the same window. If tcp_v4_rcv() wins
timer_shutdown_sync() in inet_twsk_deschedule_put(), it may still be
inside inet_twsk_kill() when the purge runs. The purge then either
fails to find the twsk, or gets false from its own
timer_shutdown_sync(). In both cases it does not wait.
The result is a WARN splat in tcp_sk_exit_batch(), then a
refcount_warn_saturate() splat from inet_twsk_kill(). Systems with
panic_on_warn would panic on the first one. inet_twsk_kill() can also
keep dereferencing twsk_net(tw) and tw->tw_dr after
tcp_sk_exit_batch() has returned for that netns. An unprivileged user
with user and network namespaces can reach this setup, the same as in
the reproducer in the commit message.
The window predates this patch. However, the new comment documents the
shared case as not needing the wait. That seems to contradict the
invariant stated above the mutex_lock() in tcp_sk_exit_batch(): the
thread dismantling a twsk must finish before another thread reaches
refcount_dec_and_test() of tcp_death_row.tw_refcount.
Should the wait cover the shared case too? It could run
unconditionally. Or it could run when
refcount_read(&net->ipv4.tcp_death_row.tw_refcount) != 1 for some net
in the batch after tcp_twsk_purge(), which would keep the common
no-TIME_WAIT case cheap.
[Severity: Medium]
This isn't a bug introduced by this patch, but is the shared tcp_hashinfo
case really safe to skip?
The hashinfo memory doesn't need the wait in that case. The per-netns
tw_refcount, though, lives in struct net, and the WARN_ON_ONCE() above
checks it for every netns, shared ehash or private.
inet_twsk_kill() in net/ipv4/inet_timewait_sock.c unhashes first. It
drops the death-row reference only after the bind table locking:
net/ipv4/inet_timewait_sock.c:inet_twsk_kill() {
...
spin_lock(lock);
sk_nulls_del_node_init_rcu((struct sock *)tw);
spin_unlock(lock);
...
spin_lock(&bhead->lock);
spin_lock(&bhead2->lock);
inet_twsk_bind_unhash(tw, hashinfo);
spin_unlock(&bhead2->lock);
spin_unlock(&bhead->lock);
refcount_dec(&tw->tw_dr->tw_refcount);
inet_twsk_put(tw);
}
Take a batch where every netns uses the shared tcp_hashinfo (the default,
tcp_child_ehash_entries == 0). Could this sequence happen?
CPU A
tw_timer_handler()
inet_twsk_kill()
sk_nulls_del_node_init_rcu() /* twsk of dying netns N unhashed */
CPU B
tcp_sk_exit_batch()
tcp_twsk_purge()
inet_twsk_purge(&tcp_hashinfo) /* never sees the twsk, so no
timer_shutdown_sync() */
/* no pernet hashinfo, synchronize_net() skipped */
WARN_ON_ONCE(!refcount_dec_and_test(&net->ipv4.tcp_death_row.tw_refcount));
/* count is 2 -> 1, WARN fires */
CPU A
refcount_dec(&tw->tw_dr->tw_refcount);
/* 1 -> 0, refcount_warn_saturate()
with REFCOUNT_DEC_LEAK */
That would give two warning splats and leave tw_refcount saturated in the
dying netns. On panic_on_warn systems, the first one would panic. An
unprivileged user with user and network namespaces can reach this, the
same setup as the reproducer in the commit message.
The comment above mutex_lock() in tcp_sk_exit_batch() states the
invariant:
* tcp_twsk_purge() handles twsk in any dead netns, not just those in
* net_exit_list, the thread that dismantles a particular twsk must
* do so without other thread progressing to refcount_dec_and_test() of
* tcp_death_row.tw_refcount.
The commit message also says:
Wait for an RCU grace period after purging the whole batch.
TIME_WAIT removal runs with bottom halves disabled, so this also waits
for removers that were already invisible to the purge.
With the pernet condition in place, that only holds for batches that
contain at least one private-ehash netns. Mixed batches are covered only
because the single synchronize_net() also covers the shared-ehash nets.
This same window existed before this patch for both shared and private
ehash. The shared-ehash path is unchanged here.
Would it make sense to drop the pernet condition and wait for every batch
before the refcount_dec_and_test() loop? If not, should the new comment
say that the shared case still has this tw_refcount window?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261028162510.2180528-1-edumazet%40kernel.org
^ permalink raw reply [flat|nested] 4+ messages in thread