mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-31 12:52:29 -04:00
Bluetooth: ISO: hold sk properly in iso_conn_ready
sk deref in iso_conn_ready must be done either under conn->lock, or
holding a refcount, to avoid concurrent close. conn->sk is currently
accessed without either:
[Task 1] [Task 2]
iso_sock_release
iso_conn_ready
sk = conn->sk
lock_sock(sk)
conn->sk = NULL
lock_sock(sk)
release_sock(sk)
iso_sock_kill(sk)
UAF on sk deref
Fix possible UAF by holding sk refcount in iso_conn_ready(). Also
recheck after lock_sock that the socket is still valid. Adjust locking
so conn->sk is cleared only under lock_sock.
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>
This commit is contained in:
committed by
Luiz Augusto von Dentz
parent
4e20192d46
commit
0d255e63fc
@@ -805,11 +805,13 @@ static void iso_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 (iso_pi(sk)->conn) {
|
||||
iso_conn_lock(iso_pi(sk)->conn);
|
||||
iso_pi(sk)->conn->sk = NULL;
|
||||
iso_conn_unlock(iso_pi(sk)->conn);
|
||||
}
|
||||
release_sock(sk);
|
||||
|
||||
/* Kill poor orphan */
|
||||
bt_sock_unlink(&iso_sk_list, sk);
|
||||
@@ -2047,23 +2049,17 @@ static void iso_sock_ready(struct sock *sk)
|
||||
{
|
||||
BT_DBG("sk %p", sk);
|
||||
|
||||
if (!sk)
|
||||
return;
|
||||
|
||||
lock_sock(sk);
|
||||
lockdep_assert(lockdep_sock_is_held(sk));
|
||||
|
||||
switch (sk->sk_state) {
|
||||
case BT_DISCONN:
|
||||
case BT_CLOSED:
|
||||
release_sock(sk);
|
||||
return;
|
||||
}
|
||||
|
||||
iso_sock_clear_timer(sk);
|
||||
sk->sk_state = BT_CONNECTED;
|
||||
sk->sk_state_change(sk);
|
||||
|
||||
release_sock(sk);
|
||||
}
|
||||
|
||||
static bool iso_match_big(struct sock *sk, void *data)
|
||||
@@ -2093,7 +2089,7 @@ static bool iso_match_dst(struct sock *sk, void *data)
|
||||
static void iso_conn_ready(struct iso_conn *conn)
|
||||
{
|
||||
struct sock *parent = NULL;
|
||||
struct sock *sk = conn->sk;
|
||||
struct sock *sk;
|
||||
struct hci_ev_le_big_sync_established *ev = NULL;
|
||||
struct hci_ev_le_pa_sync_established *ev2 = NULL;
|
||||
struct hci_ev_le_per_adv_report *ev3 = NULL;
|
||||
@@ -2102,7 +2098,22 @@ static void iso_conn_ready(struct iso_conn *conn)
|
||||
|
||||
BT_DBG("conn %p", conn);
|
||||
|
||||
iso_conn_lock(conn);
|
||||
sk = iso_sock_hold(conn);
|
||||
iso_conn_unlock(conn);
|
||||
|
||||
if (sk) {
|
||||
lock_sock(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) {
|
||||
release_sock(sk);
|
||||
sock_put(sk);
|
||||
return;
|
||||
}
|
||||
|
||||
/* Attempt to update source address in case of BIS Sender if
|
||||
* the advertisement is using a random address.
|
||||
*/
|
||||
@@ -2115,14 +2126,15 @@ static void iso_conn_ready(struct iso_conn *conn)
|
||||
adv = hci_find_adv_instance(bis->hdev,
|
||||
bis->iso_qos.bcast.bis);
|
||||
if (adv && bacmp(&adv->random_addr, BDADDR_ANY)) {
|
||||
lock_sock(sk);
|
||||
iso_pi(sk)->src_type = BDADDR_LE_RANDOM;
|
||||
bacpy(&iso_pi(sk)->src, &adv->random_addr);
|
||||
release_sock(sk);
|
||||
}
|
||||
}
|
||||
|
||||
iso_sock_ready(conn->sk);
|
||||
iso_sock_ready(sk);
|
||||
|
||||
release_sock(sk);
|
||||
sock_put(sk);
|
||||
} else {
|
||||
hcon = conn->hcon;
|
||||
if (!hcon)
|
||||
|
||||
Reference in New Issue
Block a user