* Re: [PATCH bpf 1/3] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup [not found] ` <20260723-sockmap-lookup-tcp-leak-v1-1-1bbdc58ce514@rbox.co> @ 2026-07-25 0:00 ` John Fastabend 2026-07-25 18:07 ` Michal Luczaj 0 siblings, 1 reply; 2+ messages in thread From: John Fastabend @ 2026-07-25 0:00 UTC (permalink / raw) To: Michal Luczaj Cc: Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn, Jakub Sitnicki, Jiayuan Chen, David S. Miller, Jakub Kicinski, Simon Horman, Daniel Borkmann, Stanislav Fomichev, Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, Joe Stringer, netdev, bpf, linux-kernel 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 > ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH bpf 1/3] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup 2026-07-25 0:00 ` [PATCH bpf 1/3] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup John Fastabend @ 2026-07-25 18:07 ` Michal Luczaj 0 siblings, 0 replies; 2+ messages in thread From: Michal Luczaj @ 2026-07-25 18:07 UTC (permalink / raw) To: John Fastabend Cc: Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn, Jakub Sitnicki, Jiayuan Chen, David S. Miller, Jakub Kicinski, Simon Horman, Daniel Borkmann, Stanislav Fomichev, Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, Joe Stringer, netdev, bpf, linux-kernel On 7/25/26 02:00, John Fastabend wrote: > 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? Right. Would you rather have it squashed with patch 3 or should I save it for bpf-next? >> 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; >> } ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-25 18:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260723-sockmap-lookup-tcp-leak-v1-0-1bbdc58ce514@rbox.co>
[not found] ` <20260723-sockmap-lookup-tcp-leak-v1-1-1bbdc58ce514@rbox.co>
2026-07-25 0:00 ` [PATCH bpf 1/3] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup John Fastabend
2026-07-25 18:07 ` Michal Luczaj
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox