[PATCH v2] Bluetooth: SCO: Fix use-after-free on listening socket in sco_conn_ready()

Sanghyun Park posted 1 patch 9 hours ago
net/bluetooth/sco.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH v2] Bluetooth: SCO: Fix use-after-free on listening socket in sco_conn_ready()
Posted by Sanghyun Park 9 hours ago
sco_conn_ready() calls sco_get_sock_listen() which returns a raw
pointer to a listening socket without taking a reference. After
releasing sco_sk_list.lock, a concurrent close() of the listening socket
can free it between the list lookup return and lock_sock(parent),
resulting in a use-after-free.

Fix by taking a reference with sock_hold() inside sco_get_sock_listen()
while the list lock is still held, and dropping it with sock_put() in
sco_conn_ready() after release_sock().

Race:

  CPU0 (HCI event workqueue)         CPU1 (userspace)
  ============================       ==========================
  sco_conn_ready():
    parent = sco_get_sock_listen()
      // returns parent without a reference
                                     close(listen_fd):
                                       sco_sock_release()
                                         sco_sock_kill()
                                           sock_put(parent) -> frees parent
    lock_sock(parent)
    // UAF: parent may already be freed

Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---
Changes in v2:
 - Move sock_hold() from sco_conn_ready() into sco_get_sock_listen() so
   the reference is taken while sco_sk_list.lock is still held.
 - Drop the verbose reproduction/KASAN text, personal note, and unverified
   Fixes tag from the commit message.
 - Fix whitespace damage.

 net/bluetooth/sco.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 6383db54657..20518bbc93a 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -410,6 +410,11 @@ static struct sock *sco_get_sock_listen(bdaddr_t *src)
 			sk1 = sk;
 	}
 
+	if (sk)
+		sock_hold(sk);
+	else if (sk1)
+		sock_hold(sk1);
+
 	read_unlock(&sco_sk_list.lock);
 
 	return sk ? sk : sk1;
@@ -1336,6 +1341,7 @@ static void sco_conn_ready(struct sco_conn *conn)
 				    BTPROTO_SCO, GFP_ATOMIC, 0);
 		if (!sk) {
 			release_sock(parent);
+			sock_put(parent);
 			sco_conn_unlock(conn);
 			return;
 		}
@@ -1357,6 +1363,7 @@ static void sco_conn_ready(struct sco_conn *conn)
 		parent->sk_data_ready(parent);
 
 		release_sock(parent);
+		sock_put(parent);
 
 		sco_conn_unlock(conn);
 	}