BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] bpf: Cleanup & optimization
@ 2026-08-13 12:41 Michal Luczaj
  2026-08-13 12:41 ` [PATCH bpf-next 1/2] bpf: Extract shared reqsk-to-listener upgrade Michal Luczaj
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Michal Luczaj @ 2026-08-13 12:41 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, John Fastabend, Stanislav Fomichev,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jakub Sitnicki, Jiayuan Chen, Kuniyuki Iwashima,
	Willem de Bruijn
  Cc: bpf, netdev, linux-kernel, Michal Luczaj

Non-functional cleanup and a refcnt bumping simplification.

Series extracted from a fix attempt[1] currently under discussion.

[1]: https://lore.kernel.org/bpf/20260803-sockmap-lookup-tcp-leak-v2-0-306e025bfe66@rbox.co/

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

 net/core/filter.c   | 58 ++++++++++++++++++++++++-----------------------------
 net/core/sock_map.c |  8 ++++----
 2 files changed, 30 insertions(+), 36 deletions(-)
---
base-commit: 6f033615ef8fb2374daa7e50a8ff68616bc850d2
change-id: 20260803-sockmap-lookup-get-ref-3f76e4c1ffe2

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 1/2] bpf: Extract shared reqsk-to-listener upgrade
  2026-08-13 12:41 [PATCH bpf-next 0/2] bpf: Cleanup & optimization Michal Luczaj
@ 2026-08-13 12:41 ` Michal Luczaj
  2026-08-13 12:42 ` [PATCH bpf-next 2/2] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup Michal Luczaj
  2026-08-14 10:45 ` [PATCH bpf-next 0/2] bpf: Cleanup & optimization Jakub Sitnicki
  2 siblings, 0 replies; 5+ messages in thread
From: Michal Luczaj @ 2026-08-13 12:41 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, John Fastabend, Stanislav Fomichev,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jakub Sitnicki, Jiayuan Chen, Kuniyuki Iwashima,
	Willem de Bruijn
  Cc: bpf, netdev, linux-kernel, 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.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 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 3423734124a5..031fb2aad792 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7167,6 +7167,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,
@@ -7176,22 +7198,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;
 }
@@ -7222,22 +7230,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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 2/2] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup
  2026-08-13 12:41 [PATCH bpf-next 0/2] bpf: Cleanup & optimization Michal Luczaj
  2026-08-13 12:41 ` [PATCH bpf-next 1/2] bpf: Extract shared reqsk-to-listener upgrade Michal Luczaj
@ 2026-08-13 12:42 ` Michal Luczaj
  2026-08-14 10:42   ` Jakub Sitnicki
  2026-08-14 10:45 ` [PATCH bpf-next 0/2] bpf: Cleanup & optimization Jakub Sitnicki
  2 siblings, 1 reply; 5+ messages in thread
From: Michal Luczaj @ 2026-08-13 12:42 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, John Fastabend, Stanislav Fomichev,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jakub Sitnicki, Jiayuan Chen, Kuniyuki Iwashima,
	Willem de Bruijn
  Cc: bpf, netdev, linux-kernel, 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.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup
  2026-08-13 12:42 ` [PATCH bpf-next 2/2] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup Michal Luczaj
@ 2026-08-14 10:42   ` Jakub Sitnicki
  0 siblings, 0 replies; 5+ messages in thread
From: Jakub Sitnicki @ 2026-08-14 10:42 UTC (permalink / raw)
  To: Michal Luczaj
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, John Fastabend, Stanislav Fomichev,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jiayuan Chen, Kuniyuki Iwashima, Willem de Bruijn,
	bpf, netdev, linux-kernel

On Thu, Aug 13, 2026 at 02:42 PM +02, 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.
>
> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> ---

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 0/2] bpf: Cleanup & optimization
  2026-08-13 12:41 [PATCH bpf-next 0/2] bpf: Cleanup & optimization Michal Luczaj
  2026-08-13 12:41 ` [PATCH bpf-next 1/2] bpf: Extract shared reqsk-to-listener upgrade Michal Luczaj
  2026-08-13 12:42 ` [PATCH bpf-next 2/2] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup Michal Luczaj
@ 2026-08-14 10:45 ` Jakub Sitnicki
  2 siblings, 0 replies; 5+ messages in thread
From: Jakub Sitnicki @ 2026-08-14 10:45 UTC (permalink / raw)
  To: Michal Luczaj
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, John Fastabend, Stanislav Fomichev,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jiayuan Chen, Kuniyuki Iwashima, Willem de Bruijn,
	bpf, netdev, linux-kernel

On Thu, Aug 13, 2026 at 02:41 PM +02, Michal Luczaj wrote:
> Non-functional cleanup and a refcnt bumping simplification.
>
> Series extracted from a fix attempt[1] currently under discussion.
>
> [1]: https://lore.kernel.org/bpf/20260803-sockmap-lookup-tcp-leak-v2-0-306e025bfe66@rbox.co/

Discussion seems to have stalled. Or it's just vacation season.
Anyway, I'm fine with fixing the refcounting your way.
Merely wanted to offer an alternative idea for consideration.
Thanks for working on this.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-14 10:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 12:41 [PATCH bpf-next 0/2] bpf: Cleanup & optimization Michal Luczaj
2026-08-13 12:41 ` [PATCH bpf-next 1/2] bpf: Extract shared reqsk-to-listener upgrade Michal Luczaj
2026-08-13 12:42 ` [PATCH bpf-next 2/2] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup Michal Luczaj
2026-08-14 10:42   ` Jakub Sitnicki
2026-08-14 10:45 ` [PATCH bpf-next 0/2] bpf: Cleanup & optimization Jakub Sitnicki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox