mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2026-09-22 09:34:56 +02:00
Bluetooth: SCO: hold sk properly in sco_conn_ready
commit4e37f6452dupstream. sk deref in sco_conn_ready must be done either under conn->lock, or holding a refcount, to avoid concurrent close. conn->sk and parent sk is currently accessed without either, and without checking parent->sk_state: [Task 1] [Task 2] sco_sock_release sco_conn_ready sk = conn->sk lock_sock(sk) conn->sk = NULL lock_sock(sk) release_sock(sk) sco_sock_kill(sk) UAF on sk deref and similarly for access to sco_get_sock_listen() return value. Fix possible UAF by holding sk refcount in sco_conn_ready() and making sco_get_sock_listen() increase refcount. Also recheck after lock_sock that the socket is still valid. Adjust conn->sk locking so it's protected also by lock_sock() of the associated socket if any. Fixes:27c24fda62("Bluetooth: switch to lock_sock in SCO") Signed-off-by: Pauli Virtanen <pav@iki.fi> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
44a203afae
commit
d141d9b769
+32
-12
@@ -409,9 +409,13 @@ static struct sock *sco_get_sock_listen(bdaddr_t *src)
|
||||
sk1 = sk;
|
||||
}
|
||||
|
||||
sk = sk ? sk : sk1;
|
||||
if (sk)
|
||||
sock_hold(sk);
|
||||
|
||||
read_unlock(&sco_sk_list.lock);
|
||||
|
||||
return sk ? sk : sk1;
|
||||
return sk;
|
||||
}
|
||||
|
||||
static void sco_sock_destruct(struct sock *sk)
|
||||
@@ -451,11 +455,13 @@ static void sco_sock_kill(struct sock *sk)
|
||||
BT_DBG("sk %p state %d", sk, sk->sk_state);
|
||||
|
||||
/* Sock is dead, so set conn->sk to NULL to avoid possible UAF */
|
||||
lock_sock(sk);
|
||||
if (sco_pi(sk)->conn) {
|
||||
sco_conn_lock(sco_pi(sk)->conn);
|
||||
sco_pi(sk)->conn->sk = NULL;
|
||||
sco_conn_unlock(sco_pi(sk)->conn);
|
||||
}
|
||||
release_sock(sk);
|
||||
|
||||
/* Kill poor orphan */
|
||||
bt_sock_unlink(&sco_sk_list, sk);
|
||||
@@ -1302,17 +1308,28 @@ static int sco_sock_release(struct socket *sock)
|
||||
|
||||
static void sco_conn_ready(struct sco_conn *conn)
|
||||
{
|
||||
struct sock *parent;
|
||||
struct sock *sk = conn->sk;
|
||||
struct sock *parent, *sk;
|
||||
|
||||
sco_conn_lock(conn);
|
||||
sk = sco_sock_hold(conn);
|
||||
sco_conn_unlock(conn);
|
||||
|
||||
BT_DBG("conn %p", conn);
|
||||
|
||||
if (sk) {
|
||||
lock_sock(sk);
|
||||
sco_sock_clear_timer(sk);
|
||||
sk->sk_state = BT_CONNECTED;
|
||||
sk->sk_state_change(sk);
|
||||
|
||||
/* conn->sk may have become NULL if racing with sk close, but
|
||||
* due to held hdev->lock, it can't become different sk.
|
||||
*/
|
||||
if (conn->sk) {
|
||||
sco_sock_clear_timer(sk);
|
||||
sk->sk_state = BT_CONNECTED;
|
||||
sk->sk_state_change(sk);
|
||||
}
|
||||
|
||||
release_sock(sk);
|
||||
sock_put(sk);
|
||||
} else {
|
||||
if (!conn->hcon)
|
||||
return;
|
||||
@@ -1327,13 +1344,15 @@ static void sco_conn_ready(struct sco_conn *conn)
|
||||
|
||||
sco_conn_lock(conn);
|
||||
|
||||
/* hdev->lock guarantees conn->sk == NULL still here */
|
||||
|
||||
if (parent->sk_state != BT_LISTEN)
|
||||
goto release;
|
||||
|
||||
sk = sco_sock_alloc(sock_net(parent), NULL,
|
||||
BTPROTO_SCO, GFP_ATOMIC, 0);
|
||||
if (!sk) {
|
||||
sco_conn_unlock(conn);
|
||||
release_sock(parent);
|
||||
return;
|
||||
}
|
||||
if (!sk)
|
||||
goto release;
|
||||
|
||||
sco_sock_init(sk, parent);
|
||||
|
||||
@@ -1351,9 +1370,10 @@ static void sco_conn_ready(struct sco_conn *conn)
|
||||
/* Wake up parent */
|
||||
parent->sk_data_ready(parent);
|
||||
|
||||
release:
|
||||
sco_conn_unlock(conn);
|
||||
|
||||
release_sock(parent);
|
||||
sock_put(parent);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user