Re: [PATCH bpf-next 3/7] bpf: tcp: Support bpf_sock_ops_cb_flags_set() for bpf_tcp_ops.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: data race in bpf_tcp_ops_cb_flags_set() on unlocked sockets
--

commit 56d7f542a8c499237807954e17f2fae35e5e276b
Author: Kuniyuki Iwashima <kuniyu@google.com>

bpf: tcp: Support bpf_sock_ops_cb_flags_set() for bpf_tcp_ops.

This patch allows clearing or setting bpf_sock_ops_cb_flags via the
bpf_sock_ops_cb_flags_set() helper within various bpf_tcp_ops callbacks.

> diff --git a/net/ipv4/bpf_tcp_ops.c b/net/ipv4/bpf_tcp_ops.c
> index c68d1fa323058..6d0452441b6c5 100644
> --- a/net/ipv4/bpf_tcp_ops.c
> +++ b/net/ipv4/bpf_tcp_ops.c
[ ... ]
> @@ -220,6 +220,24 @@ const struct bpf_func_proto bpf_tcp_ops_get_retval_proto = {
>  	.ret_type	= RET_INTEGER,
>  };
>  
> +BPF_CALL_2(bpf_tcp_ops_cb_flags_set, struct sock *, sk, int, argval)
> +{
> +	int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS;
> +
> +	tcp_sk(sk)->bpf_sock_ops_cb_flags = val;
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
If an unlocked socket is passed to bpf_tcp_ops_cb_flags_set(), it looks like
concurrent updates to the flags from other CPUs could be clobbered, causing
BPF TCP callbacks to fail to invoke when expected.

Does this code modify the socket flags without acquiring the socket lock
or asserting that the socket is currently owned by the caller?

> +
> +	return argval & ~BPF_SOCK_OPS_ALL_CB_FLAGS;
> +}
> +
> +static const struct bpf_func_proto bpf_tcp_ops_cb_flags_set_proto = {
> +	.func		= bpf_tcp_ops_cb_flags_set,
> +	.gpl_only	= false,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_PTR_TO_BTF_ID,
> +	.arg1_btf_id	= &btf_sock_ids[BTF_SOCK_TYPE_TCP],
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Since the verifier accepts any PTR_TO_BTF_ID of type TCP here, it seems
privileged BPF programs could pass refcounted but unlocked sockets, leading
to the data race mentioned above.

Could the use of ARG_PTR_TO_BTF_ID allow a BPF program to pass an unlocked
socket obtained via helpers like bpf_sk_lookup_tcp() and bpf_skc_to_tcp_sock()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920195633.3033620-1-kuniyu@google.com?part=3