[PATCH bpf v2 0/2] bpf: Fix socket leaks around connect(AF_UNSPEC)+listen()

[PATCH bpf v2 0/2] bpf: Fix socket leaks around connect(AF_UNSPEC)+listen()

[Michal Luczaj]

This is a follow-up to Sashiko's report[1].

Several BPF socket helpers acquire a socket reference only when
sk_is_refcounted() == true, and release it, independently, by
re-evaluating sk_is_refcounted() again at the time the release runs. TCP
connect(AF_UNSPEC)+listen() sets SOCK_RCU_FREE on an established socket.
If that happens while a reference is outstanding, the release side sees
sk_is_refcounted() == false and skips the put; the socket is leaked.

unreferenced object 0xffff88811617ce00 (size 3200):
  comm "softirq", pid 0, jiffies 4294848512
  hex dump (first 32 bytes):
    7f 00 00 01 7f 00 00 01 4d 43 02 f6 00 00 00 00  ........MC......
    02 00 07 41 00 00 00 00 00 00 00 00 00 00 00 00  ...A............
  backtrace (crc fb5bd4c8):
    kmem_cache_alloc_noprof+0x53e/0x640
    sk_prot_alloc+0x69/0x240
    sk_clone+0x79/0x1230
    inet_csk_clone_lock+0x30/0x760
    tcp_create_openreq_child+0x34/0x2750
    tcp_v4_syn_recv_sock+0x12e/0x1080
    tcp_check_req+0x447/0x2310
    tcp_v4_rcv+0x1026/0x3c90
    ip_protocol_deliver_rcu+0x93/0x340
    ip_local_deliver_finish+0x356/0x5c0
    ip_local_deliver+0x184/0x4a0
    ip_rcv+0x4f4/0x5b0
    __netif_receive_skb_one_core+0x153/0x1b0
    process_backlog+0x28d/0x1190
    __napi_poll+0xab/0x520
    net_rx_action+0x3f0/0xca0

The approach taken is to make acquire and release unconditional and
symmetric.

[1]: https://lore.kernel.org/bpf/20260701235552.2B0AA1F00A3F@smtp.kernel.org/

Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
Changes in v2:
- Defer "Use sock_hold() instead of refcount_inc_not_zero() in lookup" to
  bpf-next [John]
- Fix comment style [Sashiko]
- Link to v1: https://patch.msgid.link/20260723-sockmap-lookup-tcp-leak-v1-0-1bbdc58ce514@rbox.co

To: Alexei Starovoitov <ast@kernel.org>
To: Daniel Borkmann <daniel@iogearbox.net>
To: Andrii Nakryiko <andrii@kernel.org>
To: Eduard Zingerman <eddyz87@gmail.com>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: Martin KaFai Lau <martin.lau@linux.dev>
To: Song Liu <song@kernel.org>
To: Yonghong Song <yonghong.song@linux.dev>
To: Jiri Olsa <jolsa@kernel.org>
To: Emil Tsalapatis <emil@etsalapatis.com>
To: John Fastabend <john.fastabend@gmail.com>
To: Stanislav Fomichev <sdf@fomichev.me>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Simon Horman <horms@kernel.org>
To: Kuniyuki Iwashima <kuniyu@google.com>
To: Willem de Bruijn <willemb@google.com>
To: Jakub Sitnicki <jakub@cloudflare.com>
To: Jiayuan Chen <jiayuan.chen@linux.dev>
To: Joe Stringer <joe@wand.net.nz>
Cc: bpf@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Michal Luczaj (2):
      bpf: Extract shared reqsk-to-listener upgrade
      bpf: Unconditionally take socket references in lookup helpers

 net/core/filter.c   | 83 +++++++++++++++++++++++++++--------------------------
 net/core/sock_map.c |  4 +--
 2 files changed, 45 insertions(+), 42 deletions(-)
---
base-commit: e5fd3f514e27db1f05fbd72ba615d74941e23c51
change-id: 20260628-sockmap-lookup-tcp-leak-bdaba3e083c5

Best regards,
--  
Michal Luczaj <mhal@rbox.co>

[PATCH bpf v2 1/2] bpf: Extract shared reqsk-to-listener upgrade

[Michal Luczaj]

__bpf_sk_lookup() and bpf_sk_lookup() duplicate the same sk_to_full_sk()
reqsk-to-listener upgrade. Extract it into a helper.

Leave the currently unreachable WARN_ONCE as a defensive assert.

No functional change.

Signed-off-by: Michal Luczaj <mhal@rbox.co>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
---
 net/core/filter.c | 58 +++++++++++++++++++++++++------------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 11bb0d236822..fede810ef37f 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7079,6 +7079,28 @@ __bpf_skc_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
 	return sk;
 }
 
+static struct sock *
+bpf_sk_lookup_full_sk(struct sock *sk)
+{
+	struct sock *sk2 = sk_to_full_sk(sk);
+
+	/*
+	 * sk_to_full_sk() may return sk->rsk_listener, make sure the original
+	 * sk sock refcnt is decremented to prevent a request_sock leak.
+	 */
+	if (sk2 != sk) {
+		sock_gen_put(sk);
+		/* Ensure there is no need to bump sk2 refcnt. */
+		if (unlikely(sk2 && !sock_flag(sk2, SOCK_RCU_FREE))) {
+			WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
+			return NULL;
+		}
+		sk = sk2;
+	}
+
+	return sk;
+}
+
 static struct sock *
 __bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
 		struct net *caller_net, u32 ifindex, u8 proto, u64 netns_id,
@@ -7088,22 +7110,8 @@ __bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
 					   ifindex, proto, netns_id, flags,
 					   sdif);
 
-	if (sk) {
-		struct sock *sk2 = sk_to_full_sk(sk);
-
-		/* sk_to_full_sk() may return (sk)->rsk_listener, so make sure the original sk
-		 * sock refcnt is decremented to prevent a request_sock leak.
-		 */
-		if (sk2 != sk) {
-			sock_gen_put(sk);
-			/* Ensure there is no need to bump sk2 refcnt */
-			if (unlikely(sk2 && !sock_flag(sk2, SOCK_RCU_FREE))) {
-				WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
-				return NULL;
-			}
-			sk = sk2;
-		}
-	}
+	if (sk)
+		sk = bpf_sk_lookup_full_sk(sk);
 
 	return sk;
 }
@@ -7134,22 +7142,8 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
 	struct sock *sk = bpf_skc_lookup(skb, tuple, len, proto, netns_id,
 					 flags);
 
-	if (sk) {
-		struct sock *sk2 = sk_to_full_sk(sk);
-
-		/* sk_to_full_sk() may return (sk)->rsk_listener, so make sure the original sk
-		 * sock refcnt is decremented to prevent a request_sock leak.
-		 */
-		if (sk2 != sk) {
-			sock_gen_put(sk);
-			/* Ensure there is no need to bump sk2 refcnt */
-			if (unlikely(sk2 && !sock_flag(sk2, SOCK_RCU_FREE))) {
-				WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
-				return NULL;
-			}
-			sk = sk2;
-		}
-	}
+	if (sk)
+		sk = bpf_sk_lookup_full_sk(sk);
 
 	return sk;
 }

-- 
2.55.0

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

[Michal Luczaj]

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/
Signed-off-by: Michal Luczaj <mhal@rbox.co>
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.
---
 net/core/filter.c   | 27 ++++++++++++++++++---------
 net/core/sock_map.c |  4 ++--
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index fede810ef37f..d71e069f669a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7032,6 +7032,14 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
 		WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
 		sk = NULL;
 	}
+
+	/*
+	 * Always take a reference, even if the lookup skipped one;
+	 * bpf_sk_release() always puts one.
+	 */
+	if (sk && !refcounted && !refcount_inc_not_zero(&sk->sk_refcnt))
+		sk = NULL;
+
 	return sk;
 }
 
@@ -7090,11 +7098,16 @@ bpf_sk_lookup_full_sk(struct sock *sk)
 	 */
 	if (sk2 != sk) {
 		sock_gen_put(sk);
-		/* Ensure there is no need to bump sk2 refcnt. */
 		if (unlikely(sk2 && !sock_flag(sk2, SOCK_RCU_FREE))) {
 			WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
 			return NULL;
 		}
+		/*
+		 * sk2 is RCU-free, but take a reference anyway;
+		 * bpf_sk_release() puts.
+		 */
+		if (sk2 && !refcount_inc_not_zero(&sk2->sk_refcnt))
+			sk2 = NULL;
 		sk = sk2;
 	}
 
@@ -7279,7 +7292,7 @@ static const struct bpf_func_proto bpf_tc_sk_lookup_udp_proto = {
 
 BPF_CALL_1(bpf_sk_release, struct sock *, sk)
 {
-	if (sk && sk_is_refcounted(sk))
+	if (sk)
 		sock_gen_put(sk);
 	return 0;
 }
@@ -11571,11 +11584,13 @@ BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
 	bool is_sockarray = map->map_type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY;
 	struct sock_reuseport *reuse;
 	struct sock *selected_sk;
-	int err;
+	int err = 0;
 
 	selected_sk = map->ops->map_lookup_elem(map, key);
 	if (!selected_sk)
 		return -ENOENT;
+	if (!is_sockarray)
+		sock_put(selected_sk);
 
 	reuse = rcu_dereference(selected_sk->sk_reuseport_cb);
 	if (!reuse) {
@@ -11605,13 +11620,7 @@ BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
 	}
 
 	reuse_kern->selected_sk = selected_sk;
-
-	return 0;
 error:
-	/* Lookup in sock_map can return TCP ESTABLISHED sockets. */
-	if (sk_is_refcounted(selected_sk))
-		sock_put(selected_sk);
-
 	return err;
 }
 
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 9efbd8ca7db8..92a006fd3368 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -392,7 +392,7 @@ static void *sock_map_lookup(struct bpf_map *map, void *key)
 	sk = __sock_map_lookup_elem(map, *(u32 *)key);
 	if (!sk)
 		return NULL;
-	if (sk_is_refcounted(sk) && !refcount_inc_not_zero(&sk->sk_refcnt))
+	if (!refcount_inc_not_zero(&sk->sk_refcnt))
 		return NULL;
 	return sk;
 }
@@ -1218,7 +1218,7 @@ static void *sock_hash_lookup(struct bpf_map *map, void *key)
 	sk = __sock_hash_lookup_elem(map, key);
 	if (!sk)
 		return NULL;
-	if (sk_is_refcounted(sk) && !refcount_inc_not_zero(&sk->sk_refcnt))
+	if (!refcount_inc_not_zero(&sk->sk_refcnt))
 		return NULL;
 	return sk;
 }

-- 
2.55.0

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

[sashiko-bot]

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

Pre-existing issues:
- [High] A pre-existing socket reference leak exists when `bpf_sk_assign()` assigns a socket to an skb and `sock_pfree()` acts as the destructor. `bpf_sk_assign()` conditionally increments the reference count if `sk_is_refcounted()` returns true, but `sock_pfree()` independently re-evaluates `sk_is_refcounted()`. If the socket transitions to `SOCK_RCU_FREE` between these two calls (e.g., via `connect(AF_UNSPEC)` followed by `listen()`), `sock_pfree()` returns early without calling `sock_gen_put()`, permanently leaking the reference.
--

--- Patch [2]: [PATCH bpf v2 2/2] bpf: Unconditionally take socket references in lookup helpers ---
commit 7463d9a6a26a249e959688cc8e1b8c2847ee47d0
Author: Michal Luczaj <mhal@rbox.co>

bpf: Unconditionally take socket references in lookup helpers

This commit updates BPF socket lookup helpers to unconditionally acquire
and release socket references. It fixes a leak that occurs if a socket
transitions to SOCK_RCU_FREE between the time the reference is
conditionally taken and released.

Closes: https://lore.kernel.org/bpf/20260701235552.2B0AA1F00A3F@smtp.kernel.org/

> 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.

[Severity: High]
This is a pre-existing issue, but as you noted, a socket reference is
permanently leaked if the socket state changes to unrefcounted between
bpf_sk_assign() and the skb destructor running.

In net/core/filter.c:bpf_sk_assign(), the reference is conditionally acquired
and the destructor is set:

	if (sk_is_refcounted(sk) &&
	    unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
		return -ENOENT;

	skb_orphan(skb);
	skb->sk = sk;
	skb->destructor = sock_pfree;

Then in net/core/sock.c:sock_pfree(), the reference drop is independently
evaluated:

void sock_pfree(struct sk_buff *skb)
{
	struct sock *sk = skb->sk;

	if (!sk_is_refcounted(sk))
		return;

	...
	sock_gen_put(sk);
}

If the socket transitions to SOCK_RCU_FREE via a connect(AF_UNSPEC) followed
by a listen() before the skb is freed, sock_pfree() returns early without
calling sock_gen_put(). 

Could this be addressed by either applying the same unconditional reference
counting approach to bpf_sk_assign(), or by introducing a dedicated
destructor for BPF-assigned sockets that unconditionally drops the reference
without re-evaluating sk_is_refcounted()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-sockmap-lookup-tcp-leak-v2-0-306e025bfe66@rbox.co?part=2

Re: [PATCH bpf v2 1/2] bpf: Extract shared reqsk-to-listener upgrade

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 11bb0d236822..fede810ef37f 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7079,6 +7079,28 @@ __bpf_skc_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
>  	return sk;
>  }
>
> +static struct sock *
> +bpf_sk_lookup_full_sk(struct sock *sk)
> +{

This isn't a bug, but since this helper only converts a reqsk to its
listener rather than performing a lookup, would a name outside the
bpf_sk_lookup_* family - say sk_lookup_full_sk() or
bpf_reqsk_to_listener() - read more clearly next to
bpf_sk_lookup_tcp()/bpf_sk_lookup_udp() and struct bpf_sk_lookup?

The bpf_sk_lookup_ prefix in this file already denotes the BPF helper family
(bpf_sk_lookup_tcp(), bpf_sk_lookup_udp(), bpf_sk_lookup_assign(),
bpf_sk_lookup_enabled) and the UAPI context type struct bpf_sk_lookup /
struct bpf_sk_lookup_kern. Sitting about 20 lines above bpf_sk_lookup()
itself, the name reads like another lookup entry point rather than a
reqsk-to-listener conversion.

> +	struct sock *sk2 = sk_to_full_sk(sk);
> +
> +	/*
> +	 * sk_to_full_sk() may return sk->rsk_listener, make sure the original
> +	 * sk sock refcnt is decremented to prevent a request_sock leak.
> +	 */
> +	if (sk2 != sk) {
> +		sock_gen_put(sk);
> +		/* Ensure there is no need to bump sk2 refcnt. */
> +		if (unlikely(sk2 && !sock_flag(sk2, SOCK_RCU_FREE))) {
> +			WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
> +			return NULL;
> +		}
> +		sk = sk2;
> +	}
> +
> +	return sk;
> +}

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30800053349

Re: [PATCH bpf v2 1/2] bpf: Extract shared reqsk-to-listener upgrade

[Michal Luczaj]

On 8/3/26 11:44, bot+bpf-ci@kernel.org wrote:
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 11bb0d236822..fede810ef37f 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -7079,6 +7079,28 @@ __bpf_skc_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
>>  	return sk;
>>  }
>>
>> +static struct sock *
>> +bpf_sk_lookup_full_sk(struct sock *sk)
>> +{
> 
> This isn't a bug, but since this helper only converts a reqsk to its
> listener rather than performing a lookup, would a name outside the
> bpf_sk_lookup_* family - say sk_lookup_full_sk() or
> bpf_reqsk_to_listener() - read more clearly next to
> bpf_sk_lookup_tcp()/bpf_sk_lookup_udp() and struct bpf_sk_lookup?
> 
> The bpf_sk_lookup_ prefix in this file already denotes the BPF helper family
> (bpf_sk_lookup_tcp(), bpf_sk_lookup_udp(), bpf_sk_lookup_assign(),
> bpf_sk_lookup_enabled) and the UAPI context type struct bpf_sk_lookup /
> struct bpf_sk_lookup_kern. Sitting about 20 lines above bpf_sk_lookup()
> itself, the name reads like another lookup entry point rather than a
> reqsk-to-listener conversion.

I've meant it to be a bpf-lookup helper, not a bpf lookup-helper. So yeah,
I'm happy to change it to something more appropriate. sk_lookup_full_sk() then?

>> +	struct sock *sk2 = sk_to_full_sk(sk);
>> +
>> +	/*
>> +	 * sk_to_full_sk() may return sk->rsk_listener, make sure the original
>> +	 * sk sock refcnt is decremented to prevent a request_sock leak.
>> +	 */
>> +	if (sk2 != sk) {
>> +		sock_gen_put(sk);
>> +		/* Ensure there is no need to bump sk2 refcnt. */
>> +		if (unlikely(sk2 && !sock_flag(sk2, SOCK_RCU_FREE))) {
>> +			WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
>> +			return NULL;
>> +		}
>> +		sk = sk2;
>> +	}
>> +
>> +	return sk;
>> +}
> 
> [ ... ]

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

[Michal Luczaj]

On 8/3/26 11:35, sashiko-bot@kernel.org wrote:
...
>> 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.
> 
> [Severity: High]
> This is a pre-existing issue, but as you noted, a socket reference is
> permanently leaked if the socket state changes to unrefcounted between
> bpf_sk_assign() and the skb destructor running.
> 
> In net/core/filter.c:bpf_sk_assign(), the reference is conditionally acquired
> and the destructor is set:
> 
> 	if (sk_is_refcounted(sk) &&
> 	    unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
> 		return -ENOENT;
> 
> 	skb_orphan(skb);
> 	skb->sk = sk;
> 	skb->destructor = sock_pfree;
> 
> Then in net/core/sock.c:sock_pfree(), the reference drop is independently
> evaluated:
> 
> void sock_pfree(struct sk_buff *skb)
> {
> 	struct sock *sk = skb->sk;
> 
> 	if (!sk_is_refcounted(sk))
> 		return;
> 
> 	...
> 	sock_gen_put(sk);
> }
> 
> If the socket transitions to SOCK_RCU_FREE via a connect(AF_UNSPEC) followed
> by a listen() before the skb is freed, sock_pfree() returns early without
> calling sock_gen_put(). 
> 
> Could this be addressed by either applying the same unconditional reference
> counting approach to bpf_sk_assign(), or by introducing a dedicated
> destructor for BPF-assigned sockets that unconditionally drops the reference
> without re-evaluating sk_is_refcounted()?

I was considering something like this in bpf_sk_assign():

	refcounted = sk_is_refcounted(sk);
	if (refcounted && unlikely(!refcount_inc_not_zero(&sk->sk_refcnt)))
		return -ENOENT;
	skb->destructor = refcounted ? sock_edemux : sock_pfree;

which would unconditionally drop the ref in sock_edemux(), but setting a
non-sock_pfree dtor affects the logic in ip_rcv_core() where we have

	/* Must drop socket now because of tproxy. */
	if (!skb_sk_is_prefetched(skb))
		skb_orphan(skb);

And I'm uncertain about touching net/ipv4/ip_input.c.

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

[Kuniyuki Iwashima]

On Mon, Aug 3, 2026 at 2:01 AM Michal Luczaj <mhal@rbox.co> 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 <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/bpf/20260701235552.2B0AA1F00A3F@smtp.kernel.org/
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> 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.

The same class of issue was reported by listen() + shutdown() + connect().
https://lore.kernel.org/netdev/20260804015349.2353056-1-kuniyu@google.com/

Can you test the diff in the thread ?


> ---
>  net/core/filter.c   | 27 ++++++++++++++++++---------
>  net/core/sock_map.c |  4 ++--
>  2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index fede810ef37f..d71e069f669a 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7032,6 +7032,14 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
>                 WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
>                 sk = NULL;
>         }
> +
> +       /*
> +        * Always take a reference, even if the lookup skipped one;
> +        * bpf_sk_release() always puts one.
> +        */
> +       if (sk && !refcounted && !refcount_inc_not_zero(&sk->sk_refcnt))
> +               sk = NULL;
> +
>         return sk;
>  }
>
> @@ -7090,11 +7098,16 @@ bpf_sk_lookup_full_sk(struct sock *sk)
>          */
>         if (sk2 != sk) {
>                 sock_gen_put(sk);
> -               /* Ensure there is no need to bump sk2 refcnt. */
>                 if (unlikely(sk2 && !sock_flag(sk2, SOCK_RCU_FREE))) {
>                         WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
>                         return NULL;
>                 }
> +               /*
> +                * sk2 is RCU-free, but take a reference anyway;
> +                * bpf_sk_release() puts.
> +                */
> +               if (sk2 && !refcount_inc_not_zero(&sk2->sk_refcnt))
> +                       sk2 = NULL;
>                 sk = sk2;
>         }
>
> @@ -7279,7 +7292,7 @@ static const struct bpf_func_proto bpf_tc_sk_lookup_udp_proto = {
>
>  BPF_CALL_1(bpf_sk_release, struct sock *, sk)
>  {
> -       if (sk && sk_is_refcounted(sk))
> +       if (sk)
>                 sock_gen_put(sk);
>         return 0;
>  }
> @@ -11571,11 +11584,13 @@ BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
>         bool is_sockarray = map->map_type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY;
>         struct sock_reuseport *reuse;
>         struct sock *selected_sk;
> -       int err;
> +       int err = 0;
>
>         selected_sk = map->ops->map_lookup_elem(map, key);
>         if (!selected_sk)
>                 return -ENOENT;
> +       if (!is_sockarray)
> +               sock_put(selected_sk);
>
>         reuse = rcu_dereference(selected_sk->sk_reuseport_cb);
>         if (!reuse) {
> @@ -11605,13 +11620,7 @@ BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
>         }
>
>         reuse_kern->selected_sk = selected_sk;
> -
> -       return 0;
>  error:
> -       /* Lookup in sock_map can return TCP ESTABLISHED sockets. */
> -       if (sk_is_refcounted(selected_sk))
> -               sock_put(selected_sk);
> -
>         return err;
>  }
>
> diff --git a/net/core/sock_map.c b/net/core/sock_map.c
> index 9efbd8ca7db8..92a006fd3368 100644
> --- a/net/core/sock_map.c
> +++ b/net/core/sock_map.c
> @@ -392,7 +392,7 @@ static void *sock_map_lookup(struct bpf_map *map, void *key)
>         sk = __sock_map_lookup_elem(map, *(u32 *)key);
>         if (!sk)
>                 return NULL;
> -       if (sk_is_refcounted(sk) && !refcount_inc_not_zero(&sk->sk_refcnt))
> +       if (!refcount_inc_not_zero(&sk->sk_refcnt))
>                 return NULL;
>         return sk;
>  }
> @@ -1218,7 +1218,7 @@ static void *sock_hash_lookup(struct bpf_map *map, void *key)
>         sk = __sock_hash_lookup_elem(map, key);
>         if (!sk)
>                 return NULL;
> -       if (sk_is_refcounted(sk) && !refcount_inc_not_zero(&sk->sk_refcnt))
> +       if (!refcount_inc_not_zero(&sk->sk_refcnt))
>                 return NULL;
>         return sk;
>  }
>
> --
> 2.55.0
>

Re: [PATCH bpf v2 1/2] bpf: Extract shared reqsk-to-listener upgrade

[Jakub Sitnicki]

On Mon, Aug 03, 2026 at 11:00 AM +02, Michal Luczaj wrote:
> __bpf_sk_lookup() and bpf_sk_lookup() duplicate the same sk_to_full_sk()
> reqsk-to-listener upgrade. Extract it into a helper.
>
> Leave the currently unreachable WARN_ONCE as a defensive assert.
>
> No functional change.
>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---

Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>

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

[Jakub Sitnicki]

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 <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/bpf/20260701235552.2B0AA1F00A3F@smtp.kernel.org/
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> 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?

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

[Kuniyuki Iwashima]

On Tue, Aug 4, 2026 at 3:14 AM Jakub Sitnicki <jakub@cloudflare.com> 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 <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/bpf/20260701235552.2B0AA1F00A3F@smtp.kernel.org/
> > Signed-off-by: Michal Luczaj <mhal@rbox.co>
> > 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.