From: Pauli Virtanen Date: Fri, 24 Jul 2026 20:20:31 +0000 (+0300) Subject: Bluetooth: ISO: avoid deadlocks in iso_sock_timeout X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=200fa1629c57a3ca2b03d3ca63fd3a9bfd910c43;p=thirdparty%2Flinux.git 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: dc26097bdb86 ("Bluetooth: ISO: Use kref to track lifetime of iso_conn") Signed-off-by: Pauli Virtanen Signed-off-by: Luiz Augusto von Dentz --- diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c index 5f0f45a573a7..0cc08416fde5 100644 --- a/net/bluetooth/iso.c +++ b/net/bluetooth/iso.c @@ -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; }