Re: [PATCH v1 bpf/net 2/6] sockmap: Annotate sk->sk_write_space() for UDP.

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2070 bytes --]

> diff --git a/net/core/sock.c b/net/core/sock.c
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2690,7 +2690,7 @@ void sock_wfree(struct sk_buff *skb)
>  		 * after sk_write_space() call
>  		 */
>  		WARN_ON(refcount_sub_and_test(len - 1, &sk->sk_wmem_alloc));
> -		sk->sk_write_space(sk);
> +		READ_ONCE(sk->sk_write_space)(sk);
>  		len = 1;
>  	}

This annotates the invocation of sk->sk_write_space, but earlier
in the same function there is another plain read used for the
fast-path comparison:

net/core/sock.c:sock_wfree() {
    ...
    if (sock_flag(sk, SOCK_RCU_FREE) &&
        sk->sk_write_space == sock_def_write_space) {
    ...
}

Should this comparison also use READ_ONCE()?  It reads the same
field that this commit identifies as locklessly accessed, and a
plain read here would still be flagged by KCSAN when racing with
the WRITE_ONCE() added to sk_psock_start_verdict().

Additionally, udp_bpf_update_proto() restores sk->sk_write_space
on the teardown path without WRITE_ONCE():

net/ipv4/udp_bpf.c:udp_bpf_update_proto() {
    if (restore) {
        sk->sk_write_space = psock->saved_write_space;
        ...
    }
}

This write can race with sock_wfree() reading sk->sk_write_space
for a UDP socket, since sock_wfree() does not hold
sk->sk_callback_lock.  Should this also use WRITE_ONCE() to
pair with the READ_ONCE() added here?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22042899757

AI-authorship-score: low
AI-authorship-explanation: The commit is a small, focused data race annotation fix consistent with the author's established kernel networking contributions and standard kernel style.
issues-found: 2
issue-severity-score: low
issue-severity-explanation: Incomplete data race annotations leave additional plain accesses to sk->sk_write_space unannotated, which can still trigger KCSAN warnings but are unlikely to cause user-visible failures.