mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-30 06:29:06 -04:00
Bluetooth: ISO: avoid deadlocks in iso_sock_timeout
iso_sock_timeout() takes lock_sock, so sync disabling the timer while
holding that lock may deadlock.
iso_sock_timeout() may also run concurrently with iso_conn_del(), which
leads to UAF
[Task 1] [Task hdev->workqueue]
iso_sock_timeout iso_conn_del
iso_conn_hold_unless_zero iso_chan_del
`------------> iso_conn_put
caller frees hcon
iso_conn_put
iso_conn_free
conn->hcon->iso_data = NULL; /* UAF */
Fix the deadlock by removing the disable from the lock_sock sections.
Move the timer from iso_conn to iso_pinfo to decouple it from iso_conn
which may need to be freed in lock_sock section. Convert some of the
clear_timer to disable_timer.
Fixes: dc26097bdb ("Bluetooth: ISO: Use kref to track lifetime of iso_conn")
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
ce57442a37
commit
200fa1629c
@@ -37,8 +37,6 @@ struct iso_conn {
|
||||
spinlock_t lock;
|
||||
struct sock *sk;
|
||||
|
||||
struct delayed_work timeout_work;
|
||||
|
||||
struct sk_buff *rx_skb;
|
||||
__u32 rx_len;
|
||||
__u16 tx_sn;
|
||||
@@ -81,6 +79,7 @@ struct iso_pinfo {
|
||||
__u8 base_len;
|
||||
__u8 base[BASE_MAX_LENGTH];
|
||||
struct iso_conn *conn;
|
||||
struct delayed_work timeout_work;
|
||||
};
|
||||
|
||||
static struct bt_iso_qos default_qos;
|
||||
@@ -118,9 +117,6 @@ static void iso_conn_free(struct kref *ref)
|
||||
hci_conn_drop(conn->hcon);
|
||||
}
|
||||
|
||||
/* Ensure no more work items will run since hci_conn has been dropped */
|
||||
disable_delayed_work_sync(&conn->timeout_work);
|
||||
|
||||
kfree_skb(conn->rx_skb);
|
||||
|
||||
kfree(conn);
|
||||
@@ -161,48 +157,45 @@ static struct sock *iso_sock_hold(struct iso_conn *conn)
|
||||
|
||||
static void iso_sock_timeout(struct work_struct *work)
|
||||
{
|
||||
struct iso_conn *conn = container_of(work, struct iso_conn,
|
||||
timeout_work.work);
|
||||
struct sock *sk;
|
||||
|
||||
conn = iso_conn_hold_unless_zero(conn);
|
||||
if (!conn)
|
||||
return;
|
||||
|
||||
iso_conn_lock(conn);
|
||||
sk = iso_sock_hold(conn);
|
||||
iso_conn_unlock(conn);
|
||||
iso_conn_put(conn);
|
||||
|
||||
if (!sk)
|
||||
return;
|
||||
struct iso_pinfo *pi = container_of(work, struct iso_pinfo,
|
||||
timeout_work.work);
|
||||
struct sock *sk = &pi->bt.sk;
|
||||
|
||||
BT_DBG("sock %p state %d", sk, sk->sk_state);
|
||||
|
||||
lock_sock(sk);
|
||||
sk->sk_err = ETIMEDOUT;
|
||||
sk->sk_state_change(sk);
|
||||
if (!sock_flag(sk, SOCK_ZAPPED)) {
|
||||
sk->sk_err = ETIMEDOUT;
|
||||
sk->sk_state_change(sk);
|
||||
}
|
||||
release_sock(sk);
|
||||
sock_put(sk);
|
||||
}
|
||||
|
||||
static void iso_sock_set_timer(struct sock *sk, long timeout)
|
||||
{
|
||||
lockdep_assert(lockdep_sock_is_held(sk));
|
||||
|
||||
cancel_delayed_work(&iso_pi(sk)->timeout_work);
|
||||
|
||||
if (!iso_pi(sk)->conn)
|
||||
return;
|
||||
|
||||
BT_DBG("sock %p state %d timeout %ld", sk, sk->sk_state, timeout);
|
||||
cancel_delayed_work(&iso_pi(sk)->conn->timeout_work);
|
||||
schedule_delayed_work(&iso_pi(sk)->conn->timeout_work, timeout);
|
||||
schedule_delayed_work(&iso_pi(sk)->timeout_work, timeout);
|
||||
}
|
||||
|
||||
static void iso_sock_clear_timer(struct sock *sk)
|
||||
{
|
||||
if (!iso_pi(sk)->conn)
|
||||
return;
|
||||
BT_DBG("sock %p state %d", sk, sk->sk_state);
|
||||
cancel_delayed_work(&iso_pi(sk)->timeout_work);
|
||||
}
|
||||
|
||||
static void iso_sock_disable_timer(struct sock *sk)
|
||||
{
|
||||
lockdep_assert(!lockdep_sock_is_held(sk));
|
||||
|
||||
BT_DBG("sock %p state %d", sk, sk->sk_state);
|
||||
cancel_delayed_work(&iso_pi(sk)->conn->timeout_work);
|
||||
disable_delayed_work_sync(&iso_pi(sk)->timeout_work);
|
||||
}
|
||||
|
||||
/* ---- ISO connections ---- */
|
||||
@@ -227,7 +220,6 @@ static struct iso_conn *iso_conn_add(struct hci_conn *hcon)
|
||||
|
||||
kref_init(&conn->ref);
|
||||
spin_lock_init(&conn->lock);
|
||||
INIT_DELAYED_WORK(&conn->timeout_work, iso_sock_timeout);
|
||||
|
||||
hcon->iso_data = conn;
|
||||
conn->hcon = hcon;
|
||||
@@ -292,8 +284,9 @@ static void iso_conn_del(struct hci_conn *hcon, int err)
|
||||
return;
|
||||
}
|
||||
|
||||
iso_sock_disable_timer(sk);
|
||||
|
||||
lock_sock(sk);
|
||||
iso_sock_clear_timer(sk);
|
||||
iso_chan_del(sk, err);
|
||||
release_sock(sk);
|
||||
iso_sock_kill(sk);
|
||||
@@ -800,6 +793,8 @@ static void iso_sock_cleanup_listen(struct sock *parent)
|
||||
*/
|
||||
static void iso_sock_kill(struct sock *sk)
|
||||
{
|
||||
iso_sock_disable_timer(sk);
|
||||
|
||||
lock_sock(sk);
|
||||
|
||||
if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket ||
|
||||
@@ -895,8 +890,9 @@ static void __iso_sock_close(struct sock *sk)
|
||||
/* Must be called on unlocked socket. */
|
||||
static void iso_sock_close(struct sock *sk)
|
||||
{
|
||||
iso_sock_disable_timer(sk);
|
||||
|
||||
lock_sock(sk);
|
||||
iso_sock_clear_timer(sk);
|
||||
__iso_sock_close(sk);
|
||||
release_sock(sk);
|
||||
}
|
||||
@@ -965,6 +961,8 @@ static struct sock *iso_sock_alloc(struct net *net, struct socket *sock,
|
||||
iso_pi(sk)->qos = default_qos;
|
||||
iso_pi(sk)->sync_handle = -1;
|
||||
|
||||
INIT_DELAYED_WORK(&iso_pi(sk)->timeout_work, iso_sock_timeout);
|
||||
|
||||
bt_sock_link(&iso_sk_list, sk);
|
||||
return sk;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user