Re: [PATCH bpf v3 2/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The refcount leak fixed for UDP sockets by this patch still exists for TCP sockets (and via bpf_sk_lookup_tcp) because bpf_sk_release relies on the mutable SOCK_RCU_FREE flag.
--

commit 120f1940dc9e4cc498f383925d488cf77882caea
Author: Michal Luczaj <mhal@rbox.co>

bpf, sockmap: Reject unhashed UDP sockets on sockmap update

This commit rejects unhashed UDP sockets during sockmap updates to prevent
a memory leak when a socket transitions from unbound to bound. This transition
causes bpf_sk_release() to skip the refcount decrement due to the SOCK_RCU_FREE
flag being set.

> diff --git a/net/core/sock_map.c b/net/core/sock_map.c
> index c60ba6d292f9a..9efbd8ca7db83 100644
> --- a/net/core/sock_map.c
> +++ b/net/core/sock_map.c
> @@ -542,6 +542,8 @@ static bool sock_map_sk_state_allowed(const struct sock *sk)
>  {
>  	if (sk_is_tcp(sk))
>  		return (1 << sk->sk_state) & (TCPF_ESTABLISHED | TCPF_LISTEN);

[Severity: High]
This isn't a bug introduced by this patch, but does the same refcount leak
still exist for TCP sockets?

A BPF program can acquire a reference to a TCP socket in the TCP_ESTABLISHED
state (e.g., via bpf_sk_lookup_tcp or bpf_map_lookup_elem on a sockmap) because
TCP_ESTABLISHED sockets do not have the SOCK_RCU_FREE flag set.

If a user-space thread concurrently calls connect(AF_UNSPEC) to transition the
socket to TCP_CLOSE, followed by listen(), inet_hash() will dynamically set the
SOCK_RCU_FREE flag on the socket.

When the BPF program finishes and calls bpf_sk_release():

net/core/filter.c:bpf_sk_release() {
	if (sk && sk_is_refcounted(sk))
		sock_gen_put(sk);
	return 0;
}

Because the flag was just set, sk_is_refcounted(sk) will dynamically evaluate
to false, skipping sock_gen_put(sk) and permanently leaking the acquired
reference.

> +	if (sk_is_udp(sk))
> +		return sk_hashed(sk);
>  	if (sk_is_stream_unix(sk))
>  		return (1 << READ_ONCE(sk->sk_state)) & TCPF_ESTABLISHED;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-sockmap-lookup-udp-leak-v3-0-ff8de8782468@rbox.co?part=2