[PATCH net v4 0/2] inet: fix lockless ehash walk races

[PATCH net v4 0/2] inet: fix lockless ehash walk races

[Hyunwoo Kim]

Fix two issues in lockless ehash walks. Reject unhashed sockets after
taking a reference in established lookups, and restore the acquire/release
ordering around refcount publication and acquisition.

Changes in v4:
- 2/2: add acquire ordering to inet_twsk_purge() and update the changelog
  (Sashiko)
- v3: https://lore.kernel.org/r/20261024053720.1147022-1-imv4bel@gmail.com

Changes in v3:
- 1/2: clarify the request reference comment (Eric)
- 2/2: drop the sk_clone() change, keep the TCP Fast Open update in
  place, add release ordering to TIME_WAIT, and rework the changelog
  (Eric)
- 2/2: note that backports to trees without the refcount helpers need
  adjustment (Eric)
- v2: https://lore.kernel.org/r/20261023052542.3505250-1-imv4bel@gmail.com

Changes in v2:
- 1/2: split the unhashed checks and __reqsk_free() to reqsk_put()
  conversions from the ordering changes, set the Fixes tag to commit
  079096f103fa ("tcp/dccp: install syn_recv requests into ehash table"),
  mention the TIME_WAIT hashdance and request replacement cases, clarify
  request reference ownership, and drop the full socket race discussion
  (Eric)
- 2/2: new, split out the acquire/release changes, set the Fixes tag to
  commit fb041bb7c0a9 ("locking/refcount: Consolidate implementations of
  refcount_t"), cover sk_clone(), and leave the already ordered TIME_WAIT
  path unchanged (Eric)
- v1: https://lore.kernel.org/r/ath58Egc-oI6p3Jl@v4bel

Hyunwoo Kim (2):
  inet: reject unhashed sockets during established lookups
  inet: restore refcount ordering for lockless ehash walks

 net/ipv4/inet_connection_sock.c | 16 ++++++++++------
 net/ipv4/inet_hashtables.c      |  7 ++++---
 net/ipv4/inet_timewait_sock.c   |  4 ++--
 net/ipv4/syncookies.c           |  4 +++-
 net/ipv4/tcp_fastopen.c         |  2 +-
 net/ipv4/tcp_input.c            |  4 +++-
 net/ipv6/inet6_hashtables.c     |  6 ++++--
 7 files changed, 27 insertions(+), 16 deletions(-)

-- 
2.43.0

[PATCH net v4 1/2] inet: reject unhashed sockets during established lookups

[Hyunwoo Kim]

TCP established ehash lookups are lockless. The socket caches involved
use SLAB_TYPESAFE_BY_RCU, so memory can be reused before an RCU reader
that retained its old address has finished.

After a tuple match, __inet_lookup_established() and
__inet6_lookup_established() acquire a reference and repeat the tuple
check. They do not check whether the new object is hashed. If the
address is reused for a socket with the same tuple, the lookup can
acquire a reference to the new object and return it while its nulls node
is unhashed.

A PoC that drives reuse with a SYN cookie request socket reproduced the
crash. The new request socket is unhashed and has a NULL rsk_listener.
A lookup that had loaded the address of the old request socket acquired
the reference published by tcp_get_cookie_sock() and returned the new
object. tcp_v4_rcv() then read sk_state through the NULL listener.

The resulting crash was:

  BUG: kernel NULL pointer dereference, address: 0000000000000012
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  ...
  Oops: Oops: 0000 [#1] SMP NOPTI
  CPU: 4 UID: 0 PID: 0 Comm: swapper/4 Not tainted 7.3.0-00591-g0e135910deec #1 PREEMPT(lazy)
  ...
  RIP: 0010:tcp_v4_rcv+0x44e/0x1280
  ...
  R10: 0000000000000001 R11: 00000000024ca8c0 R12: 0000000000000000
  ...
  CR2: 0000000000000012 CR3: 0000000103d50004 CR4: 0000000000770ef0
  ...
  Call Trace:
   <IRQ>
   ? raw_local_deliver+0xae/0x240
   ip_protocol_deliver_rcu+0x2d/0x180
   ip_local_deliver_finish+0x84/0x100
   ip_sublist_rcv_finish+0x5f/0x70
   ip_sublist_rcv+0x12a/0x1d0
   ip_list_rcv+0xfd/0x130
   __netif_receive_skb_list_core+0x223/0x250
   netif_receive_skb_list_internal+0x1d2/0x2c0
   ? try_fill_recv+0x39/0x7b0
   napi_complete_done+0x74/0x170
   virtnet_poll+0x445/0x600
   __napi_poll+0x27/0x1e0
   net_rx_action+0x2fe/0x3b0
   handle_softirqs+0xc2/0x2c0
   __irq_exit_rcu+0x74/0xd0
   common_interrupt+0x7f/0x90
   </IRQ>
  ...
  Kernel panic - not syncing: Fatal exception in interrupt

After taking a reference, check whether the nulls node is unhashed in
both IPv4 and IPv6 established lookups. If it is unhashed or the tuple
no longer matches, drop the reference and restart the lookup.

Lookups racing with the TIME_WAIT hashdance, or with a request being
replaced by its child, now restart and find the new socket instead of
returning the old one.

Request sockets that were never inserted into ehash can have a nonzero
refcount, and a stale lookup can hold a transient reference. Therefore,
when SYN cookie, Fast Open, and request migration paths fail after
setting a nonzero refcount, release the references held by those paths
with reqsk_put() instead of freeing the request directly.

Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@kernel.org>
---
 net/ipv4/inet_connection_sock.c | 10 +++++++---
 net/ipv4/inet_hashtables.c      |  5 +++--
 net/ipv4/syncookies.c           |  2 ++
 net/ipv4/tcp_input.c            |  4 +++-
 net/ipv6/inet6_hashtables.c     |  4 +++-
 5 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 6a30f11384547..bd0bad96c93c3 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -1136,10 +1136,14 @@ static void reqsk_timer_handler(struct timer_list *t)
 	 */
 	if (nreq) {
 		__NET_INC_STATS(net, LINUX_MIB_TCPMIGRATEREQFAILURE);
+		/* The timer was not armed, drop its reference here. */
+		reqsk_put(nreq);
 no_ownership:
 		reqsk_migrate_reset(nreq);
 		reqsk_queue_removed(queue, nreq);
-		__reqsk_free(nreq);
+		/* Drop the temporary reference and the one reserved for ehash. */
+		reqsk_put(nreq);
+		reqsk_put(nreq);
 	}
 
 drop:
@@ -1447,7 +1451,7 @@ struct sock *inet_csk_complete_hashdance(struct sock *sk, struct sock *child,
 
 			__NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMIGRATEREQFAILURE);
 			reqsk_migrate_reset(nreq);
-			__reqsk_free(nreq);
+			reqsk_put(nreq);
 		} else if (inet_csk_reqsk_queue_add(sk, req, child)) {
 			return child;
 		}
@@ -1502,7 +1506,7 @@ void inet_csk_listen_stop(struct sock *sk)
 					__NET_INC_STATS(sock_net(nsk),
 							LINUX_MIB_TCPMIGRATEREQFAILURE);
 					reqsk_migrate_reset(nreq);
-					__reqsk_free(nreq);
+					reqsk_put(nreq);
 				}
 				rcu_read_unlock();
 
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index ba0faa9ae2bb3..58d54965036a2 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -552,8 +552,9 @@ struct sock *__inet_lookup_established(const struct net *net,
 		if (likely(inet_match(net, sk, acookie, ports, dif, sdif))) {
 			if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
 				goto out;
-			if (unlikely(!inet_match(net, sk, acookie,
-						 ports, dif, sdif))) {
+			if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
+				     !inet_match(net, sk, acookie, ports, dif,
+						 sdif))) {
 				sock_gen_put(sk);
 				goto begin;
 			}
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 73e1297681847..6440305615378 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -218,6 +218,8 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
 
 		bh_unlock_sock(child);
 		sock_put(child);
+		reqsk_put(req);
+		return NULL;
 	}
 	__reqsk_free(req);
 
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 99baf14afdfd8..3583867487d7f 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -7777,7 +7777,9 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
 		if (!inet_csk_reqsk_queue_add(sk, req, fastopen_sk)) {
 			bh_unlock_sock(fastopen_sk);
 			sock_put(fastopen_sk);
-			goto drop_and_free;
+			/* tcp_disconnect() already dropped one reqsk ref. */
+			reqsk_put(req);
+			goto drop;
 		}
 		READ_ONCE(sk->sk_data_ready)(sk);
 		bh_unlock_sock(fastopen_sk);
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index b111b51d69fc9..aa5fa1dd062f7 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -108,7 +108,9 @@ struct sock *__inet6_lookup_established(const struct net *net,
 		if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
 			goto out;
 
-		if (unlikely(!inet6_match(net, sk, saddr, daddr, ports, dif, sdif))) {
+		if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
+			     !inet6_match(net, sk, saddr, daddr, ports, dif,
+					  sdif))) {
 			sock_gen_put(sk);
 			goto begin;
 		}
-- 
2.43.0

[PATCH net v4 2/2] inet: restore refcount ordering for lockless ehash walks

[Hyunwoo Kim]

Before commit fb041bb7c0a9 ("locking/refcount: Consolidate implementations
of refcount_t"), default builds implemented refcount_inc_not_zero() using
atomic_add_unless(), which provided full memory ordering on success. The
commit removed that ordering on weakly ordered architectures, allowing
validation after a successful reference acquisition in lockless ehash
walks to be reordered before it.

Use refcount_inc_not_zero_acquire() in the IPv4 and IPv6 established
lookups and in inet_twsk_purge(), where the network namespace is validated
again after taking a reference. Use refcount_set_release() when setting
refcounts in the SYN cookie, TCP Fast Open, request migration, and
TIME_WAIT paths. Keep the existing TIME_WAIT smp_wmb(), which separately
orders the refcount store before ehash publication.

Fixes: fb041bb7c0a9 ("locking/refcount: Consolidate implementations of refcount_t")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 net/ipv4/inet_connection_sock.c | 6 +++---
 net/ipv4/inet_hashtables.c      | 2 +-
 net/ipv4/inet_timewait_sock.c   | 4 ++--
 net/ipv4/syncookies.c           | 2 +-
 net/ipv4/tcp_fastopen.c         | 2 +-
 net/ipv6/inet6_hashtables.c     | 2 +-
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index bd0bad96c93c3..837cb91e5b173 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -1056,8 +1056,8 @@ static void reqsk_timer_handler(struct timer_list *t)
 		 * hold another count to prevent use-after-free and
 		 * call reqsk_put() just before return.
 		 */
-		refcount_set(&nreq->rsk_refcnt, 2 + 1);
 		timer_setup(&nreq->rsk_timer, reqsk_timer_handler, TIMER_PINNED);
+		refcount_set_release(&nreq->rsk_refcnt, 2 + 1);
 		reqsk_queue_migrated(&inet_csk(nsk)->icsk_accept_queue, req);
 
 		req = nreq;
@@ -1441,7 +1441,7 @@ struct sock *inet_csk_complete_hashdance(struct sock *sk, struct sock *child,
 				goto child_put;
 			}
 
-			refcount_set(&nreq->rsk_refcnt, 1);
+			refcount_set_release(&nreq->rsk_refcnt, 1);
 			if (inet_csk_reqsk_queue_add(sk, nreq, child)) {
 				__NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMIGRATEREQSUCCESS);
 				reqsk_migrate_reset(req);
@@ -1494,7 +1494,7 @@ void inet_csk_listen_stop(struct sock *sk)
 		if (nsk) {
 			nreq = inet_reqsk_clone(req, nsk);
 			if (nreq) {
-				refcount_set(&nreq->rsk_refcnt, 1);
+				refcount_set_release(&nreq->rsk_refcnt, 1);
 
 				rcu_read_lock();
 				if (inet_csk_reqsk_queue_add(nsk, nreq, child)) {
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 58d54965036a2..7ffa9171d6de1 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -550,7 +550,7 @@ struct sock *__inet_lookup_established(const struct net *net,
 		if (sk->sk_hash != hash)
 			continue;
 		if (likely(inet_match(net, sk, acookie, ports, dif, sdif))) {
-			if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
+			if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
 				goto out;
 			if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
 				     !inet_match(net, sk, acookie, ports, dif,
diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index d4c781a0667fe..74ff40d42e56b 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -141,7 +141,7 @@ void inet_twsk_hashdance_schedule(struct inet_timewait_sock *tw,
 	 * Also note that after this point, we lost our implicit reference
 	 * so we are not allowed to use tw anymore.
 	 */
-	refcount_set(&tw->tw_refcnt, 3);
+	refcount_set_release(&tw->tw_refcnt, 3);
 
 	/* Ensure tw_refcnt has been set before tw is published.
 	 * smp_wmb() provides the necessary memory barrier to enforce this
@@ -328,7 +328,7 @@ void inet_twsk_purge(struct inet_hashinfo *hashinfo)
 			if (check_net(sock_net(sk)))
 				continue;
 
-			if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
+			if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
 				continue;
 
 			if (check_net(sock_net(sk))) {
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 6440305615378..4b2d2713653b7 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -205,7 +205,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
 	child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
 						 NULL, &own_req, NULL);
 	if (child) {
-		refcount_set(&req->rsk_refcnt, 1);
+		refcount_set_release(&req->rsk_refcnt, 1);
 		sock_rps_save_rxhash(child, skb);
 
 		if (rsk_drop_req(req)) {
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 471c78be55137..e40477b6b5549 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -364,7 +364,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
 	tcp_reset_xmit_timer(child, ICSK_TIME_RETRANS,
 			     req->timeout, false);
 
-	refcount_set(&req->rsk_refcnt, 2);
+	refcount_set_release(&req->rsk_refcnt, 2);
 
 	sk_mark_napi_id_set(child, skb);
 
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index aa5fa1dd062f7..1eea584d80df8 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -105,7 +105,7 @@ struct sock *__inet6_lookup_established(const struct net *net,
 			continue;
 		if (!inet6_match(net, sk, saddr, daddr, ports, dif, sdif))
 			continue;
-		if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
+		if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
 			goto out;
 
 		if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
-- 
2.43.0

Re: [PATCH net v4 0/2] inet: fix lockless ehash walk races

[netdev-bot+sinfo]

Hi!

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

 - 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.

Re: [PATCH net v4 1/2] inet: reject unhashed sockets during established lookups

[Kuniyuki Iwashima]

On Mon, 26 Oct 2026 09:37:44 +0900, Hyunwoo Kim wrote:
> TCP established ehash lookups are lockless. The socket caches involved
> use SLAB_TYPESAFE_BY_RCU, so memory can be reused before an RCU reader
> that retained its old address has finished.
> 
> After a tuple match, __inet_lookup_established() and
> __inet6_lookup_established() acquire a reference and repeat the tuple
> check. They do not check whether the new object is hashed. If the
> address is reused for a socket with the same tuple, the lookup can
> acquire a reference to the new object and return it while its nulls node
> is unhashed.
> 
> A PoC that drives reuse with a SYN cookie request socket reproduced the
> crash. The new request socket is unhashed and has a NULL rsk_listener.
> A lookup that had loaded the address of the old request socket acquired
> the reference published by tcp_get_cookie_sock() and returned the new
> object. tcp_v4_rcv() then read sk_state through the NULL listener.
> 
> The resulting crash was:
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000012
>   #PF: supervisor read access in kernel mode
>   #PF: error_code(0x0000) - not-present page
>   ...
>   Oops: Oops: 0000 [#1] SMP NOPTI
>   CPU: 4 UID: 0 PID: 0 Comm: swapper/4 Not tainted 7.3.0-00591-g0e135910deec #1 PREEMPT(lazy)
>   ...
>   RIP: 0010:tcp_v4_rcv+0x44e/0x1280
>   ...
>   R10: 0000000000000001 R11: 00000000024ca8c0 R12: 0000000000000000
>   ...
>   CR2: 0000000000000012 CR3: 0000000103d50004 CR4: 0000000000770ef0
>   ...
>   Call Trace:
>    <IRQ>
>    ? raw_local_deliver+0xae/0x240
>    ip_protocol_deliver_rcu+0x2d/0x180
>    ip_local_deliver_finish+0x84/0x100
>    ip_sublist_rcv_finish+0x5f/0x70
>    ip_sublist_rcv+0x12a/0x1d0
>    ip_list_rcv+0xfd/0x130
>    __netif_receive_skb_list_core+0x223/0x250
>    netif_receive_skb_list_internal+0x1d2/0x2c0
>    ? try_fill_recv+0x39/0x7b0
>    napi_complete_done+0x74/0x170
>    virtnet_poll+0x445/0x600
>    __napi_poll+0x27/0x1e0
>    net_rx_action+0x2fe/0x3b0
>    handle_softirqs+0xc2/0x2c0
>    __irq_exit_rcu+0x74/0xd0
>    common_interrupt+0x7f/0x90
>    </IRQ>
>   ...
>   Kernel panic - not syncing: Fatal exception in interrupt
> 
> After taking a reference, check whether the nulls node is unhashed in
> both IPv4 and IPv6 established lookups. If it is unhashed or the tuple
> no longer matches, drop the reference and restart the lookup.
> 
> Lookups racing with the TIME_WAIT hashdance, or with a request being
> replaced by its child, now restart and find the new socket instead of
> returning the old one.
> 
> Request sockets that were never inserted into ehash can have a nonzero
> refcount, and a stale lookup can hold a transient reference. Therefore,
> when SYN cookie, Fast Open, and request migration paths fail after
> setting a nonzero refcount, release the references held by those paths
> with reqsk_put() instead of freeing the request directly.
> 
> Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> Reviewed-by: Eric Dumazet <edumazet@kernel.org>

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

Re: [PATCH net v4 2/2] inet: restore refcount ordering for lockless ehash walks

[Kuniyuki Iwashima]

On Mon, 26 Oct 2026 09:37:45 +0900, Hyunwoo Kim wrote:
> Before commit fb041bb7c0a9 ("locking/refcount: Consolidate implementations
> of refcount_t"), default builds implemented refcount_inc_not_zero() using
> atomic_add_unless(), which provided full memory ordering on success. The
> commit removed that ordering on weakly ordered architectures, allowing
> validation after a successful reference acquisition in lockless ehash
> walks to be reordered before it.
> 
> Use refcount_inc_not_zero_acquire() in the IPv4 and IPv6 established

Does bpf_sk_assign() need it too ?

If the socket is taken from lookup helper, it's safe, but what
happens if the pointer is taken from BTF (27e5203bd9c5), e.g.
tp->inet_conn.icsk_accept_queue.rskq_accept_head ?


> lookups and in inet_twsk_purge(), where the network namespace is validated
> again after taking a reference. Use refcount_set_release() when setting
> refcounts in the SYN cookie, TCP Fast Open, request migration, and
> TIME_WAIT paths. Keep the existing TIME_WAIT smp_wmb(), which separately
> orders the refcount store before ehash publication.
> 
> Fixes: fb041bb7c0a9 ("locking/refcount: Consolidate implementations of refcount_t")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  net/ipv4/inet_connection_sock.c | 6 +++---
>  net/ipv4/inet_hashtables.c      | 2 +-
>  net/ipv4/inet_timewait_sock.c   | 4 ++--
>  net/ipv4/syncookies.c           | 2 +-
>  net/ipv4/tcp_fastopen.c         | 2 +-
>  net/ipv6/inet6_hashtables.c     | 2 +-
>  6 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index bd0bad96c93c3..837cb91e5b173 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -1056,8 +1056,8 @@ static void reqsk_timer_handler(struct timer_list *t)
>  		 * hold another count to prevent use-after-free and
>  		 * call reqsk_put() just before return.
>  		 */
> -		refcount_set(&nreq->rsk_refcnt, 2 + 1);
>  		timer_setup(&nreq->rsk_timer, reqsk_timer_handler, TIMER_PINNED);
> +		refcount_set_release(&nreq->rsk_refcnt, 2 + 1);
>  		reqsk_queue_migrated(&inet_csk(nsk)->icsk_accept_queue, req);
>  
>  		req = nreq;
> @@ -1441,7 +1441,7 @@ struct sock *inet_csk_complete_hashdance(struct sock *sk, struct sock *child,
>  				goto child_put;
>  			}
>  
> -			refcount_set(&nreq->rsk_refcnt, 1);
> +			refcount_set_release(&nreq->rsk_refcnt, 1);
>  			if (inet_csk_reqsk_queue_add(sk, nreq, child)) {
>  				__NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMIGRATEREQSUCCESS);
>  				reqsk_migrate_reset(req);
> @@ -1494,7 +1494,7 @@ void inet_csk_listen_stop(struct sock *sk)
>  		if (nsk) {
>  			nreq = inet_reqsk_clone(req, nsk);
>  			if (nreq) {
> -				refcount_set(&nreq->rsk_refcnt, 1);
> +				refcount_set_release(&nreq->rsk_refcnt, 1);
>  
>  				rcu_read_lock();
>  				if (inet_csk_reqsk_queue_add(nsk, nreq, child)) {
> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> index 58d54965036a2..7ffa9171d6de1 100644
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -550,7 +550,7 @@ struct sock *__inet_lookup_established(const struct net *net,
>  		if (sk->sk_hash != hash)
>  			continue;
>  		if (likely(inet_match(net, sk, acookie, ports, dif, sdif))) {
> -			if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
> +			if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
>  				goto out;
>  			if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
>  				     !inet_match(net, sk, acookie, ports, dif,
> diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
> index d4c781a0667fe..74ff40d42e56b 100644
> --- a/net/ipv4/inet_timewait_sock.c
> +++ b/net/ipv4/inet_timewait_sock.c
> @@ -141,7 +141,7 @@ void inet_twsk_hashdance_schedule(struct inet_timewait_sock *tw,
>  	 * Also note that after this point, we lost our implicit reference
>  	 * so we are not allowed to use tw anymore.
>  	 */
> -	refcount_set(&tw->tw_refcnt, 3);
> +	refcount_set_release(&tw->tw_refcnt, 3);
>  
>  	/* Ensure tw_refcnt has been set before tw is published.
>  	 * smp_wmb() provides the necessary memory barrier to enforce this
> @@ -328,7 +328,7 @@ void inet_twsk_purge(struct inet_hashinfo *hashinfo)
>  			if (check_net(sock_net(sk)))
>  				continue;
>  
> -			if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
> +			if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
>  				continue;
>  
>  			if (check_net(sock_net(sk))) {
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index 6440305615378..4b2d2713653b7 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -205,7 +205,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
>  	child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
>  						 NULL, &own_req, NULL);
>  	if (child) {
> -		refcount_set(&req->rsk_refcnt, 1);
> +		refcount_set_release(&req->rsk_refcnt, 1);
>  		sock_rps_save_rxhash(child, skb);
>  
>  		if (rsk_drop_req(req)) {
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index 471c78be55137..e40477b6b5549 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -364,7 +364,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
>  	tcp_reset_xmit_timer(child, ICSK_TIME_RETRANS,
>  			     req->timeout, false);
>  
> -	refcount_set(&req->rsk_refcnt, 2);
> +	refcount_set_release(&req->rsk_refcnt, 2);
>  
>  	sk_mark_napi_id_set(child, skb);
>  
> diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
> index aa5fa1dd062f7..1eea584d80df8 100644
> --- a/net/ipv6/inet6_hashtables.c
> +++ b/net/ipv6/inet6_hashtables.c
> @@ -105,7 +105,7 @@ struct sock *__inet6_lookup_established(const struct net *net,
>  			continue;
>  		if (!inet6_match(net, sk, saddr, daddr, ports, dif, sdif))
>  			continue;
> -		if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
> +		if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
>  			goto out;
>  
>  		if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
> -- 
> 2.43.0
> 

Re: [PATCH net v4 2/2] inet: restore refcount ordering for lockless ehash walks

[Hyunwoo Kim]

On Wed, Oct 28, 2026 at 02:13:15AM +0000, Kuniyuki Iwashima wrote:
> On Mon, 26 Oct 2026 09:37:45 +0900, Hyunwoo Kim wrote:
> > Before commit fb041bb7c0a9 ("locking/refcount: Consolidate implementations
> > of refcount_t"), default builds implemented refcount_inc_not_zero() using
> > atomic_add_unless(), which provided full memory ordering on success. The
> > commit removed that ordering on weakly ordered architectures, allowing
> > validation after a successful reference acquisition in lockless ehash
> > walks to be reordered before it.
> > 
> > Use refcount_inc_not_zero_acquire() in the IPv4 and IPv6 established
> 
> Does bpf_sk_assign() need it too ?
> 
> If the socket is taken from lookup helper, it's safe, but what
> happens if the pointer is taken from BTF (27e5203bd9c5), e.g.
> tp->inet_conn.icsk_accept_queue.rskq_accept_head ?

Yeah the same ordering issue remains when the pointer is obtained from 
BTF without a reference. Fixes: 27e5203bd9c5.

However, I think this is better handled in a separate follow-up patch. 
What do you think?

> 
> 
> > lookups and in inet_twsk_purge(), where the network namespace is validated
> > again after taking a reference. Use refcount_set_release() when setting
> > refcounts in the SYN cookie, TCP Fast Open, request migration, and
> > TIME_WAIT paths. Keep the existing TIME_WAIT smp_wmb(), which separately
> > orders the refcount store before ehash publication.
> > 
> > Fixes: fb041bb7c0a9 ("locking/refcount: Consolidate implementations of refcount_t")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > ---
> >  net/ipv4/inet_connection_sock.c | 6 +++---
> >  net/ipv4/inet_hashtables.c      | 2 +-
> >  net/ipv4/inet_timewait_sock.c   | 4 ++--
> >  net/ipv4/syncookies.c           | 2 +-
> >  net/ipv4/tcp_fastopen.c         | 2 +-
> >  net/ipv6/inet6_hashtables.c     | 2 +-
> >  6 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> > index bd0bad96c93c3..837cb91e5b173 100644
> > --- a/net/ipv4/inet_connection_sock.c
> > +++ b/net/ipv4/inet_connection_sock.c
> > @@ -1056,8 +1056,8 @@ static void reqsk_timer_handler(struct timer_list *t)
> >  		 * hold another count to prevent use-after-free and
> >  		 * call reqsk_put() just before return.
> >  		 */
> > -		refcount_set(&nreq->rsk_refcnt, 2 + 1);
> >  		timer_setup(&nreq->rsk_timer, reqsk_timer_handler, TIMER_PINNED);
> > +		refcount_set_release(&nreq->rsk_refcnt, 2 + 1);
> >  		reqsk_queue_migrated(&inet_csk(nsk)->icsk_accept_queue, req);
> >  
> >  		req = nreq;
> > @@ -1441,7 +1441,7 @@ struct sock *inet_csk_complete_hashdance(struct sock *sk, struct sock *child,
> >  				goto child_put;
> >  			}
> >  
> > -			refcount_set(&nreq->rsk_refcnt, 1);
> > +			refcount_set_release(&nreq->rsk_refcnt, 1);
> >  			if (inet_csk_reqsk_queue_add(sk, nreq, child)) {
> >  				__NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMIGRATEREQSUCCESS);
> >  				reqsk_migrate_reset(req);
> > @@ -1494,7 +1494,7 @@ void inet_csk_listen_stop(struct sock *sk)
> >  		if (nsk) {
> >  			nreq = inet_reqsk_clone(req, nsk);
> >  			if (nreq) {
> > -				refcount_set(&nreq->rsk_refcnt, 1);
> > +				refcount_set_release(&nreq->rsk_refcnt, 1);
> >  
> >  				rcu_read_lock();
> >  				if (inet_csk_reqsk_queue_add(nsk, nreq, child)) {
> > diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> > index 58d54965036a2..7ffa9171d6de1 100644
> > --- a/net/ipv4/inet_hashtables.c
> > +++ b/net/ipv4/inet_hashtables.c
> > @@ -550,7 +550,7 @@ struct sock *__inet_lookup_established(const struct net *net,
> >  		if (sk->sk_hash != hash)
> >  			continue;
> >  		if (likely(inet_match(net, sk, acookie, ports, dif, sdif))) {
> > -			if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
> > +			if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
> >  				goto out;
> >  			if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
> >  				     !inet_match(net, sk, acookie, ports, dif,
> > diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
> > index d4c781a0667fe..74ff40d42e56b 100644
> > --- a/net/ipv4/inet_timewait_sock.c
> > +++ b/net/ipv4/inet_timewait_sock.c
> > @@ -141,7 +141,7 @@ void inet_twsk_hashdance_schedule(struct inet_timewait_sock *tw,
> >  	 * Also note that after this point, we lost our implicit reference
> >  	 * so we are not allowed to use tw anymore.
> >  	 */
> > -	refcount_set(&tw->tw_refcnt, 3);
> > +	refcount_set_release(&tw->tw_refcnt, 3);
> >  
> >  	/* Ensure tw_refcnt has been set before tw is published.
> >  	 * smp_wmb() provides the necessary memory barrier to enforce this
> > @@ -328,7 +328,7 @@ void inet_twsk_purge(struct inet_hashinfo *hashinfo)
> >  			if (check_net(sock_net(sk)))
> >  				continue;
> >  
> > -			if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
> > +			if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
> >  				continue;
> >  
> >  			if (check_net(sock_net(sk))) {
> > diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> > index 6440305615378..4b2d2713653b7 100644
> > --- a/net/ipv4/syncookies.c
> > +++ b/net/ipv4/syncookies.c
> > @@ -205,7 +205,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
> >  	child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
> >  						 NULL, &own_req, NULL);
> >  	if (child) {
> > -		refcount_set(&req->rsk_refcnt, 1);
> > +		refcount_set_release(&req->rsk_refcnt, 1);
> >  		sock_rps_save_rxhash(child, skb);
> >  
> >  		if (rsk_drop_req(req)) {
> > diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> > index 471c78be55137..e40477b6b5549 100644
> > --- a/net/ipv4/tcp_fastopen.c
> > +++ b/net/ipv4/tcp_fastopen.c
> > @@ -364,7 +364,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
> >  	tcp_reset_xmit_timer(child, ICSK_TIME_RETRANS,
> >  			     req->timeout, false);
> >  
> > -	refcount_set(&req->rsk_refcnt, 2);
> > +	refcount_set_release(&req->rsk_refcnt, 2);
> >  
> >  	sk_mark_napi_id_set(child, skb);
> >  
> > diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
> > index aa5fa1dd062f7..1eea584d80df8 100644
> > --- a/net/ipv6/inet6_hashtables.c
> > +++ b/net/ipv6/inet6_hashtables.c
> > @@ -105,7 +105,7 @@ struct sock *__inet6_lookup_established(const struct net *net,
> >  			continue;
> >  		if (!inet6_match(net, sk, saddr, daddr, ports, dif, sdif))
> >  			continue;
> > -		if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
> > +		if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
> >  			goto out;
> >  
> >  		if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
> > -- 
> > 2.43.0
> > 

Re: [PATCH net v4 2/2] inet: restore refcount ordering for lockless ehash walks

[Kuniyuki Iwashima]

On Wed, 28 Oct 2026 12:29:10 +0900, Hyunwoo Kim wrote:
> On Wed, Oct 28, 2026 at 02:13:15AM +0000, Kuniyuki Iwashima wrote:
> > On Mon, 26 Oct 2026 09:37:45 +0900, Hyunwoo Kim wrote:
> > > Before commit fb041bb7c0a9 ("locking/refcount: Consolidate implementations
> > > of refcount_t"), default builds implemented refcount_inc_not_zero() using
> > > atomic_add_unless(), which provided full memory ordering on success. The
> > > commit removed that ordering on weakly ordered architectures, allowing
> > > validation after a successful reference acquisition in lockless ehash
> > > walks to be reordered before it.
> > > 
> > > Use refcount_inc_not_zero_acquire() in the IPv4 and IPv6 established
> > 
> > Does bpf_sk_assign() need it too ?
> > 
> > If the socket is taken from lookup helper, it's safe, but what
> > happens if the pointer is taken from BTF (27e5203bd9c5), e.g.
> > tp->inet_conn.icsk_accept_queue.rskq_accept_head ?
> 
> Yeah the same ordering issue remains when the pointer is obtained from 
> BTF without a reference. Fixes: 27e5203bd9c5.
> 
> However, I think this is better handled in a separate follow-up patch. 
> What do you think?

I think it can be just squashed given 27e5203bd9c5 exist
in the oldest LTS 5.10.


> 
> > 
> > 
> > > lookups and in inet_twsk_purge(), where the network namespace is validated
> > > again after taking a reference. Use refcount_set_release() when setting
> > > refcounts in the SYN cookie, TCP Fast Open, request migration, and
> > > TIME_WAIT paths. Keep the existing TIME_WAIT smp_wmb(), which separately
> > > orders the refcount store before ehash publication.
> > > 
> > > Fixes: fb041bb7c0a9 ("locking/refcount: Consolidate implementations of refcount_t")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > > ---
> > >  net/ipv4/inet_connection_sock.c | 6 +++---
> > >  net/ipv4/inet_hashtables.c      | 2 +-
> > >  net/ipv4/inet_timewait_sock.c   | 4 ++--
> > >  net/ipv4/syncookies.c           | 2 +-
> > >  net/ipv4/tcp_fastopen.c         | 2 +-
> > >  net/ipv6/inet6_hashtables.c     | 2 +-
> > >  6 files changed, 9 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> > > index bd0bad96c93c3..837cb91e5b173 100644
> > > --- a/net/ipv4/inet_connection_sock.c
> > > +++ b/net/ipv4/inet_connection_sock.c
> > > @@ -1056,8 +1056,8 @@ static void reqsk_timer_handler(struct timer_list *t)
> > >  		 * hold another count to prevent use-after-free and
> > >  		 * call reqsk_put() just before return.
> > >  		 */
> > > -		refcount_set(&nreq->rsk_refcnt, 2 + 1);
> > >  		timer_setup(&nreq->rsk_timer, reqsk_timer_handler, TIMER_PINNED);
> > > +		refcount_set_release(&nreq->rsk_refcnt, 2 + 1);
> > >  		reqsk_queue_migrated(&inet_csk(nsk)->icsk_accept_queue, req);
> > >  
> > >  		req = nreq;
> > > @@ -1441,7 +1441,7 @@ struct sock *inet_csk_complete_hashdance(struct sock *sk, struct sock *child,
> > >  				goto child_put;
> > >  			}
> > >  
> > > -			refcount_set(&nreq->rsk_refcnt, 1);
> > > +			refcount_set_release(&nreq->rsk_refcnt, 1);
> > >  			if (inet_csk_reqsk_queue_add(sk, nreq, child)) {
> > >  				__NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMIGRATEREQSUCCESS);
> > >  				reqsk_migrate_reset(req);
> > > @@ -1494,7 +1494,7 @@ void inet_csk_listen_stop(struct sock *sk)
> > >  		if (nsk) {
> > >  			nreq = inet_reqsk_clone(req, nsk);
> > >  			if (nreq) {
> > > -				refcount_set(&nreq->rsk_refcnt, 1);
> > > +				refcount_set_release(&nreq->rsk_refcnt, 1);
> > >  
> > >  				rcu_read_lock();
> > >  				if (inet_csk_reqsk_queue_add(nsk, nreq, child)) {
> > > diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> > > index 58d54965036a2..7ffa9171d6de1 100644
> > > --- a/net/ipv4/inet_hashtables.c
> > > +++ b/net/ipv4/inet_hashtables.c
> > > @@ -550,7 +550,7 @@ struct sock *__inet_lookup_established(const struct net *net,
> > >  		if (sk->sk_hash != hash)
> > >  			continue;
> > >  		if (likely(inet_match(net, sk, acookie, ports, dif, sdif))) {
> > > -			if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
> > > +			if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
> > >  				goto out;
> > >  			if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
> > >  				     !inet_match(net, sk, acookie, ports, dif,
> > > diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
> > > index d4c781a0667fe..74ff40d42e56b 100644
> > > --- a/net/ipv4/inet_timewait_sock.c
> > > +++ b/net/ipv4/inet_timewait_sock.c
> > > @@ -141,7 +141,7 @@ void inet_twsk_hashdance_schedule(struct inet_timewait_sock *tw,
> > >  	 * Also note that after this point, we lost our implicit reference
> > >  	 * so we are not allowed to use tw anymore.
> > >  	 */
> > > -	refcount_set(&tw->tw_refcnt, 3);
> > > +	refcount_set_release(&tw->tw_refcnt, 3);
> > >  
> > >  	/* Ensure tw_refcnt has been set before tw is published.
> > >  	 * smp_wmb() provides the necessary memory barrier to enforce this
> > > @@ -328,7 +328,7 @@ void inet_twsk_purge(struct inet_hashinfo *hashinfo)
> > >  			if (check_net(sock_net(sk)))
> > >  				continue;
> > >  
> > > -			if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
> > > +			if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
> > >  				continue;
> > >  
> > >  			if (check_net(sock_net(sk))) {
> > > diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> > > index 6440305615378..4b2d2713653b7 100644
> > > --- a/net/ipv4/syncookies.c
> > > +++ b/net/ipv4/syncookies.c
> > > @@ -205,7 +205,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
> > >  	child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
> > >  						 NULL, &own_req, NULL);
> > >  	if (child) {
> > > -		refcount_set(&req->rsk_refcnt, 1);
> > > +		refcount_set_release(&req->rsk_refcnt, 1);
> > >  		sock_rps_save_rxhash(child, skb);
> > >  
> > >  		if (rsk_drop_req(req)) {
> > > diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> > > index 471c78be55137..e40477b6b5549 100644
> > > --- a/net/ipv4/tcp_fastopen.c
> > > +++ b/net/ipv4/tcp_fastopen.c
> > > @@ -364,7 +364,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
> > >  	tcp_reset_xmit_timer(child, ICSK_TIME_RETRANS,
> > >  			     req->timeout, false);
> > >  
> > > -	refcount_set(&req->rsk_refcnt, 2);
> > > +	refcount_set_release(&req->rsk_refcnt, 2);
> > >  
> > >  	sk_mark_napi_id_set(child, skb);
> > >  
> > > diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
> > > index aa5fa1dd062f7..1eea584d80df8 100644
> > > --- a/net/ipv6/inet6_hashtables.c
> > > +++ b/net/ipv6/inet6_hashtables.c
> > > @@ -105,7 +105,7 @@ struct sock *__inet6_lookup_established(const struct net *net,
> > >  			continue;
> > >  		if (!inet6_match(net, sk, saddr, daddr, ports, dif, sdif))
> > >  			continue;
> > > -		if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
> > > +		if (unlikely(!refcount_inc_not_zero_acquire(&sk->sk_refcnt)))
> > >  			goto out;
> > >  
> > >  		if (unlikely(hlist_nulls_unhashed_lockless(&sk->sk_nulls_node) ||
> > > -- 
> > > 2.43.0
> > >