@@ -514,8 +514,8 @@ struct hci_dev {
struct work_struct error_reset;
struct work_struct cmd_sync_work;
struct list_head cmd_sync_work_list;
- struct mutex cmd_sync_work_lock;
- struct mutex unregister_lock;
+ spinlock_t cmd_sync_work_lock;
+ spinlock_t unregister_lock;
struct work_struct cmd_sync_cancel_work;
struct work_struct reenable_adv_work;
@@ -2716,9 +2716,9 @@ void hci_unregister_dev(struct hci_dev *hdev)
{
BT_DBG("%p name %s bus %d", hdev, hdev->name, hdev->bus);
- mutex_lock(&hdev->unregister_lock);
+ spin_lock(&hdev->unregister_lock);
hci_dev_set_flag(hdev, HCI_UNREGISTER);
- mutex_unlock(&hdev->unregister_lock);
+ spin_unlock(&hdev->unregister_lock);
write_lock(&hci_dev_list_lock);
list_del(&hdev->list);
@@ -286,13 +286,13 @@ static void hci_cmd_sync_work(struct work_struct *work)
while (1) {
struct hci_cmd_sync_work_entry *entry;
- mutex_lock(&hdev->cmd_sync_work_lock);
+ spin_lock(&hdev->cmd_sync_work_lock);
entry = list_first_entry_or_null(&hdev->cmd_sync_work_list,
struct hci_cmd_sync_work_entry,
list);
if (entry)
list_del(&entry->list);
- mutex_unlock(&hdev->cmd_sync_work_lock);
+ spin_unlock(&hdev->cmd_sync_work_lock);
if (!entry)
break;
@@ -629,8 +629,8 @@ void hci_cmd_sync_init(struct hci_dev *hdev)
{
INIT_WORK(&hdev->cmd_sync_work, hci_cmd_sync_work);
INIT_LIST_HEAD(&hdev->cmd_sync_work_list);
- mutex_init(&hdev->cmd_sync_work_lock);
- mutex_init(&hdev->unregister_lock);
+ spin_lock_init(&hdev->cmd_sync_work_lock);
+ spin_lock_init(&hdev->unregister_lock);
INIT_WORK(&hdev->cmd_sync_cancel_work, hci_cmd_sync_cancel_work);
INIT_WORK(&hdev->reenable_adv_work, reenable_adv);
@@ -646,15 +646,17 @@ void hci_cmd_sync_clear(struct hci_dev *hdev)
cancel_work_sync(&hdev->cmd_sync_work);
cancel_work_sync(&hdev->reenable_adv_work);
- mutex_lock(&hdev->cmd_sync_work_lock);
+ spin_lock(&hdev->cmd_sync_work_lock);
list_for_each_entry_safe(entry, tmp, &hdev->cmd_sync_work_list, list) {
+ list_del(&entry->list);
+ spin_unlock(&hdev->cmd_sync_work_lock);
+
if (entry->destroy)
entry->destroy(hdev, entry->data, -ECANCELED);
-
- list_del(&entry->list);
kfree(entry);
+ spin_lock(&hdev->cmd_sync_work_lock);
}
- mutex_unlock(&hdev->cmd_sync_work_lock);
+ spin_unlock(&hdev->cmd_sync_work_lock);
}
void __hci_cmd_sync_cancel(struct hci_dev *hdev, int err)
@@ -696,13 +698,13 @@ int hci_cmd_sync_submit(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
struct hci_cmd_sync_work_entry *entry;
int err = 0;
- mutex_lock(&hdev->unregister_lock);
+ spin_lock(&hdev->unregister_lock);
if (hci_dev_test_flag(hdev, HCI_UNREGISTER)) {
err = -ENODEV;
goto unlock;
}
- entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
if (!entry) {
err = -ENOMEM;
goto unlock;
@@ -711,14 +713,14 @@ int hci_cmd_sync_submit(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
entry->data = data;
entry->destroy = destroy;
- mutex_lock(&hdev->cmd_sync_work_lock);
+ spin_lock(&hdev->cmd_sync_work_lock);
list_add_tail(&entry->list, &hdev->cmd_sync_work_list);
- mutex_unlock(&hdev->cmd_sync_work_lock);
+ spin_unlock(&hdev->cmd_sync_work_lock);
queue_work(hdev->req_workqueue, &hdev->cmd_sync_work);
unlock:
- mutex_unlock(&hdev->unregister_lock);
+ spin_unlock(&hdev->unregister_lock);
return err;
}
EXPORT_SYMBOL(hci_cmd_sync_submit);
Syzbot reports a sleeping function called from invalid context problem. BUG: sleeping function called from invalid context at kernel/locking/mutex.c:580 in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 4440, name: kworker/u5:1 preempt_count: 0, expected: 0 RCU nest depth: 1, expected: 0 ... Call Trace: <TASK> __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0x125/0x1b0 lib/dump_stack.c:106 __might_resched+0x3c3/0x5e0 kernel/sched/core.c:10187 __mutex_lock_common kernel/locking/mutex.c:580 [inline] __mutex_lock+0xee/0x1340 kernel/locking/mutex.c:747 hci_cmd_sync_submit+0x3f/0x340 net/bluetooth/hci_sync.c:699 hci_cmd_sync_queue+0x79/0xa0 net/bluetooth/hci_sync.c:739 hci_abort_conn+0x15b/0x330 net/bluetooth/hci_conn.c:2928 hci_disconnect+0xc4/0x220 net/bluetooth/hci_conn.c:258 hci_link_tx_to net/bluetooth/hci_core.c:3421 [inline] __check_timeout net/bluetooth/hci_core.c:3567 [inline] __check_timeout+0x331/0x4e0 net/bluetooth/hci_core.c:3547 hci_sched_le net/bluetooth/hci_core.c:3750 [inline] hci_tx_work+0x818/0x1d30 net/bluetooth/hci_core.c:3828 process_one_work+0xaa2/0x16f0 kernel/workqueue.c:2600 worker_thread+0x687/0x1110 kernel/workqueue.c:2751 kthread+0x33a/0x430 kernel/kthread.c:389 ret_from_fork+0x2c/0x70 arch/x86/kernel/process.c:145 ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:304 Use rcu_read_lock() to protect hci_conn_hash->list in hci_link_tx_to(). rcu_read_lock() will disable preempt when CONFIG_PREEMPT_RCU is enabled. When it needs to abort connection, it will submit a command by hci_cmd_sync_submit() which has some sleeping functions like mutex_lock() and kmalloc(, GFP_KERNEL). That triggers the bug. Convert cmd_sync_work_lock and unregister_lock to spin_lock type and replace GFP_KERNEL with GFP_ATOMIC to fix the bug. Fixes: 1857c19941c8 ("Bluetooth: hci_sync: add lock to protect HCI_UNREGISTER") Fixes: 6a98e3836fa2 ("Bluetooth: Add helper for serialized HCI command execution") Reported-by: syzbot+e7be5be00de0c3c2d782@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=e7be5be00de0c3c2d782 Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> --- include/net/bluetooth/hci_core.h | 4 ++-- net/bluetooth/hci_core.c | 4 ++-- net/bluetooth/hci_sync.c | 28 +++++++++++++++------------- 3 files changed, 19 insertions(+), 17 deletions(-)