Skip to content

Commit e36bea6

Browse files
Vasyl VavrychukVudentz
authored andcommitted
Bluetooth: core: Fix deadlock on hci_power_on_sync.
`cancel_work_sync(&hdev->power_on)` was moved to hci_dev_close_sync in commit [1] to ensure that power_on work is canceled after HCI interface down. But, in certain cases power_on work function may call hci_dev_close_sync itself: hci_power_on -> hci_dev_do_close -> hci_dev_close_sync -> cancel_work_sync(&hdev->power_on), causing deadlock. In particular, this happens when device is rfkilled on boot. To avoid deadlock, move power_on work canceling out of hci_dev_do_close/hci_dev_close_sync. Deadlock introduced by commit [1] was reported in [2,3] as broken suspend. Suspend did not work because `hdev->req_lock` held as result of `power_on` work deadlock. In fact, other BT features were not working. It was not observed when testing [1] since it was verified without rfkill in place. NOTE: It is not needed to cancel power_on work from other places where hci_dev_do_close/hci_dev_close_sync is called in case: * Requests were serialized due to `hdev->req_workqueue`. The power_on work is first in that workqueue. * hci_rfkill_set_block which won't close device anyway until HCI_SETUP is on. * hci_sock_release which runs after hci_sock_bind which ensures HCI_SETUP was cleared. As result, behaviour is the same as in pre-dd06ed7 commit, except power_on work cancel added to hci_dev_close. [1]: commit ff7f292 ("Bluetooth: core: Fix missing power_on work cancel on HCI close") [2]: https://lore.kernel.org/lkml/[email protected]/ [2]: https://lore.kernel.org/lkml/[email protected]/ Fixes: ff7f292 ("Bluetooth: core: Fix missing power_on work cancel on HCI close") Signed-off-by: Vasyl Vavrychuk <[email protected]> Reported-by: Max Krummenacher <[email protected]> Reported-by: Mateusz Jonczyk <[email protected]> Tested-by: Max Krummenacher <[email protected]> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
1 parent 029cc09 commit e36bea6

File tree

2 files changed

+3
-1
lines changed

2 files changed

+3
-1
lines changed

net/bluetooth/hci_core.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,7 @@ int hci_dev_close(__u16 dev)
571571
goto done;
572572
}
573573

574+
cancel_work_sync(&hdev->power_on);
574575
if (hci_dev_test_and_clear_flag(hdev, HCI_AUTO_OFF))
575576
cancel_delayed_work(&hdev->power_off);
576577

@@ -2675,6 +2676,8 @@ void hci_unregister_dev(struct hci_dev *hdev)
26752676
list_del(&hdev->list);
26762677
write_unlock(&hci_dev_list_lock);
26772678

2679+
cancel_work_sync(&hdev->power_on);
2680+
26782681
hci_cmd_sync_clear(hdev);
26792682

26802683
if (!test_bit(HCI_QUIRK_NO_SUSPEND_NOTIFIER, &hdev->quirks))

net/bluetooth/hci_sync.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4088,7 +4088,6 @@ int hci_dev_close_sync(struct hci_dev *hdev)
40884088

40894089
bt_dev_dbg(hdev, "");
40904090

4091-
cancel_work_sync(&hdev->power_on);
40924091
cancel_delayed_work(&hdev->power_off);
40934092
cancel_delayed_work(&hdev->ncmd_timer);
40944093

0 commit comments

Comments
 (0)