From: Jakub Sitnicki <ja...@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/00000000000073b14905ef2e7...@google.com/

Suggested-by: Eric Dumazet <eduma...@google.com>
Signed-off-by: Jakub Sitnicki <ja...@cloudflare.com>
Acked-by: John Fastabend <john.fastab...@gmail.com>
Link: 
https://lore.kernel.org/r/20230113-sockmap-fix-v2-1-1e0ee7ac2...@cloudflare.com
Signed-off-by: Alexei Starovoitov <a...@kernel.org>
Signed-off-by: Sasha Levin <sas...@kernel.org>

This is a fix for CVE-2023-52735

Signed-off-by: He Zhe <zhe...@windriver.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 d1d0ee2dbfaa..985254fb3051 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -1555,15 +1555,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)
@@ -1575,16 +1576,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);
 
@@ -1599,13 +1601,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.25.1

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#14252): 
https://lists.yoctoproject.org/g/linux-yocto/message/14252
Mute This Topic: https://lists.yoctoproject.org/mt/107892400/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to