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);
}