When commit 3ab5aee7fe84 ("net: Convert TCP & DCCP hash tables to use RCU / hlist_nulls") was written, atomic_inc_not_zero() provided full memory ordering. Even after commit 41c6d650f653 ("net: convert sock.sk_refcnt from atomic_t to refcount_t"), default builds used atomic_add_unless() and retained that ordering. Commit fb041bb7c0a9 ("locking/refcount: Consolidate implementations of refcount_t") consolidated the refcount_t implementations in v5.5. On weakly ordered architectures, this removed the ordering required by validation after a successful reference acquisition. Use refcount_inc_not_zero_acquire() in the IPv4 and IPv6 established lookups. Use refcount_set_release() when setting initial refcounts in sk_clone(), SYN cookie, and request migration paths. In the TCP Fast Open path, move the refcount update after the required initialization and use release ordering. The TIME_WAIT path already has an smp_wmb() between refcount_set() and hash publication, so leave it unchanged. Fixes: fb041bb7c0a9 ("locking/refcount: Consolidate implementations of refcount_t") Cc: stable@vger.kernel.org Signed-off-by: Hyunwoo Kim --- net/core/sock.c | 3 +-- net/ipv4/inet_connection_sock.c | 6 +++--- net/ipv4/inet_hashtables.c | 2 +- net/ipv4/syncookies.c | 2 +- net/ipv4/tcp_fastopen.c | 3 +-- net/ipv6/inet6_hashtables.c | 2 +- 6 files changed, 8 insertions(+), 10 deletions(-) diff --git a/net/core/sock.c b/net/core/sock.c index 3c25d66ba31a..f3ef9601ab73 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -2622,8 +2622,7 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority, /* Before updating sk_refcnt, we must commit prior changes to memory * (Documentation/RCU/rculist_nulls.rst for details) */ - smp_wmb(); - refcount_set(&newsk->sk_refcnt, 2); + refcount_set_release(&newsk->sk_refcnt, 2); sk_set_socket(newsk, NULL); sk_tx_queue_clear(newsk); diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index cff02f4c5ac8..2d8e54d3a31b 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 58d54965036a..7ffa9171d6de 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/syncookies.c b/net/ipv4/syncookies.c index 644030561537..4b2d2713653b 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 471c78be5513..2506f415f410 100644 --- a/net/ipv4/tcp_fastopen.c +++ b/net/ipv4/tcp_fastopen.c @@ -364,8 +364,6 @@ 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); - sk_mark_napi_id_set(child, skb); /* Now finish processing the fastopen child socket. */ @@ -378,6 +376,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk, tcp_rsk(req)->rcv_nxt = tp->rcv_nxt; tp->rcv_wup = tp->rcv_nxt; tp->rcv_mwnd_seq = tp->rcv_wup + tp->rcv_wnd; + refcount_set_release(&req->rsk_refcnt, 2); /* tcp_conn_request() is sending the SYNACK, * and queues the child into listener accept queue. */ diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c index aa5fa1dd062f..1eea584d80df 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