Re: [PATCH net-next v2 3/6] bpf: support bpf_setsockopt for bpf timestamping rx feature

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lockless invocation of `sk_bpf_set_get_cb_flags` causes `sk_flags` corruption and global netstamp refcount leaks via `sock_enable_timestamp()` / `sock_disable_timestamp()`.
--

commit 3f400ad3640a19655fbde51eedae40506ebf95a8
Author: Jason Xing <kerneljasonxing@gmail.com>

bpf: support bpf_setsockopt for bpf timestamping rx feature

Add SK_BPF_CB_RX_TIMESTAMPING callback flag to enable RX timestamping via
bpf_setsockopt. It updates sk_flags centrally to manage the global network
timestamp static keys.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9590877b0714f..08ad102f204e0 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5304,6 +5304,14 @@ static int sk_bpf_set_get_cb_flags(struct sock *sk, char *optval, bool getopt)
>  	if (sk_bpf_cb_flags & ~SK_BPF_CB_MASK)
>  		return -EINVAL;
>  
> +	if ((sk_bpf_cb_flags ^ sk->sk_bpf_cb_flags) & SK_BPF_CB_RX_TIMESTAMPING) {
> +		if (sk_bpf_cb_flags & SK_BPF_CB_RX_TIMESTAMPING)
> +			sock_enable_timestamp(sk, SOCK_BPF_TIMESTAMPING_RX);
> +		else
> +			sock_disable_timestamp(sk,
> +					       (1UL << SOCK_BPF_TIMESTAMPING_RX));
> +	}

[Severity: High]
Could this lead to socket flags corruption if called locklessly?

sk_bpf_set_get_cb_flags() can be reached without the socket lock held, such
as through the following call chain:

bpf_unlocked_sk_setsockopt()
  __bpf_setsockopt()
    sk_bpf_set_get_cb_flags()

Since sock_enable_timestamp() and sock_disable_timestamp() use non-atomic
bitwise operations to modify sk->sk_flags, concurrent execution might race
and accidentally drop critical flags like SOCK_DEAD or SOCK_DONE.

Additionally, because the checks against previous_flags in these functions
are not atomic with the bitwise updates, could a race here lead to unbalanced
net_enable_timestamp() or net_disable_timestamp() calls, permanently skewing
the global netstamp static key refcount?

> +
>  	sk->sk_bpf_cb_flags = sk_bpf_cb_flags;
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521135244.40869-1-kerneljasonxing@gmail.com?part=3