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

[PATCH bpf 0/3] 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.

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

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

Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
Michal Luczaj (3):
      bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup
      bpf: Extract shared reqsk-to-listener upgrade
      bpf: Unconditionally take socket references in lookup helpers

 net/core/filter.c   | 80 ++++++++++++++++++++++++++---------------------------
 net/core/sock_map.c | 12 +++-----
 2 files changed, 44 insertions(+), 48 deletions(-)
---
base-commit: 94515f3a7d4256a5062176b7d6ed0471938cd51a
change-id: 20260628-sockmap-lookup-tcp-leak-bdaba3e083c5

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

[PATCH bpf 1/3] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup

[Michal Luczaj]

psock's hold on the looked up socket isn't dropped until sk_psock_drop() ->
queue_rcu_work() -> sk_psock_destroy() runs, which happens only after the
entry is unlinked and an RCU grace period elapses. Since the lookup runs
under RCU, a non-NULL result guarantees sk_refcnt >= 1:
refcount_inc_not_zero() can never fail here. Use sock_hold() instead.

Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 net/core/sock_map.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 9efbd8ca7db8..ca49bc7f8687 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -392,8 +392,8 @@ 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))
-		return NULL;
+	if (sk_is_refcounted(sk))
+		sock_hold(sk);
 	return sk;
 }
 
@@ -1218,8 +1218,8 @@ 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))
-		return NULL;
+	if (sk_is_refcounted(sk))
+		sock_hold(sk);
 	return sk;
 }
 

-- 
2.55.0

[PATCH bpf 2/3] 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>
---
 net/core/filter.c | 57 ++++++++++++++++++++++++-------------------------------
 1 file changed, 25 insertions(+), 32 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index b446aa8be5c3..403aba3ce891 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7074,6 +7074,27 @@ __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,
@@ -7083,22 +7104,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;
 }
@@ -7129,22 +7136,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 3/3] 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>
---
 net/core/filter.c   | 25 ++++++++++++++++---------
 net/core/sock_map.c |  8 ++------
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 403aba3ce891..4bc0b59668fa 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7027,6 +7027,13 @@ 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;
 }
 
@@ -7084,11 +7091,15 @@ 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;
 	}
 
@@ -7273,7 +7284,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;
 }
@@ -11565,11 +11576,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) {
@@ -11599,13 +11612,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 ca49bc7f8687..ae18dc4d60f9 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -390,9 +390,7 @@ static void *sock_map_lookup(struct bpf_map *map, void *key)
 	struct sock *sk;
 
 	sk = __sock_map_lookup_elem(map, *(u32 *)key);
-	if (!sk)
-		return NULL;
-	if (sk_is_refcounted(sk))
+	if (sk)
 		sock_hold(sk);
 	return sk;
 }
@@ -1216,9 +1214,7 @@ static void *sock_hash_lookup(struct bpf_map *map, void *key)
 	struct sock *sk;
 
 	sk = __sock_hash_lookup_elem(map, key);
-	if (!sk)
-		return NULL;
-	if (sk_is_refcounted(sk))
+	if (sk)
 		sock_hold(sk);
 	return sk;
 }

-- 
2.55.0

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

[bot+bpf-ci]

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

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 403aba3ce891..4bc0b59668fa 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7027,6 +7027,13 @@ 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;
>  }

The BPF subsystem guide specifies that multi-line comments under
net/core/filter.c should have the opening '/*' on its own line. Could these
comments follow that format?

[ ... ]

> @@ -7084,11 +7091,15 @@ 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;
>  	}

The same multi-line comment style appears here as well.

[ ... ]

---
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/30016591832

Re: [PATCH bpf 1/3] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup

[John Fastabend]

On Thu, Jul 23, 2026 at 01:33:27PM +0200, Michal Luczaj wrote:
>psock's hold on the looked up socket isn't dropped until sk_psock_drop() ->
>queue_rcu_work() -> sk_psock_destroy() runs, which happens only after the
>entry is unlinked and an RCU grace period elapses. Since the lookup runs
>under RCU, a non-NULL result guarantees sk_refcnt >= 1:
>refcount_inc_not_zero() can never fail here. Use sock_hold() instead.
>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---

Should bpf-next right? this is an optimization not a fix?

> net/core/sock_map.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/net/core/sock_map.c b/net/core/sock_map.c
>index 9efbd8ca7db8..ca49bc7f8687 100644
>--- a/net/core/sock_map.c
>+++ b/net/core/sock_map.c
>@@ -392,8 +392,8 @@ 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))
>-		return NULL;
>+	if (sk_is_refcounted(sk))
>+		sock_hold(sk);
> 	return sk;
> }
>
>@@ -1218,8 +1218,8 @@ 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))
>-		return NULL;
>+	if (sk_is_refcounted(sk))
>+		sock_hold(sk);
> 	return sk;
> }
>
>
>-- 
>2.55.0
>