net: annotate data-races around sk->sk_{data_ready,write_space}

skmsg (and probably other layers) are changing these pointers
while other cpus might read them concurrently.

Add corresponding READ_ONCE()/WRITE_ONCE() annotations
for UDP, TCP and AF_UNIX.

Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
Reported-by: syzbot+87f770387a9e5dc6b79b@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/699ee9fc.050a0220.1cd54b.0009.GAE@google.com/
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jakub Sitnicki <jakub@cloudflare.com>
Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20260225131547.1085509-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 2e26174c991970f194c86bce0382d810251bec26..3261793abe8332ed1c888bc519a27c9290dd483f 100644 (file)
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -1205,8 +1205,8 @@ void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock)
                return;
 
        psock->saved_data_ready = sk->sk_data_ready;
-       sk->sk_data_ready = sk_psock_strp_data_ready;
-       sk->sk_write_space = sk_psock_write_space;
+       WRITE_ONCE(sk->sk_data_ready, sk_psock_strp_data_ready);
+       WRITE_ONCE(sk->sk_write_space, sk_psock_write_space);
 }
 
 void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock)
@@ -1216,8 +1216,8 @@ void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock)
        if (!psock->saved_data_ready)
                return;
 
-       sk->sk_data_ready = psock->saved_data_ready;
-       psock->saved_data_ready = NULL;
+       WRITE_ONCE(sk->sk_data_ready, psock->saved_data_ready);
+       WRITE_ONCE(psock->saved_data_ready, NULL);
        strp_stop(&psock->strp);
 }
 
@@ -1296,8 +1296,8 @@ void sk_psock_start_verdict(struct sock *sk, struct sk_psock *psock)
                return;
 
        psock->saved_data_ready = sk->sk_data_ready;
-       sk->sk_data_ready = sk_psock_verdict_data_ready;
-       sk->sk_write_space = sk_psock_write_space;
+       WRITE_ONCE(sk->sk_data_ready, sk_psock_verdict_data_ready);
+       WRITE_ONCE(sk->sk_write_space, sk_psock_write_space);
 }
 
 void sk_psock_stop_verdict(struct sock *sk, struct sk_psock *psock)
@@ -1308,6 +1308,6 @@ void sk_psock_stop_verdict(struct sock *sk, struct sk_psock *psock)
        if (!psock->saved_data_ready)
                return;
 
-       sk->sk_data_ready = psock->saved_data_ready;
+       WRITE_ONCE(sk->sk_data_ready, psock->saved_data_ready);
        psock->saved_data_ready = NULL;
 }

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index f84d9a45cc9d2876aa3bb436ccbd23e4040d770f..8cdc26e8ad689154886a7ae66e0f375bebdfe035 100644 (file)
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1446,7 +1446,7 @@ out_err:
        err = sk_stream_error(sk, flags, err);
        /* make sure we wake any epoll edge trigger waiter */
        if (unlikely(tcp_rtx_and_write_queues_empty(sk) && err == -EAGAIN)) {
-               sk->sk_write_space(sk);
+               READ_ONCE(sk->sk_write_space)(sk);
                tcp_chrono_stop(sk, TCP_CHRONO_SNDBUF_LIMITED);
        }
        if (binding)
@@ -4181,7 +4181,7 @@ ao_parse:
                break;
        case TCP_NOTSENT_LOWAT:
                WRITE_ONCE(tp->notsent_lowat, val);
-               sk->sk_write_space(sk);
+               READ_ONCE(sk->sk_write_space)(sk);
                break;
        case TCP_INQ:
                if (val > 1 || val < 0)

diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index c449a044895e6e71c59937fb8ac8d0b7860a0552..813d2e498c93ab71b422506bf10d17eb45c71e9f 100644 (file)
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -725,7 +725,7 @@ int tcp_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool restore)
                        WRITE_ONCE(sk->sk_prot->unhash, psock->saved_unhash);
                        tcp_update_ulp(sk, psock->sk_proto, psock->saved_write_space);
                } else {
-                       sk->sk_write_space = psock->saved_write_space;
+                       WRITE_ONCE(sk->sk_write_space, psock->saved_write_space);
                        /* Pairs with lockless read in sk_clone_lock() */
                        sock_replace_proto(sk, psock->sk_proto);
                }

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 41c57efd125caa124ac5ae135868515348587c56..6404e53382ca93d7972a8da7fbd4f66bad2803c4 100644 (file)
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5425,7 +5425,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 
        if (unlikely(tcp_try_rmem_schedule(sk, skb, skb->truesize))) {
                NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP);
-               sk->sk_data_ready(sk);
+               READ_ONCE(sk->sk_data_ready)(sk);
                tcp_drop_reason(sk, skb, SKB_DROP_REASON_PROTO_MEM);
                return;
        }
@@ -5635,7 +5635,7 @@ err:
 void tcp_data_ready(struct sock *sk)
 {
        if (tcp_epollin_ready(sk, sk->sk_rcvlowat) || sock_flag(sk, SOCK_DONE))
-               sk->sk_data_ready(sk);
+               READ_ONCE(sk->sk_data_ready)(sk);
 }
 
 static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
@@ -5691,7 +5691,7 @@ queue_and_out:
                        inet_csk(sk)->icsk_ack.pending |=
                                        (ICSK_ACK_NOMEM | ICSK_ACK_NOW);
                        inet_csk_schedule_ack(sk);
-                       sk->sk_data_ready(sk);
+                       READ_ONCE(sk->sk_data_ready)(sk);
 
                        if (skb_queue_len(&sk->sk_receive_queue) && skb->len) {
                                reason = SKB_DROP_REASON_PROTO_MEM;
@@ -6114,7 +6114,9 @@ static void tcp_new_space(struct sock *sk)
                tp->snd_cwnd_stamp = tcp_jiffies32;
        }
 
-       INDIRECT_CALL_1(sk->sk_write_space, sk_stream_write_space, sk);
+       INDIRECT_CALL_1(READ_ONCE(sk->sk_write_space),
+                       sk_stream_write_space,
+                       sk);
 }
 
 /* Caller made space either from:
@@ -6325,7 +6327,7 @@ static void tcp_urg(struct sock *sk, struct sk_buff *skb, const struct tcphdr *t
                                BUG();
                        WRITE_ONCE(tp->urg_data, TCP_URG_VALID | tmp);
                        if (!sock_flag(sk, SOCK_DEAD))
-                               sk->sk_data_ready(sk);
+                               READ_ONCE(sk->sk_data_ready)(sk);
                }
        }
 }
@@ -7792,7 +7794,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
                        sock_put(fastopen_sk);
                        goto drop_and_free;
                }
-               sk->sk_data_ready(sk);
+               READ_ONCE(sk->sk_data_ready)(sk);
                bh_unlock_sock(fastopen_sk);
                sock_put(fastopen_sk);
        } else {

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index d9c5a43bd2818c17d79d8433ac5dd2bdd199b812..dafb63b923d0d08cb1a0e9a37d8ec025386a960a 100644 (file)
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -1004,7 +1004,7 @@ enum skb_drop_reason tcp_child_process(struct sock *parent, struct sock *child,
                reason = tcp_rcv_state_process(child, skb);
                /* Wakeup parent, send SIGIO */
                if (state == TCP_SYN_RECV && child->sk_state != state)
-                       parent->sk_data_ready(parent);
+                       READ_ONCE(parent->sk_data_ready)(parent);
        } else {
                /* Alas, it is possible again, because we do lookup
                 * in main socket hash table and lock on listening

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 6c6b68a66dcd3b3d8f1747fead868c195e04a0a9..014fdfdd331b98491b02ff4252a493926a60d3b7 100644 (file)
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1787,7 +1787,7 @@ int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
                 * using prepare_to_wait_exclusive().
                 */
                while (nb) {
-                       INDIRECT_CALL_1(sk->sk_data_ready,
+                       INDIRECT_CALL_1(READ_ONCE(sk->sk_data_ready),
                                        sock_def_readable, sk);
                        nb--;
                }

diff --git a/net/ipv4/udp_bpf.c b/net/ipv4/udp_bpf.c
index 91233e37cd97a2b0f0c891723b20b1235cc628d1..779a3a03762f1e3ff4da40597c9e4d7a7a4f04d6 100644 (file)
--- a/net/ipv4/udp_bpf.c
+++ b/net/ipv4/udp_bpf.c
@@ -158,7 +158,7 @@ int udp_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool restore)
        int family = sk->sk_family == AF_INET ? UDP_BPF_IPV4 : UDP_BPF_IPV6;
 
        if (restore) {
-               sk->sk_write_space = psock->saved_write_space;
+               WRITE_ONCE(sk->sk_write_space, psock->saved_write_space);
                sock_replace_proto(sk, psock->sk_proto);
                return 0;
        }

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 3756a93dc63aabc814f7edd31e8c0b6fafa02ce7..7eaa5b187fef4d7ad067d10c3560726f64c1295a 100644 (file)
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1785,7 +1785,7 @@ restart:
        __skb_queue_tail(&other->sk_receive_queue, skb);
        spin_unlock(&other->sk_receive_queue.lock);
        unix_state_unlock(other);
-       other->sk_data_ready(other);
+       READ_ONCE(other->sk_data_ready)(other);
        sock_put(other);
        return 0;
 
@@ -2278,7 +2278,7 @@ restart_locked:
        scm_stat_add(other, skb);
        skb_queue_tail(&other->sk_receive_queue, skb);
        unix_state_unlock(other);
-       other->sk_data_ready(other);
+       READ_ONCE(other->sk_data_ready)(other);
        sock_put(other);
        scm_destroy(&scm);
        return len;
@@ -2351,7 +2351,7 @@ static int queue_oob(struct sock *sk, struct msghdr *msg, struct sock *other,
 
        sk_send_sigurg(other);
        unix_state_unlock(other);
-       other->sk_data_ready(other);
+       READ_ONCE(other->sk_data_ready)(other);
 
        return 0;
 out_unlock:
@@ -2477,7 +2477,7 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
                spin_unlock(&other->sk_receive_queue.lock);
 
                unix_state_unlock(other);
-               other->sk_data_ready(other);
+               READ_ONCE(other->sk_data_ready)(other);
                sent += size;
        }