Re: [PATCH bpf v4 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 patch prevents a refcount leak for UDP sockets by rejecting unhashed sockets, but an identical reference count leak can be triggered for TCP sockets because `sk_is_refcounted()` relies on the mutable `SOCK_RCU_FREE` flag.
--

--- Patch [2]: [PATCH bpf v4 2/4] bpf, sockmap: Reject unhashed UDP sockets on sockmap update ---
commit efe688dfced966a49066087e860ab73696a8f75a
Author: Michal Luczaj <mhal@rbox.co>
Date:   Tue Jul 7 04:23:57 2026 +0000

bpf, sockmap: Reject unhashed UDP sockets on sockmap update

This patch addresses a socket reference count leak by preventing unhashed UDP
sockets from being updated in sockmaps. It removes the ability for BPF programs
to increment a refcount on an unbound socket that subsequently gets bound and
skips the refcount decrement upon release.

> diff --git a/net/core/sock_map.c b/net/core/sock_map.c
> --- 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 is a pre-existing issue, but can the exact same reference count leak
happen for TCP sockets because sk_is_refcounted() relies on the mutable
SOCK_RCU_FREE flag?

If a user creates a TCP socket, connects it (entering TCP_ESTABLISHED), and
adds it to a sockmap, a BPF program can call bpf_map_lookup_elem() on the
sockmap. This increments the socket's refcount because SOCK_RCU_FREE is not
set on established sockets.

If a user thread concurrently calls connect(AF_UNSPEC) to disconnect the
socket (moving it to TCP_CLOSE and removing it from the map), and then calls
listen() on the same socket, inet_hash() unconditionally sets the
SOCK_RCU_FREE flag.

When the BPF program subsequently calls bpf_sk_release(sk):

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

sk_is_refcounted(sk) will now return false because SOCK_RCU_FREE is set,
causing bpf_sk_release() to skip the decrement and permanently leak the
acquired reference. 

Could this sequence be abused to cause memory exhaustion, and is there a way
to safely track the acquired references without relying on a flag that might
change mid-flight?

> +	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/20260707-sockmap-lookup-udp-leak-v4-0-f878346f27ab@rbox.co?part=2