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

On Tue, Aug 4, 2026 at 3:14 AM Jakub Sitnicki <[email protected]> wrote:
>
> On Mon, Aug 03, 2026 at 11:00 AM +02, Michal Luczaj wrote:
> > 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 <[email protected]>
> > Closes: https://lore.kernel.org/bpf/[email protected]/
> > Signed-off-by: Michal Luczaj <[email protected]>
> > Reviewed-by: Emil Tsalapatis <[email protected]>
> > ---
> > 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.