Re: [PATCH bpf] bpf, sockmap: use READ_ONCE/WRITE_ONCE for saved_data_ready in the verdict path
flat view
From: Jakub Sitnicki <jakub@cloudflare.com>
Date: 2026-10-21 20:31:00
Also in:
bpf
On Wed, Oct 21, 2026 at 11:56 PM +08, Dudu Lu wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- 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).
quoted hunk ↗ jump to hunk
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); }