Re: [PATCH net v3 2/4] net: core: take the reuseport migration reference with acquire ordering

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] net: unnecessary refcount_inc_not_zero_acquire() in
  reuseport_migrate_sock()
--

commit 451ae80ccee42826773bf0a6549978307e27e9aa
Author: Hang Nan <2122295973@qq.com>

net: core: take the reuseport migration reference with acquire ordering

This commit switches reuseport_migrate_sock() from refcount_inc_not_zero()
to refcount_inc_not_zero_acquire() when taking a reference on the selected
listener socket.

> diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
> index b9e5648fa4c51..56230aa55a3e2 100644
> --- a/net/core/sock_reuseport.c
> +++ b/net/core/sock_reuseport.c
> @@ -667,7 +667,8 @@ struct sock *reuseport_migrate_sock(struct sock *sk,
>  	if (!nsk)
>  		nsk = reuseport_select_sock_by_hash(reuse, hash, socks);
>  
> -	if (IS_ERR_OR_NULL(nsk) || unlikely(!refcount_inc_not_zero(&nsk->sk_refcnt))) {
> +	if (IS_ERR_OR_NULL(nsk) ||
> +	    unlikely(!refcount_inc_not_zero_acquire(&nsk->sk_refcnt))) {
>  		nsk = NULL;
>  		goto failure;
>  	}

[Severity: Low]
Is refcount_inc_not_zero_acquire() needed here?

Listener teardown in __tcp_close() and tcp_disconnect() sets sk->sk_state
to TCP_CLOSE and runs inet_csk_listen_stop() while sk->sk_refcnt is still
non-zero, before the final sock_put().  An acquire barrier on
nsk->sk_refcnt only prevents reordering subsequent reads to before the
increment (which would observe older, pre-teardown state rather than
half-torn-down state), and does not serialize against concurrent listener
teardown; callers instead recheck sk->sk_state != TCP_LISTEN under
queue->rskq_lock in inet_csk_reqsk_queue_add().

Unlike the SLAB_TYPESAFE_BY_RCU lookups updated in 5b5d8f351dd9 ("net:
restore refcount ordering for lockless socket access"), sockets in a
reuseport group are freed via call_rcu() in sk_destruct() and cannot be
reallocated during the rcu_read_lock() section in reuseport_migrate_sock(),
nor does reuseport_migrate_sock() perform post-refcount validation checks
that need to be ordered after taking the reference.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_CCBF2706A4D1AFF8F48632FE8B56FB67E207@qq.com?part=2