Re: [PATCH v2 net] tcp: wait for TIME_WAIT teardown before freeing per-netns ehash

[Kuniyuki Iwashima <kuniyu@google.com>]

On Fri, 30 Oct 2026 08:58:43 +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, if one
> netns in the batch uses a private ehash. 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.
> 
> The same window exists with the shared tcp_hashinfo. There, the
> hashinfo memory stays valid, but tcp_sk_exit_batch() can decrement
> tcp_death_row.tw_refcount from 2 to 1 before inet_twsk_kill() drops its
> own reference. This triggers the WARN_ON_ONCE() in tcp_sk_exit_batch(),
> then a refcount_t "decrement hit 0" warning from inet_twsk_kill().
> 
> Adding a grace period for the shared case would be costly on busy
> hosts. It is not needed: struct net is freed by a later cleanup_net(),
> after the synchronize_rcu() in ops_undo_list(), so inet_twsk_kill()
> can safely drop the last tw_refcount reference itself.
> Use refcount_dec_if_one() and only warn if tw_refcount is not 1
> after a grace period.
> 
> 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,
>  use refcount_dec_if_one() to avoid a false WARN with the shared ehash]
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---
> Cc: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
> v2: [edumazet]
>     Avoid a false WARN_ON_ONCE() in tcp_sk_exit_batch() when a concurrent
>     inet_twsk_kill() still holds a tw_refcount reference, as reported by
>     Jiayuan Chen and Sashiko. Use refcount_dec_if_one() and only warn
>     after a grace period, instead of adding synchronize_net() for the
>     shared ehash, which would be costly on busy hosts.
> 
>     When net-next reopens, I plan to free the private hashinfo after
>     an RCU grace period (call_rcu()), so that tcp_sk_exit_batch() no
>     longer needs synchronize_net(). This would also cover request
>     sockets and RCU lookups that could still touch a private ehash.

Sounds good, thanks !

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