Re: [PATCH net-next v2 2/6] net: export sock_disable_timestamp() declaration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Lockless modification of `sk->sk_flags` from a BPF context will race with user context socket modifications, corrupting critical socket flags.
- [High] Concurrent lockless calls to `sock_disable_timestamp` result in a Time-of-Check to Time-of-Use (TOCTOU) race that unbalances the global timestamp reference counter.
--

commit 070236e8d76283be57936f36bb4d9ae98296d6e1
Author: Jason Xing <kerneljasonxing@gmail.com>

net: export sock_disable_timestamp() declaration

This commit removes the static label from sock_disable_timestamp() and
exports its declaration. This change allows the BPF timestamping RX feature
to dynamically turn off the global time record function.

> diff --git a/net/core/sock.c b/net/core/sock.c
> index f362e3ce1efb6..f3d78da3aeba3 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -474,7 +474,7 @@ static bool sock_needs_netstamp(const struct sock *sk)
>  	}
>  }
>  
> -static void sock_disable_timestamp(struct sock *sk, unsigned long flags)
> +void sock_disable_timestamp(struct sock *sk, unsigned long flags)
>  {
>  	if (sk->sk_flags & flags) {
>  		sk->sk_flags &= ~flags;

[Severity: Critical]
Since this function is being exported for use by the BPF timestamping rx
feature, will it be called from a lockless context?

If the BPF_SOCK_OPS_TSTAMP_RCV_CB hook calls this outside the socket lock
(for instance, after tcp_recvmsg() drops lock_sock(sk)), this non-atomic
read-modify-write of sk->sk_flags could race with user context socket
modifications like setsockopt().

Could this concurrent access cause torn reads and writes, permanently
corrupting sk_flags and potentially leading to use-after-free issues if
lifecycle flags like SOCK_DEAD are lost?

[Severity: High]
Could exporting this for lockless BPF contexts also result in a Time-of-Check
to Time-of-Use race that unbalances the global network timestamping reference
count?

If multiple threads execute recvmsg() on the same socket concurrently without
the socket lock, they might both evaluate "if (sk->sk_flags & flags)" as true,
clear the flags, and then subsequently call net_disable_timestamp().

Would this double-decrement unbalance the global counter and prematurely
disable network timestamping for all sockets on the system?

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