Re: [PATCH bpf v2 2/2] bpf: Unconditionally take socket references in lookup helpers

On Tue, Aug 04, 2026 at 09:01 PM -07, Kuniyuki Iwashima wrote:

On Tue, Aug 4, 2026 at 3:14 AM Jakub Sitnicki [off-list ref] wrote:

quoted

On Mon, Aug 03, 2026 at 11:00 AM +02, Michal Luczaj wrote:

quoted

Lookup helpers gate whether to acquire a socket reference on
sk_is_refcounted(), a check re-evaluated at release. An established socket
refcounted at acquire time can gain SOCK_RCU_FREE via
connect(AF_UNSPEC)+listen() before release runs; the release-side re-check
then reads sk_is_refcounted() == false and skips the put. The reference
leaks.

Make acquire and release unconditional and symmetric: always take a
reference, always put it. Adapt sk_select_reuseport().

Fixes: 6acc9b432e67 ("bpf: Add helper to retrieve socket in BPF")
Fixes: 64d85290d79c ("bpf: Allow bpf_map_lookup_elem for SOCKMAP and SOCKHASH")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/bpf/20260701235552.2B0AA1F00A3F@smtp.kernel.org/ (local)
Signed-off-by: Michal Luczaj <redacted>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
---
TC bpf_sk_assign() has the same issue; it takes a reference only when
sk_is_refcounted() is true at assign time, but sock_pfree() (the skb
destructor it installs) re-checks sk_is_refcounted() independently at
release time. The same connect(AF_UNSPEC)+listen() transition leaks the
socket here too. I'd welcome suggestions on the right way to handle this.

Can we make this scenario unsupported?

listen() could return EBUSY if called on a socket that is refcounted.

WDYT?

I discussed this kind of buggy rehash with Eric today.

We can't make it unsupported although it's super unlikely
that this is used by a real application.

I'm just wondering why not?

First I thought is was due to POSIX compatibility but POSIX seems to
define connect(AF_UNSPEC) only for connection-less sockets [1]:

"""
If he initiating socket is not connection-mode, then connect() shall set
the socket's peer address [...] If the sa_family member of address is
AF_UNSPEC, the socket's peer address shall be reset.
"""

So if this is Linux-specific behavior (?) and we don't exect any users
to rely on it, why not change it and see if anyone complains?

Seems like wasted effort to try to make it work properly.

[1] https://man.archlinux.org/man/connect.3p