&hci_dev_list_lock is acquired under a2mp_chan_recv_cb(), which I think should be a softirq context cb. So it seems that the write_lock() on &hci_dev_list_lock should at least disable bh. hci_register_dev() and hci_unregister_dev() are exactly that two functions acquire &hci_dev_list_lock with write_lock(), and should be called under process context without disable bh at most case. Note that I am not sure whether this could happen at real, as I am not sure whether the rx callback could be invoked during register() and unregister(). <deadlock #1> hci_register_dev() --> write_lock(&hci_dev_list_lock) <interrupt> --> a2mp_chan_recv_cb() --> a2mp_discover_req() --> read_lock(&hci_dev_list_lock) <deadlock #2> hci_unregister_dev() --> write_lock(&hci_dev_list_lock) <interrupt> --> a2mp_chan_recv_cb() --> a2mp_discover_req() --> read_lock(&hci_dev_list_lock) This flaw was found by an experimental static analysis tool I am developing for irq-related deadlock. To prevent the potential problem, I change to write_lock_bh(). Signed-off-by: Chengfeng Ye <dg573847474@xxxxxxxxx> --- net/bluetooth/hci_core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index a5992f1b3c9b..dd3107daed03 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -2670,9 +2670,9 @@ int hci_register_dev(struct hci_dev *hdev) hci_dev_set_flag(hdev, HCI_BREDR_ENABLED); } - write_lock(&hci_dev_list_lock); + write_lock_bh(&hci_dev_list_lock); list_add(&hdev->list, &hci_dev_list); - write_unlock(&hci_dev_list_lock); + write_unlock_bh(&hci_dev_list_lock); /* Devices that are marked for raw-only usage are unconfigured * and should not be included in normal operation. @@ -2720,9 +2720,9 @@ void hci_unregister_dev(struct hci_dev *hdev) hci_dev_set_flag(hdev, HCI_UNREGISTER); mutex_unlock(&hdev->unregister_lock); - write_lock(&hci_dev_list_lock); + write_lock_bh(&hci_dev_list_lock); list_del(&hdev->list); - write_unlock(&hci_dev_list_lock); + write_unlock_bh(&hci_dev_list_lock); cancel_work_sync(&hdev->power_on); -- 2.17.1
