Re: [PATCH bpf] bpf, sockmap: use READ_ONCE/WRITE_ONCE for saved_data_ready in the verdict path

[Jakub Sitnicki <jakub@cloudflare.com>]

On Wed, Oct 21, 2026 at 11:56 PM +08, Dudu Lu wrote:
> sk_psock_start_verdict() and sk_psock_stop_verdict() access
> psock->saved_data_ready with plain loads and stores, while the sibling
> sk_psock_stop_strp() uses WRITE_ONCE for the same field.  sock_map_link()
> / sock_map_unref() touch the same psock from another CPU without the
> lock, so KCSAN reports the race:
>
>   BUG: KCSAN: data-race in sock_map_link / sock_map_unref
>   BUG: KCSAN: data-race in sk_psock_start_verdict / sock_map_unref
>
> Use READ_ONCE() for the reads and WRITE_ONCE() for the writes, matching
> the strp sibling.
>
> Fixes: 2ef2b20cf4e0 ("net: annotate data-races around sk->sk_{data_ready,write_space}")
> Assisted-by: LLM
> Signed-off-by: Dudu Lu <phx0fer@gmail.com>
> ---
> Tested base
> -----------
> Detected on Linux 7.3-rc4 (commit 93f51579e7df), arm64 QEMU guest.
> Re-checked against mainline f47d223627f8 (v7.3-341): the affected code
> is unchanged between the two, so the bug is still present there.
> Checked against linux-next next-20261021 (commit 9a482184d910): the
> affected code is unchanged there as well, so the bug is still present in
> linux-next too.
>
> Reproducer status
> -----------------
> A reproducer was written and executed on an arm64 QEMU guest (KCSAN); it
> reported the race.  The reproducer source is not posted here per
> Documentation/process/security-bugs.rst; it is available on request.
>
> Triggering conditions
> ---------------------
> CAP_BPF / the ability to update sockmap and attach verdict programs.
>
> Notes
> -----
> Built and checkpatch-clean: this patch was applied on top of 7.3-rc4
> (commit 93f51579e7df) and a full arm64 kernel (defconfig) was built with
> it -- no new warnings or errors.  scripts/checkpatch.pl reports 0 errors,
> 0 warnings.
>
>  net/core/skmsg.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> index df385a5a961e..0bfb8c4e7f36 100644
> --- a/net/core/skmsg.c
> +++ b/net/core/skmsg.c
> @@ -1260,10 +1260,10 @@ static void sk_psock_verdict_data_ready(struct sock *sk)
>  
>  void sk_psock_start_verdict(struct sock *sk, struct sk_psock *psock)
>  {
> -	if (psock->saved_data_ready)
> +	if (READ_ONCE(psock->saved_data_ready))
>  		return;

Okay, but if we're going down that route, now sk_psock_stop_strp() is
missing a READ_ONCE around psock->saved_data_ready, or rather we should
do it once and use it through the whole function:

void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock)
{
	psock_set_prog(&psock->progs.stream_parser, NULL);

	if (!psock->saved_data_ready)
		return;

	WRITE_ONCE(sk->sk_data_ready, psock->saved_data_ready);
	WRITE_ONCE(psock->saved_data_ready, NULL);
	strp_stop(&psock->strp);
}

>  
> -	psock->saved_data_ready = sk->sk_data_ready;
> +	WRITE_ONCE(psock->saved_data_ready, sk->sk_data_ready);

I belive you also need to READ_ONCE(sk->sk_data_ready).

>  	WRITE_ONCE(sk->sk_data_ready, sk_psock_verdict_data_ready);
>  	WRITE_ONCE(sk->sk_write_space, sk_psock_write_space);
>  }
> @@ -1273,9 +1273,9 @@ void sk_psock_stop_verdict(struct sock *sk, struct sk_psock *psock)
>  	psock_set_prog(&psock->progs.stream_verdict, NULL);
>  	psock_set_prog(&psock->progs.skb_verdict, NULL);
>  
> -	if (!psock->saved_data_ready)
> +	if (!READ_ONCE(psock->saved_data_ready))
>  		return;
>  
>  	WRITE_ONCE(sk->sk_data_ready, psock->saved_data_ready);
> -	psock->saved_data_ready = NULL;
> +	WRITE_ONCE(psock->saved_data_ready, NULL);
>  }