[PATCH v5.10.y] bpf, sockmap: Don't let sock_map_{close,destroy,unhash} call itself

HarinadhD posted 1 patch 1 month, 4 weeks ago
There is a newer version of this series
net/core/sock_map.c | 53 +++++++++++++++++++++++++--------------------
1 file changed, 30 insertions(+), 23 deletions(-)
[PATCH v5.10.y] bpf, sockmap: Don't let sock_map_{close,destroy,unhash} call itself
Posted by HarinadhD 1 month, 4 weeks ago
From: Jakub Sitnicki <jakub@cloudflare.com>

[ Upstream commit 5b4a79ba65a1ab479903fff2e604865d229b70a9 ]

sock_map proto callbacks should never call themselves by design. Protect
against bugs like [1] and break out of the recursive loop to avoid a stack
overflow in favor of a resource leak.

[1] https://lore.kernel.org/all/00000000000073b14905ef2e7401@google.com/

Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Link: https://lore.kernel.org/r/20230113-sockmap-fix-v2-1-1e0ee7ac2f90@cloudflare.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Harinadh: Modified to apply on v5.10.y ]
Signed-off-by: HarinadhD <Harinadh.Dommaraju@broadcom.com>
---
 net/core/sock_map.c | 53 +++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 3a9e0046a780..438bbef5ff75 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -1558,15 +1558,16 @@ void sock_map_unhash(struct sock *sk)
 	psock = sk_psock(sk);
 	if (unlikely(!psock)) {
 		rcu_read_unlock();
-		if (sk->sk_prot->unhash)
-			sk->sk_prot->unhash(sk);
-		return;
+		saved_unhash = READ_ONCE(sk->sk_prot)->unhash;
+	} else {
+		saved_unhash = psock->saved_unhash;
+		sock_map_remove_links(sk, psock);
+		rcu_read_unlock();
 	}
-
-	saved_unhash = psock->saved_unhash;
-	sock_map_remove_links(sk, psock);
-	rcu_read_unlock();
-	saved_unhash(sk);
+	if (WARN_ON_ONCE(saved_unhash == sock_map_unhash))
+		return;
+	if (saved_unhash)
+		saved_unhash(sk);
 }
 
 void sock_map_destroy(struct sock *sk)
@@ -1578,16 +1579,17 @@ void sock_map_destroy(struct sock *sk)
 	psock = sk_psock_get(sk);
 	if (unlikely(!psock)) {
 		rcu_read_unlock();
-		if (sk->sk_prot->destroy)
-			sk->sk_prot->destroy(sk);
-		return;
+		saved_destroy = READ_ONCE(sk->sk_prot)->destroy;
+	} else {
+		saved_destroy = psock->saved_destroy;
+		sock_map_remove_links(sk, psock);
+		rcu_read_unlock();
+		sk_psock_put(sk, psock);
 	}
-
-	saved_destroy = psock->saved_destroy;
-	sock_map_remove_links(sk, psock);
-	rcu_read_unlock();
-	sk_psock_put(sk, psock);
-	saved_destroy(sk);
+	if (WARN_ON_ONCE(saved_destroy == sock_map_destroy))
+		return;
+	if (saved_destroy)
+		saved_destroy(sk);
 }
 EXPORT_SYMBOL_GPL(sock_map_destroy);
 
@@ -1602,13 +1604,18 @@ void sock_map_close(struct sock *sk, long timeout)
 	if (unlikely(!psock)) {
 		rcu_read_unlock();
 		release_sock(sk);
-		return sk->sk_prot->close(sk, timeout);
+		saved_close = READ_ONCE(sk->sk_prot)->close;
+	} else {
+		saved_close = psock->saved_close;
+		sock_map_remove_links(sk, psock);
+		rcu_read_unlock();
+		release_sock(sk);
 	}
-
-	saved_close = psock->saved_close;
-	sock_map_remove_links(sk, psock);
-	rcu_read_unlock();
-	release_sock(sk);
+	/* Make sure we do not recurse. This is a bug.
+	 * Leak the socket instead of crashing on a stack overflow.
+	 */
+	if (WARN_ON_ONCE(saved_close == sock_map_close))
+		return;
 	saved_close(sk, timeout);
 }
 
-- 
2.43.7
Re: [PATCH v5.10.y] bpf, sockmap: Don't let sock_map_{close,destroy,unhash} call itself
Posted by Greg KH 1 month ago
On Fri, Dec 12, 2025 at 03:54:58AM +0000, HarinadhD wrote:
> From: Jakub Sitnicki <jakub@cloudflare.com>
> 
> [ Upstream commit 5b4a79ba65a1ab479903fff2e604865d229b70a9 ]
> 
> sock_map proto callbacks should never call themselves by design. Protect
> against bugs like [1] and break out of the recursive loop to avoid a stack
> overflow in favor of a resource leak.
> 
> [1] https://lore.kernel.org/all/00000000000073b14905ef2e7401@google.com/
> 
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
> Acked-by: John Fastabend <john.fastabend@gmail.com>
> Link: https://lore.kernel.org/r/20230113-sockmap-fix-v2-1-1e0ee7ac2f90@cloudflare.com
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> [ Harinadh: Modified to apply on v5.10.y ]
> Signed-off-by: HarinadhD <Harinadh.Dommaraju@broadcom.com>

Please use your name for your signed-off-by.

thanks,

greg k-h
Re: [PATCH v5.10.y] bpf, sockmap: Don't let sock_map_{close,destroy,unhash} call itself
Posted by Harinadh Dommaraju 1 month ago
On Thu, Jan 8, 2026 at 9:24 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Dec 12, 2025 at 03:54:58AM +0000, HarinadhD wrote:
> > From: Jakub Sitnicki <jakub@cloudflare.com>
> >
> > [ Upstream commit 5b4a79ba65a1ab479903fff2e604865d229b70a9 ]
> >
> > sock_map proto callbacks should never call themselves by design. Protect
> > against bugs like [1] and break out of the recursive loop to avoid a stack
> > overflow in favor of a resource leak.
> >
> > [1] https://lore.kernel.org/all/00000000000073b14905ef2e7401@google.com/
> >
> > Suggested-by: Eric Dumazet <edumazet@google.com>
> > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
> > Acked-by: John Fastabend <john.fastabend@gmail.com>
> > Link: https://lore.kernel.org/r/20230113-sockmap-fix-v2-1-1e0ee7ac2f90@cloudflare.com
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > [ Harinadh: Modified to apply on v5.10.y ]
> > Signed-off-by: HarinadhD <Harinadh.Dommaraju@broadcom.com>
>
> Please use your name for your signed-off-by.
>

Thanks Greg.
I have sent v2.

- Harinadh