Re: [PATCH bpf v2] bpf, sockmap: annotate the psock teardown data races

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Wed, Oct 21, 2026 at 10:48 PM Dudu Lu <phx0fer@gmail.com> wrote:
> Annotate every one of these accesses with READ_ONCE()/WRITE_ONCE(),
> matching the existing convention for the same fields: sk->sk_data_ready

This makes KCSAN quiet and leaves the race where it is.

sock_map_link() runs under the sock lock. sock_map_delete_elem() doesn't
take it. sk_callback_lock is the only lock they share and sock_map_link()
takes it after psock_set_prog() and sock_map_init_proto().
So two tcp_bpf_update_proto(sk, psock, false) can run in parallel:
  cpu0: sock_map_link() for a map with msg_parser sees
        psock->progs.stream_verdict and picks TCP_BPF_TXRX
  cpu1: sock_map_del_link() stops the verdict and stores TCP_BPF_TX
  cpu0: stores TCP_BPF_TXRX
The socket is left with tcp_bpf_recvmsg_parser() and no verdict prog.
That's what commit c0d95d3380ee fixed.
Take the lock earlier. Then psock_update_sk_prot needs no annotation.

All accesses to saved_data_ready are under sk_callback_lock except the
three reads in sock_map_del_link(). Move that check under the lock instead
of annotating it. No need to touch skmsg.c and skmsg.h.

Only the two lockless reads of sk->sk_prot need READ_ONCE().

Something like this (compile tested only):

diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 38df842..20fb605 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -152,11 +152,11 @@ static void sock_map_del_link(struct sock *sk,
 			struct bpf_map *map = link->map;
 			struct sk_psock_progs *progs = sock_map_progs(map);

-			if (psock->saved_data_ready && progs->stream_parser)
+			if (progs->stream_parser)
 				strp_stop = true;
-			if (psock->saved_data_ready && progs->stream_verdict)
+			if (progs->stream_verdict)
 				verdict_stop = true;
-			if (psock->saved_data_ready && progs->skb_verdict)
+			if (progs->skb_verdict)
 				verdict_stop = true;
 			list_del(&link->list);
 			sk_psock_free_link(link);
@@ -164,8 +164,11 @@ static void sock_map_del_link(struct sock *sk,
 		}
 	}
 	spin_unlock_bh(&psock->link_lock);
-	if (strp_stop || verdict_stop) {
-		write_lock_bh(&sk->sk_callback_lock);
+	if (!strp_stop && !verdict_stop)
+		return;
+
+	write_lock_bh(&sk->sk_callback_lock);
+	if (psock->saved_data_ready) {
 		if (strp_stop)
 			sk_psock_stop_strp(sk, psock);
 		if (verdict_stop)
@@ -173,8 +176,8 @@ static void sock_map_del_link(struct sock *sk,

 		if (psock->psock_update_sk_prot)
 			psock->psock_update_sk_prot(sk, psock, false);
-		write_unlock_bh(&sk->sk_callback_lock);
 	}
+	write_unlock_bh(&sk->sk_callback_lock);
 }

 static void sock_map_unref(struct sock *sk, void *link_raw)
@@ -202,7 +205,7 @@ static struct sk_psock *sock_map_psock_get_checked(struct sock *sk)
 	rcu_read_lock();
 	psock = sk_psock(sk);
 	if (psock) {
-		if (sk->sk_prot->close != sock_map_close) {
+		if (READ_ONCE(sk->sk_prot)->close != sock_map_close) {
 			psock = ERR_PTR(-EBUSY);
 			goto out;
 		}
@@ -284,6 +287,7 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
 		}
 	}

+	write_lock_bh(&sk->sk_callback_lock);
 	if (msg_parser)
 		psock_set_prog(&psock->progs.msg_parser, msg_parser);
 	if (stream_parser)
@@ -298,11 +302,11 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
 	 */
 	ret = sock_map_init_proto(sk, psock);
 	if (ret < 0) {
+		write_unlock_bh(&sk->sk_callback_lock);
 		sk_psock_put(sk, psock);
 		goto out;
 	}

-	write_lock_bh(&sk->sk_callback_lock);
 	if (stream_parser && stream_verdict && !psock->saved_data_ready) {
 		if (sk_is_tcp(sk))
 			ret = sk_psock_init_strp(sk, psock);
@@ -536,7 +540,7 @@ static bool sock_map_redirect_allowed(const struct sock *sk)

 static bool sock_map_sk_is_suitable(const struct sock *sk)
 {
-	return !!sk->sk_prot->psock_update_sk_prot;
+	return !!READ_ONCE(sk->sk_prot)->psock_update_sk_prot;
 }

 static bool sock_map_sk_state_allowed(const struct sock *sk)

Pls run your reproducer on it, post the reproducer with the respin
and target bpf-next.

pw-bot: cr