| Message ID | 20230907122234.146449-1-william.xuanziyang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bluetooth: Fix sleeping function called from invalid context in hci_cmd_sync_submit() | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | fail | WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (85>80): "Bluetooth: Fix sleeping function called from invalid context in hci_cmd_sync_submit()" |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/BuildKernel32 | success | BuildKernel32 PASS |
| tedd_an/TestRunnerSetup | success | TestRunnerSetup PASS |
| tedd_an/TestRunner_l2cap-tester | success | TestRunner PASS |
| tedd_an/TestRunner_iso-tester | success | TestRunner PASS |
| tedd_an/TestRunner_bnep-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mgmt-tester | success | TestRunner PASS |
| tedd_an/TestRunner_rfcomm-tester | success | TestRunner PASS |
| tedd_an/TestRunner_sco-tester | success | TestRunner PASS |
| tedd_an/TestRunner_ioctl-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mesh-tester | success | TestRunner PASS |
| tedd_an/TestRunner_smp-tester | success | TestRunner PASS |
| tedd_an/TestRunner_userchan-tester | success | TestRunner PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=782320 ---Test result--- Test Summary: CheckPatch PASS 1.26 seconds GitLint FAIL 0.69 seconds SubjectPrefix PASS 0.13 seconds BuildKernel PASS 40.08 seconds CheckAllWarning PASS 42.91 seconds CheckSparse PASS 49.03 seconds CheckSmatch PASS 132.91 seconds BuildKernel32 PASS 38.10 seconds TestRunnerSetup PASS 572.79 seconds TestRunner_l2cap-tester PASS 31.53 seconds TestRunner_iso-tester PASS 73.02 seconds TestRunner_bnep-tester PASS 12.11 seconds TestRunner_mgmt-tester PASS 247.10 seconds TestRunner_rfcomm-tester PASS 20.03 seconds TestRunner_sco-tester PASS 22.58 seconds TestRunner_ioctl-tester PASS 21.66 seconds TestRunner_mesh-tester PASS 16.07 seconds TestRunner_smp-tester PASS 16.94 seconds TestRunner_userchan-tester PASS 14.02 seconds IncrementalBuild PASS 34.24 seconds Details ############################## Test: GitLint - FAIL Desc: Run gitlint Output: Bluetooth: Fix sleeping function called from invalid context in hci_cmd_sync_submit() WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (85>80): "Bluetooth: Fix sleeping function called from invalid context in hci_cmd_sync_submit()" --- Regards, Linux Bluetooth
Hi, On Thu, Sep 7, 2023 at 5:22 AM Ziyang Xuan <william.xuanziyang@huawei.com> wrote: > > 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. Seems like this was already fixed by: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=d606d5f4024de697167584ad5136a3e56d7debcd > 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(-) > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index e6359f7346f1..7a046a80a1ac 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -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; > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index a5992f1b3c9b..c94bfde68228 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -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); > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > index 9b93653c6197..4356ee29cd5f 100644 > --- a/net/bluetooth/hci_sync.c > +++ b/net/bluetooth/hci_sync.c > @@ -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); > -- > 2.25.1 >
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index e6359f7346f1..7a046a80a1ac 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -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; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index a5992f1b3c9b..c94bfde68228 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -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); diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 9b93653c6197..4356ee29cd5f 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -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(-)