Re: [RESEND PATCH net-next v7 2/3] inet: Avoid ehash lookup race in inet_ehash_insert()
From: Eric Dumazet <edumazet@google.com>
Date: 2025-10-14 13:59:52
Subsystem:
networking [general], networking [sockets], the rest, tipc network layer · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima, Willem de Bruijn, Linus Torvalds, Jon Maloy
On Mon, Oct 13, 2025 at 7:28 PM [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Xuanqiang Luo <redacted> Since ehash lookups are lockless, if one CPU performs a lookup while another concurrently deletes and inserts (removing reqsk and inserting sk), the lookup may fail to find the socket, an RST may be sent. The call trace map is drawn as follows: CPU 0 CPU 1 ----- ----- inet_ehash_insert() spin_lock() sk_nulls_del_node_init_rcu(osk) __inet_lookup_established() (lookup failed) __sk_nulls_add_node_rcu(sk, list) spin_unlock() As both deletion and insertion operate on the same ehash chain, this patch introduces a new sk_nulls_replace_node_init_rcu() helper functions to implement atomic replacement. Fixes: 5e0724d027f0 ("tcp/dccp: fix hashdance race for passive sessions") Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com> Signed-off-by: Xuanqiang Luo <redacted> --- include/net/sock.h | 14 ++++++++++++++ net/ipv4/inet_hashtables.c | 8 ++++++-- 2 files changed, 20 insertions(+), 2 deletions(-)diff --git a/include/net/sock.h b/include/net/sock.h index 60bcb13f045c..5d299a954859 100644 --- a/include/net/sock.h +++ b/include/net/sock.h@@ -858,6 +858,20 @@ static inline bool sk_nulls_del_node_init_rcu(struct sock *sk) return rc; } +static inline bool sk_nulls_replace_node_init_rcu(struct sock *old, + struct sock *new) +{ + if (sk_hashed(old)) { + hlist_nulls_replace_init_rcu(&old->sk_nulls_node, + &new->sk_nulls_node); + DEBUG_NET_WARN_ON_ONCE(refcount_read(&old->sk_refcnt) == 1);
This DEBUG_NET_WARN_ON_ONCE() is not needed. You copied a WARN_ON(refcount_read(&sk->sk_refcnt) == 1) which predates the conversion of sk_refcnt from atomic_t to refcount_t refcount_dec() has a check which will complain just the same. I will send a patch removing the leftovers:
diff --git a/include/net/sock.h b/include/net/sock.h
index 60bcb13f045c..596302bffedf 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h@@ -828,11 +828,9 @@ static inline bool sk_del_node_init(struct sock *sk) { bool rc = __sk_del_node_init(sk); - if (rc) { - /* paranoid for a while -acme */ - WARN_ON(refcount_read(&sk->sk_refcnt) == 1); + if (rc) __sock_put(sk); - } + return rc; } #define sk_del_node_init_rcu(sk) sk_del_node_init(sk)
@@ -850,11 +848,9 @@ static inline boolsk_nulls_del_node_init_rcu(struct sock *sk)
{
bool rc = __sk_nulls_del_node_init_rcu(sk);
- if (rc) {
- /* paranoid for a while -acme */
- WARN_ON(refcount_read(&sk->sk_refcnt) == 1);
+ if (rc)
__sock_put(sk);
- }
+
return rc;
}
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 2b46c0cd752a..687a84c48882 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c@@ -596,10 +596,8 @@ static void netlink_remove(struct sock *sk) table = &nl_table[sk->sk_protocol]; if (!rhashtable_remove_fast(&table->hash, &nlk_sk(sk)->node, - netlink_rhashtable_params)) { - WARN_ON(refcount_read(&sk->sk_refcnt) == 1); + netlink_rhashtable_params)) __sock_put(sk); - } netlink_table_grab(); if (nlk_sk(sk)->subscriptions) {
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 1574a83384f8..bc614a1f019c 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c@@ -3031,10 +3031,8 @@ static void tipc_sk_remove(struct tipc_sock *tsk) struct sock *sk = &tsk->sk; struct tipc_net *tn = net_generic(sock_net(sk), tipc_net_id); - if (!rhashtable_remove_fast(&tn->sk_rht, &tsk->node, tsk_rht_params)) { - WARN_ON(refcount_read(&sk->sk_refcnt) == 1); + if (!rhashtable_remove_fast(&tn->sk_rht, &tsk->node, tsk_rht_params)) __sock_put(sk); - } } static const struct rhashtable_params tsk_rht_params = {