| Message ID | 20241204165849.392564-2-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 20c9afdfb2879dc9ee50e73a5e2d0fe8a06b1a75 |
| Headers | show |
| Series | [v2] Bluetooth: hci_core: Fix sleeping function called from invalid context | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | warning | CheckSparse WARNING net/bluetooth/hci_core.c:60:1: warning: symbol 'hci_cb_list_lock' was not declared. Should it be static?net/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:147:35: warning: array of flexible structures |
| 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 | fail | TestRunner_iso-tester: WARNING: possible circular locking dependency detected |
| 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 |
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=914655 ---Test result--- Test Summary: CheckPatch PENDING 0.41 seconds GitLint PENDING 0.26 seconds SubjectPrefix PASS 0.07 seconds BuildKernel PASS 25.20 seconds CheckAllWarning PASS 27.74 seconds CheckSparse WARNING 31.19 seconds BuildKernel32 PASS 24.93 seconds TestRunnerSetup PASS 446.35 seconds TestRunner_l2cap-tester PASS 20.67 seconds TestRunner_iso-tester FAIL 27.63 seconds TestRunner_bnep-tester PASS 4.83 seconds TestRunner_mgmt-tester PASS 121.29 seconds TestRunner_rfcomm-tester PASS 7.66 seconds TestRunner_sco-tester PASS 9.60 seconds TestRunner_ioctl-tester PASS 8.20 seconds TestRunner_mesh-tester PASS 6.06 seconds TestRunner_smp-tester PASS 7.02 seconds TestRunner_userchan-tester PASS 5.05 seconds IncrementalBuild PENDING 0.47 seconds Details ############################## Test: CheckPatch - PENDING Desc: Run checkpatch.pl script Output: ############################## Test: GitLint - PENDING Desc: Run gitlint Output: ############################## Test: CheckSparse - WARNING Desc: Run sparse tool with linux kernel Output: net/bluetooth/hci_core.c:60:1: warning: symbol 'hci_cb_list_lock' was not declared. Should it be static?net/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:147:35: warning: array of flexible structures ############################## Test: TestRunner_iso-tester - FAIL Desc: Run iso-tester with test-runner Output: WARNING: possible circular locking dependency detected Total: 125, Passed: 121 (96.8%), Failed: 0, Not Run: 4 ############################## Test: IncrementalBuild - PENDING Desc: Incremental build with the patches in the series Output: --- Regards, Linux Bluetooth
Hi Luiz, ke, 2024-12-04 kello 11:58 -0500, Luiz Augusto von Dentz kirjoitti: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This reworks hci_cb_list to not use mutex hci_cb_list_lock to avoid bugs > like the bellow: This also in principle allows callbacks to be still running on another CPU when hci_unregister_cb() exits. Not sure if that is unwanted. > > BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585 > in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2 > preempt_count: 0, expected: 0 > RCU nest depth: 1, expected: 0 > 4 locks held by kworker/u9:2/5070: > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline] > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335 > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline] > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335 > #2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914 > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline] > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline] > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915 > CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024 > Workqueue: hci0 hci_rx_work > Call Trace: > <TASK> > __dump_stack lib/dump_stack.c:88 [inline] > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114 > __might_resched+0x5d4/0x780 kernel/sched/core.c:10187 > __mutex_lock_common kernel/locking/mutex.c:585 [inline] > __mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752 > hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline] > hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939 > hci_event_func net/bluetooth/hci_event.c:7514 [inline] > hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569 > hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171 > process_one_work kernel/workqueue.c:3254 [inline] > process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335 > worker_thread+0x86d/0xd70 kernel/workqueue.c:3416 > kthread+0x2f0/0x390 kernel/kthread.c:388 > ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147 > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243 > </TASK> > > Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614 > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > include/net/bluetooth/hci_core.h | 108 ++++++++++++++++++++----------- > net/bluetooth/hci_core.c | 9 +-- > net/bluetooth/iso.c | 6 ++ > net/bluetooth/l2cap_core.c | 12 ++-- > net/bluetooth/rfcomm/core.c | 6 ++ > net/bluetooth/sco.c | 12 ++-- > 6 files changed, 97 insertions(+), 56 deletions(-) > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index ea798f07c5a2..ca22ead85dbe 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -804,7 +804,6 @@ struct hci_conn_params { > extern struct list_head hci_dev_list; > extern struct list_head hci_cb_list; > extern rwlock_t hci_dev_list_lock; > -extern struct mutex hci_cb_list_lock; > > #define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags) > #define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags) > @@ -2017,24 +2016,47 @@ struct hci_cb { > > char *name; > > + bool (*match) (struct hci_conn *conn); > void (*connect_cfm) (struct hci_conn *conn, __u8 status); > void (*disconn_cfm) (struct hci_conn *conn, __u8 status); > void (*security_cfm) (struct hci_conn *conn, __u8 status, > - __u8 encrypt); > + __u8 encrypt); > void (*key_change_cfm) (struct hci_conn *conn, __u8 status); > void (*role_switch_cfm) (struct hci_conn *conn, __u8 status, __u8 role); > }; > > +static inline void hci_cb_lookup(struct hci_conn *conn, struct list_head *list) > +{ > + struct hci_cb *cb, *cpy; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(cb, &hci_cb_list, list) { > + if (cb->match && cb->match(conn)) { > + cpy = kmalloc(sizeof(*cpy), GFP_ATOMIC); > + if (!cpy) > + break; > + > + *cpy = *cb; > + INIT_LIST_HEAD(&cpy->list); > + list_add_rcu(&cpy->list, list); > + } > + } > + rcu_read_unlock(); > +} > + > static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status) > { > - struct hci_cb *cb; > + struct list_head list; > + struct hci_cb *cb, *tmp; > > - mutex_lock(&hci_cb_list_lock); > - list_for_each_entry(cb, &hci_cb_list, list) { > + INIT_LIST_HEAD(&list); > + hci_cb_lookup(conn, &list); > + > + list_for_each_entry_safe(cb, tmp, &list, list) { > if (cb->connect_cfm) > cb->connect_cfm(conn, status); > + kfree(cb); > } > - mutex_unlock(&hci_cb_list_lock); > > if (conn->connect_cfm_cb) > conn->connect_cfm_cb(conn, status); > @@ -2042,22 +2064,43 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status) > > static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason) > { > - struct hci_cb *cb; > + struct list_head list; > + struct hci_cb *cb, *tmp; > > - mutex_lock(&hci_cb_list_lock); > - list_for_each_entry(cb, &hci_cb_list, list) { > + INIT_LIST_HEAD(&list); > + hci_cb_lookup(conn, &list); > + > + list_for_each_entry_safe(cb, tmp, &list, list) { > if (cb->disconn_cfm) > cb->disconn_cfm(conn, reason); > + kfree(cb); > } > - mutex_unlock(&hci_cb_list_lock); > > if (conn->disconn_cfm_cb) > conn->disconn_cfm_cb(conn, reason); > } > > +static inline void hci_security_cfm(struct hci_conn *conn, __u8 status, > + __u8 encrypt) > +{ > + struct list_head list; > + struct hci_cb *cb, *tmp; > + > + INIT_LIST_HEAD(&list); > + hci_cb_lookup(conn, &list); > + > + list_for_each_entry_safe(cb, tmp, &list, list) { > + if (cb->security_cfm) > + cb->security_cfm(conn, status, encrypt); > + kfree(cb); > + } > + > + if (conn->security_cfm_cb) > + conn->security_cfm_cb(conn, status); > +} > + > static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) > { > - struct hci_cb *cb; > __u8 encrypt; > > if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags)) > @@ -2065,20 +2108,11 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) > > encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00; > > - mutex_lock(&hci_cb_list_lock); > - list_for_each_entry(cb, &hci_cb_list, list) { > - if (cb->security_cfm) > - cb->security_cfm(conn, status, encrypt); > - } > - mutex_unlock(&hci_cb_list_lock); > - > - if (conn->security_cfm_cb) > - conn->security_cfm_cb(conn, status); > + hci_security_cfm(conn, status, encrypt); > } > > static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) > { > - struct hci_cb *cb; > __u8 encrypt; > > if (conn->state == BT_CONFIG) { > @@ -2105,40 +2139,38 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) > conn->sec_level = conn->pending_sec_level; > } > > - mutex_lock(&hci_cb_list_lock); > - list_for_each_entry(cb, &hci_cb_list, list) { > - if (cb->security_cfm) > - cb->security_cfm(conn, status, encrypt); > - } > - mutex_unlock(&hci_cb_list_lock); > - > - if (conn->security_cfm_cb) > - conn->security_cfm_cb(conn, status); > + hci_security_cfm(conn, status, encrypt); > } > > static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status) > { > - struct hci_cb *cb; > + struct list_head list; > + struct hci_cb *cb, *tmp; > > - mutex_lock(&hci_cb_list_lock); > - list_for_each_entry(cb, &hci_cb_list, list) { > + INIT_LIST_HEAD(&list); > + hci_cb_lookup(conn, &list); > + > + list_for_each_entry_safe(cb, tmp, &list, list) { > if (cb->key_change_cfm) > cb->key_change_cfm(conn, status); > + kfree(cb); > } > - mutex_unlock(&hci_cb_list_lock); > } > > static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status, > __u8 role) > { > - struct hci_cb *cb; > + struct list_head list; > + struct hci_cb *cb, *tmp; > > - mutex_lock(&hci_cb_list_lock); > - list_for_each_entry(cb, &hci_cb_list, list) { > + INIT_LIST_HEAD(&list); > + hci_cb_lookup(conn, &list); > + > + list_for_each_entry_safe(cb, tmp, &list, list) { > if (cb->role_switch_cfm) > cb->role_switch_cfm(conn, status, role); > + kfree(cb); > } > - mutex_unlock(&hci_cb_list_lock); > } > > static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type) > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index f9e19f9cb5a3..25d180d225c1 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -2993,9 +2993,7 @@ int hci_register_cb(struct hci_cb *cb) > { > BT_DBG("%p name %s", cb, cb->name); > > - mutex_lock(&hci_cb_list_lock); > - list_add_tail(&cb->list, &hci_cb_list); > - mutex_unlock(&hci_cb_list_lock); > + list_add_tail_rcu(&cb->list, &hci_cb_list); > > return 0; > } > @@ -3005,9 +3003,8 @@ int hci_unregister_cb(struct hci_cb *cb) > { > BT_DBG("%p name %s", cb, cb->name); > > - mutex_lock(&hci_cb_list_lock); > - list_del(&cb->list); > - mutex_unlock(&hci_cb_list_lock); > + list_del_rcu(&cb->list); > + synchronize_rcu(); > > return 0; > } > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c > index 8ed818254dc8..eccdaa16cba0 100644 > --- a/net/bluetooth/iso.c > +++ b/net/bluetooth/iso.c > @@ -2154,6 +2154,11 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) > return HCI_LM_ACCEPT; > } > > +static bool iso_match(struct hci_conn *hcon) > +{ > + return hcon->type == ISO_LINK || hcon->type == LE_LINK; > +} > + > static void iso_connect_cfm(struct hci_conn *hcon, __u8 status) > { > if (hcon->type != ISO_LINK) { > @@ -2335,6 +2340,7 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags) > > static struct hci_cb iso_cb = { > .name = "ISO", > + .match = iso_match, > .connect_cfm = iso_connect_cfm, > .disconn_cfm = iso_disconn_cfm, > }; > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 6544c1ed7143..27b4c4a2ba1f 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -7217,6 +7217,11 @@ static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c, > return NULL; > } > > +static bool l2cap_match(struct hci_conn *hcon) > +{ > + return hcon->type == ACL_LINK || hcon->type == LE_LINK; > +} > + > static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) > { > struct hci_dev *hdev = hcon->hdev; > @@ -7224,9 +7229,6 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) > struct l2cap_chan *pchan; > u8 dst_type; > > - if (hcon->type != ACL_LINK && hcon->type != LE_LINK) > - return; > - > BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status); > > if (status) { > @@ -7291,9 +7293,6 @@ int l2cap_disconn_ind(struct hci_conn *hcon) > > static void l2cap_disconn_cfm(struct hci_conn *hcon, u8 reason) > { > - if (hcon->type != ACL_LINK && hcon->type != LE_LINK) > - return; > - > BT_DBG("hcon %p reason %d", hcon, reason); > > l2cap_conn_del(hcon, bt_to_errno(reason)); > @@ -7572,6 +7571,7 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags) > > static struct hci_cb l2cap_cb = { > .name = "L2CAP", > + .match = l2cap_match, > .connect_cfm = l2cap_connect_cfm, > .disconn_cfm = l2cap_disconn_cfm, > .security_cfm = l2cap_security_cfm, > diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c > index ad5177e3a69b..4c56ca5a216c 100644 > --- a/net/bluetooth/rfcomm/core.c > +++ b/net/bluetooth/rfcomm/core.c > @@ -2134,6 +2134,11 @@ static int rfcomm_run(void *unused) > return 0; > } > > +static bool rfcomm_match(struct hci_conn *hcon) > +{ > + return hcon->type == ACL_LINK; > +} > + > static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt) > { > struct rfcomm_session *s; > @@ -2180,6 +2185,7 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt) > > static struct hci_cb rfcomm_cb = { > .name = "RFCOMM", > + .match = rfcomm_match, > .security_cfm = rfcomm_security_cfm > }; > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c > index 7eb8d3e04ec4..40c4957cfc0b 100644 > --- a/net/bluetooth/sco.c > +++ b/net/bluetooth/sco.c > @@ -1397,11 +1397,13 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) > return lm; > } > > +static bool sco_match(struct hci_conn *hcon) > +{ > + return hcon->type == SCO_LINK || hcon->type == ESCO_LINK; > +} > + > static void sco_connect_cfm(struct hci_conn *hcon, __u8 status) > { > - if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK) > - return; > - > BT_DBG("hcon %p bdaddr %pMR status %u", hcon, &hcon->dst, status); > > if (!status) { > @@ -1416,9 +1418,6 @@ static void sco_connect_cfm(struct hci_conn *hcon, __u8 status) > > static void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason) > { > - if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK) > - return; > - > BT_DBG("hcon %p reason %d", hcon, reason); > > sco_conn_del(hcon, bt_to_errno(reason)); > @@ -1444,6 +1443,7 @@ void sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb) > > static struct hci_cb sco_cb = { > .name = "SCO", > + .match = sco_match, > .connect_cfm = sco_connect_cfm, > .disconn_cfm = sco_disconn_cfm, > };
Hi Pauli, On Wed, Dec 4, 2024 at 1:44 PM Pauli Virtanen <pav@iki.fi> wrote: > > Hi Luiz, > > ke, 2024-12-04 kello 11:58 -0500, Luiz Augusto von Dentz kirjoitti: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > This reworks hci_cb_list to not use mutex hci_cb_list_lock to avoid bugs > > like the bellow: > > This also in principle allows callbacks to be still running on another > CPU when hci_unregister_cb() exits. Not sure if that is unwanted. Well hci_unregister_cb is only called if you unload the whole module e.g bt_exit, but you can't do that if the interface is not down afaik, so I don't think it is possible to hit the case where the copied entries would be invalid due to module being unloaded. > > > > BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585 > > in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2 > > preempt_count: 0, expected: 0 > > RCU nest depth: 1, expected: 0 > > 4 locks held by kworker/u9:2/5070: > > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline] > > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335 > > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline] > > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335 > > #2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914 > > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline] > > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline] > > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915 > > CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0 > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024 > > Workqueue: hci0 hci_rx_work > > Call Trace: > > <TASK> > > __dump_stack lib/dump_stack.c:88 [inline] > > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114 > > __might_resched+0x5d4/0x780 kernel/sched/core.c:10187 > > __mutex_lock_common kernel/locking/mutex.c:585 [inline] > > __mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752 > > hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline] > > hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939 > > hci_event_func net/bluetooth/hci_event.c:7514 [inline] > > hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569 > > hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171 > > process_one_work kernel/workqueue.c:3254 [inline] > > process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335 > > worker_thread+0x86d/0xd70 kernel/workqueue.c:3416 > > kthread+0x2f0/0x390 kernel/kthread.c:388 > > ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147 > > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243 > > </TASK> > > > > Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com > > Closes: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614 > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > --- > > include/net/bluetooth/hci_core.h | 108 ++++++++++++++++++++----------- > > net/bluetooth/hci_core.c | 9 +-- > > net/bluetooth/iso.c | 6 ++ > > net/bluetooth/l2cap_core.c | 12 ++-- > > net/bluetooth/rfcomm/core.c | 6 ++ > > net/bluetooth/sco.c | 12 ++-- > > 6 files changed, 97 insertions(+), 56 deletions(-) > > > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > > index ea798f07c5a2..ca22ead85dbe 100644 > > --- a/include/net/bluetooth/hci_core.h > > +++ b/include/net/bluetooth/hci_core.h > > @@ -804,7 +804,6 @@ struct hci_conn_params { > > extern struct list_head hci_dev_list; > > extern struct list_head hci_cb_list; > > extern rwlock_t hci_dev_list_lock; > > -extern struct mutex hci_cb_list_lock; > > > > #define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags) > > #define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags) > > @@ -2017,24 +2016,47 @@ struct hci_cb { > > > > char *name; > > > > + bool (*match) (struct hci_conn *conn); > > void (*connect_cfm) (struct hci_conn *conn, __u8 status); > > void (*disconn_cfm) (struct hci_conn *conn, __u8 status); > > void (*security_cfm) (struct hci_conn *conn, __u8 status, > > - __u8 encrypt); > > + __u8 encrypt); > > void (*key_change_cfm) (struct hci_conn *conn, __u8 status); > > void (*role_switch_cfm) (struct hci_conn *conn, __u8 status, __u8 role); > > }; > > > > +static inline void hci_cb_lookup(struct hci_conn *conn, struct list_head *list) > > +{ > > + struct hci_cb *cb, *cpy; > > + > > + rcu_read_lock(); > > + list_for_each_entry_rcu(cb, &hci_cb_list, list) { > > + if (cb->match && cb->match(conn)) { > > + cpy = kmalloc(sizeof(*cpy), GFP_ATOMIC); > > + if (!cpy) > > + break; > > + > > + *cpy = *cb; > > + INIT_LIST_HEAD(&cpy->list); > > + list_add_rcu(&cpy->list, list); > > + } > > + } > > + rcu_read_unlock(); > > +} > > + > > static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status) > > { > > - struct hci_cb *cb; > > + struct list_head list; > > + struct hci_cb *cb, *tmp; > > > > - mutex_lock(&hci_cb_list_lock); > > - list_for_each_entry(cb, &hci_cb_list, list) { > > + INIT_LIST_HEAD(&list); > > + hci_cb_lookup(conn, &list); > > + > > + list_for_each_entry_safe(cb, tmp, &list, list) { > > if (cb->connect_cfm) > > cb->connect_cfm(conn, status); > > + kfree(cb); > > } > > - mutex_unlock(&hci_cb_list_lock); > > > > if (conn->connect_cfm_cb) > > conn->connect_cfm_cb(conn, status); > > @@ -2042,22 +2064,43 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status) > > > > static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason) > > { > > - struct hci_cb *cb; > > + struct list_head list; > > + struct hci_cb *cb, *tmp; > > > > - mutex_lock(&hci_cb_list_lock); > > - list_for_each_entry(cb, &hci_cb_list, list) { > > + INIT_LIST_HEAD(&list); > > + hci_cb_lookup(conn, &list); > > + > > + list_for_each_entry_safe(cb, tmp, &list, list) { > > if (cb->disconn_cfm) > > cb->disconn_cfm(conn, reason); > > + kfree(cb); > > } > > - mutex_unlock(&hci_cb_list_lock); > > > > if (conn->disconn_cfm_cb) > > conn->disconn_cfm_cb(conn, reason); > > } > > > > +static inline void hci_security_cfm(struct hci_conn *conn, __u8 status, > > + __u8 encrypt) > > +{ > > + struct list_head list; > > + struct hci_cb *cb, *tmp; > > + > > + INIT_LIST_HEAD(&list); > > + hci_cb_lookup(conn, &list); > > + > > + list_for_each_entry_safe(cb, tmp, &list, list) { > > + if (cb->security_cfm) > > + cb->security_cfm(conn, status, encrypt); > > + kfree(cb); > > + } > > + > > + if (conn->security_cfm_cb) > > + conn->security_cfm_cb(conn, status); > > +} > > + > > static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) > > { > > - struct hci_cb *cb; > > __u8 encrypt; > > > > if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags)) > > @@ -2065,20 +2108,11 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) > > > > encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00; > > > > - mutex_lock(&hci_cb_list_lock); > > - list_for_each_entry(cb, &hci_cb_list, list) { > > - if (cb->security_cfm) > > - cb->security_cfm(conn, status, encrypt); > > - } > > - mutex_unlock(&hci_cb_list_lock); > > - > > - if (conn->security_cfm_cb) > > - conn->security_cfm_cb(conn, status); > > + hci_security_cfm(conn, status, encrypt); > > } > > > > static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) > > { > > - struct hci_cb *cb; > > __u8 encrypt; > > > > if (conn->state == BT_CONFIG) { > > @@ -2105,40 +2139,38 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) > > conn->sec_level = conn->pending_sec_level; > > } > > > > - mutex_lock(&hci_cb_list_lock); > > - list_for_each_entry(cb, &hci_cb_list, list) { > > - if (cb->security_cfm) > > - cb->security_cfm(conn, status, encrypt); > > - } > > - mutex_unlock(&hci_cb_list_lock); > > - > > - if (conn->security_cfm_cb) > > - conn->security_cfm_cb(conn, status); > > + hci_security_cfm(conn, status, encrypt); > > } > > > > static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status) > > { > > - struct hci_cb *cb; > > + struct list_head list; > > + struct hci_cb *cb, *tmp; > > > > - mutex_lock(&hci_cb_list_lock); > > - list_for_each_entry(cb, &hci_cb_list, list) { > > + INIT_LIST_HEAD(&list); > > + hci_cb_lookup(conn, &list); > > + > > + list_for_each_entry_safe(cb, tmp, &list, list) { > > if (cb->key_change_cfm) > > cb->key_change_cfm(conn, status); > > + kfree(cb); > > } > > - mutex_unlock(&hci_cb_list_lock); > > } > > > > static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status, > > __u8 role) > > { > > - struct hci_cb *cb; > > + struct list_head list; > > + struct hci_cb *cb, *tmp; > > > > - mutex_lock(&hci_cb_list_lock); > > - list_for_each_entry(cb, &hci_cb_list, list) { > > + INIT_LIST_HEAD(&list); > > + hci_cb_lookup(conn, &list); > > + > > + list_for_each_entry_safe(cb, tmp, &list, list) { > > if (cb->role_switch_cfm) > > cb->role_switch_cfm(conn, status, role); > > + kfree(cb); > > } > > - mutex_unlock(&hci_cb_list_lock); > > } > > > > static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type) > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > > index f9e19f9cb5a3..25d180d225c1 100644 > > --- a/net/bluetooth/hci_core.c > > +++ b/net/bluetooth/hci_core.c > > @@ -2993,9 +2993,7 @@ int hci_register_cb(struct hci_cb *cb) > > { > > BT_DBG("%p name %s", cb, cb->name); > > > > - mutex_lock(&hci_cb_list_lock); > > - list_add_tail(&cb->list, &hci_cb_list); > > - mutex_unlock(&hci_cb_list_lock); > > + list_add_tail_rcu(&cb->list, &hci_cb_list); > > > > return 0; > > } > > @@ -3005,9 +3003,8 @@ int hci_unregister_cb(struct hci_cb *cb) > > { > > BT_DBG("%p name %s", cb, cb->name); > > > > - mutex_lock(&hci_cb_list_lock); > > - list_del(&cb->list); > > - mutex_unlock(&hci_cb_list_lock); > > + list_del_rcu(&cb->list); > > + synchronize_rcu(); > > > > return 0; > > } > > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c > > index 8ed818254dc8..eccdaa16cba0 100644 > > --- a/net/bluetooth/iso.c > > +++ b/net/bluetooth/iso.c > > @@ -2154,6 +2154,11 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) > > return HCI_LM_ACCEPT; > > } > > > > +static bool iso_match(struct hci_conn *hcon) > > +{ > > + return hcon->type == ISO_LINK || hcon->type == LE_LINK; > > +} > > + > > static void iso_connect_cfm(struct hci_conn *hcon, __u8 status) > > { > > if (hcon->type != ISO_LINK) { > > @@ -2335,6 +2340,7 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags) > > > > static struct hci_cb iso_cb = { > > .name = "ISO", > > + .match = iso_match, > > .connect_cfm = iso_connect_cfm, > > .disconn_cfm = iso_disconn_cfm, > > }; > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > index 6544c1ed7143..27b4c4a2ba1f 100644 > > --- a/net/bluetooth/l2cap_core.c > > +++ b/net/bluetooth/l2cap_core.c > > @@ -7217,6 +7217,11 @@ static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c, > > return NULL; > > } > > > > +static bool l2cap_match(struct hci_conn *hcon) > > +{ > > + return hcon->type == ACL_LINK || hcon->type == LE_LINK; > > +} > > + > > static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) > > { > > struct hci_dev *hdev = hcon->hdev; > > @@ -7224,9 +7229,6 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) > > struct l2cap_chan *pchan; > > u8 dst_type; > > > > - if (hcon->type != ACL_LINK && hcon->type != LE_LINK) > > - return; > > - > > BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status); > > > > if (status) { > > @@ -7291,9 +7293,6 @@ int l2cap_disconn_ind(struct hci_conn *hcon) > > > > static void l2cap_disconn_cfm(struct hci_conn *hcon, u8 reason) > > { > > - if (hcon->type != ACL_LINK && hcon->type != LE_LINK) > > - return; > > - > > BT_DBG("hcon %p reason %d", hcon, reason); > > > > l2cap_conn_del(hcon, bt_to_errno(reason)); > > @@ -7572,6 +7571,7 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags) > > > > static struct hci_cb l2cap_cb = { > > .name = "L2CAP", > > + .match = l2cap_match, > > .connect_cfm = l2cap_connect_cfm, > > .disconn_cfm = l2cap_disconn_cfm, > > .security_cfm = l2cap_security_cfm, > > diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c > > index ad5177e3a69b..4c56ca5a216c 100644 > > --- a/net/bluetooth/rfcomm/core.c > > +++ b/net/bluetooth/rfcomm/core.c > > @@ -2134,6 +2134,11 @@ static int rfcomm_run(void *unused) > > return 0; > > } > > > > +static bool rfcomm_match(struct hci_conn *hcon) > > +{ > > + return hcon->type == ACL_LINK; > > +} > > + > > static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt) > > { > > struct rfcomm_session *s; > > @@ -2180,6 +2185,7 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt) > > > > static struct hci_cb rfcomm_cb = { > > .name = "RFCOMM", > > + .match = rfcomm_match, > > .security_cfm = rfcomm_security_cfm > > }; > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c > > index 7eb8d3e04ec4..40c4957cfc0b 100644 > > --- a/net/bluetooth/sco.c > > +++ b/net/bluetooth/sco.c > > @@ -1397,11 +1397,13 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) > > return lm; > > } > > > > +static bool sco_match(struct hci_conn *hcon) > > +{ > > + return hcon->type == SCO_LINK || hcon->type == ESCO_LINK; > > +} > > + > > static void sco_connect_cfm(struct hci_conn *hcon, __u8 status) > > { > > - if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK) > > - return; > > - > > BT_DBG("hcon %p bdaddr %pMR status %u", hcon, &hcon->dst, status); > > > > if (!status) { > > @@ -1416,9 +1418,6 @@ static void sco_connect_cfm(struct hci_conn *hcon, __u8 status) > > > > static void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason) > > { > > - if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK) > > - return; > > - > > BT_DBG("hcon %p reason %d", hcon, reason); > > > > sco_conn_del(hcon, bt_to_errno(reason)); > > @@ -1444,6 +1443,7 @@ void sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb) > > > > static struct hci_cb sco_cb = { > > .name = "SCO", > > + .match = sco_match, > > .connect_cfm = sco_connect_cfm, > > .disconn_cfm = sco_disconn_cfm, > > }; >
#syz test On Wed, Dec 4, 2024 at 1:59 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Pauli, > > On Wed, Dec 4, 2024 at 1:44 PM Pauli Virtanen <pav@iki.fi> wrote: > > > > Hi Luiz, > > > > ke, 2024-12-04 kello 11:58 -0500, Luiz Augusto von Dentz kirjoitti: > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > > > This reworks hci_cb_list to not use mutex hci_cb_list_lock to avoid bugs > > > like the bellow: > > > > This also in principle allows callbacks to be still running on another > > CPU when hci_unregister_cb() exits. Not sure if that is unwanted. > > Well hci_unregister_cb is only called if you unload the whole module > e.g bt_exit, but you can't do that if the interface is not down afaik, > so I don't think it is possible to hit the case where the copied > entries would be invalid due to module being unloaded. > > > > > > > BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585 > > > in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2 > > > preempt_count: 0, expected: 0 > > > RCU nest depth: 1, expected: 0 > > > 4 locks held by kworker/u9:2/5070: > > > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline] > > > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335 > > > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline] > > > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335 > > > #2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914 > > > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline] > > > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline] > > > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915 > > > CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0 > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024 > > > Workqueue: hci0 hci_rx_work > > > Call Trace: > > > <TASK> > > > __dump_stack lib/dump_stack.c:88 [inline] > > > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114 > > > __might_resched+0x5d4/0x780 kernel/sched/core.c:10187 > > > __mutex_lock_common kernel/locking/mutex.c:585 [inline] > > > __mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752 > > > hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline] > > > hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939 > > > hci_event_func net/bluetooth/hci_event.c:7514 [inline] > > > hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569 > > > hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171 > > > process_one_work kernel/workqueue.c:3254 [inline] > > > process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335 > > > worker_thread+0x86d/0xd70 kernel/workqueue.c:3416 > > > kthread+0x2f0/0x390 kernel/kthread.c:388 > > > ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147 > > > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243 > > > </TASK> > > > > > > Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com > > > Closes: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614 > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > --- > > > include/net/bluetooth/hci_core.h | 108 ++++++++++++++++++++----------- > > > net/bluetooth/hci_core.c | 9 +-- > > > net/bluetooth/iso.c | 6 ++ > > > net/bluetooth/l2cap_core.c | 12 ++-- > > > net/bluetooth/rfcomm/core.c | 6 ++ > > > net/bluetooth/sco.c | 12 ++-- > > > 6 files changed, 97 insertions(+), 56 deletions(-) > > > > > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > > > index ea798f07c5a2..ca22ead85dbe 100644 > > > --- a/include/net/bluetooth/hci_core.h > > > +++ b/include/net/bluetooth/hci_core.h > > > @@ -804,7 +804,6 @@ struct hci_conn_params { > > > extern struct list_head hci_dev_list; > > > extern struct list_head hci_cb_list; > > > extern rwlock_t hci_dev_list_lock; > > > -extern struct mutex hci_cb_list_lock; > > > > > > #define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags) > > > #define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags) > > > @@ -2017,24 +2016,47 @@ struct hci_cb { > > > > > > char *name; > > > > > > + bool (*match) (struct hci_conn *conn); > > > void (*connect_cfm) (struct hci_conn *conn, __u8 status); > > > void (*disconn_cfm) (struct hci_conn *conn, __u8 status); > > > void (*security_cfm) (struct hci_conn *conn, __u8 status, > > > - __u8 encrypt); > > > + __u8 encrypt); > > > void (*key_change_cfm) (struct hci_conn *conn, __u8 status); > > > void (*role_switch_cfm) (struct hci_conn *conn, __u8 status, __u8 role); > > > }; > > > > > > +static inline void hci_cb_lookup(struct hci_conn *conn, struct list_head *list) > > > +{ > > > + struct hci_cb *cb, *cpy; > > > + > > > + rcu_read_lock(); > > > + list_for_each_entry_rcu(cb, &hci_cb_list, list) { > > > + if (cb->match && cb->match(conn)) { > > > + cpy = kmalloc(sizeof(*cpy), GFP_ATOMIC); > > > + if (!cpy) > > > + break; > > > + > > > + *cpy = *cb; > > > + INIT_LIST_HEAD(&cpy->list); > > > + list_add_rcu(&cpy->list, list); > > > + } > > > + } > > > + rcu_read_unlock(); > > > +} > > > + > > > static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status) > > > { > > > - struct hci_cb *cb; > > > + struct list_head list; > > > + struct hci_cb *cb, *tmp; > > > > > > - mutex_lock(&hci_cb_list_lock); > > > - list_for_each_entry(cb, &hci_cb_list, list) { > > > + INIT_LIST_HEAD(&list); > > > + hci_cb_lookup(conn, &list); > > > + > > > + list_for_each_entry_safe(cb, tmp, &list, list) { > > > if (cb->connect_cfm) > > > cb->connect_cfm(conn, status); > > > + kfree(cb); > > > } > > > - mutex_unlock(&hci_cb_list_lock); > > > > > > if (conn->connect_cfm_cb) > > > conn->connect_cfm_cb(conn, status); > > > @@ -2042,22 +2064,43 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status) > > > > > > static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason) > > > { > > > - struct hci_cb *cb; > > > + struct list_head list; > > > + struct hci_cb *cb, *tmp; > > > > > > - mutex_lock(&hci_cb_list_lock); > > > - list_for_each_entry(cb, &hci_cb_list, list) { > > > + INIT_LIST_HEAD(&list); > > > + hci_cb_lookup(conn, &list); > > > + > > > + list_for_each_entry_safe(cb, tmp, &list, list) { > > > if (cb->disconn_cfm) > > > cb->disconn_cfm(conn, reason); > > > + kfree(cb); > > > } > > > - mutex_unlock(&hci_cb_list_lock); > > > > > > if (conn->disconn_cfm_cb) > > > conn->disconn_cfm_cb(conn, reason); > > > } > > > > > > +static inline void hci_security_cfm(struct hci_conn *conn, __u8 status, > > > + __u8 encrypt) > > > +{ > > > + struct list_head list; > > > + struct hci_cb *cb, *tmp; > > > + > > > + INIT_LIST_HEAD(&list); > > > + hci_cb_lookup(conn, &list); > > > + > > > + list_for_each_entry_safe(cb, tmp, &list, list) { > > > + if (cb->security_cfm) > > > + cb->security_cfm(conn, status, encrypt); > > > + kfree(cb); > > > + } > > > + > > > + if (conn->security_cfm_cb) > > > + conn->security_cfm_cb(conn, status); > > > +} > > > + > > > static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) > > > { > > > - struct hci_cb *cb; > > > __u8 encrypt; > > > > > > if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags)) > > > @@ -2065,20 +2108,11 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) > > > > > > encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00; > > > > > > - mutex_lock(&hci_cb_list_lock); > > > - list_for_each_entry(cb, &hci_cb_list, list) { > > > - if (cb->security_cfm) > > > - cb->security_cfm(conn, status, encrypt); > > > - } > > > - mutex_unlock(&hci_cb_list_lock); > > > - > > > - if (conn->security_cfm_cb) > > > - conn->security_cfm_cb(conn, status); > > > + hci_security_cfm(conn, status, encrypt); > > > } > > > > > > static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) > > > { > > > - struct hci_cb *cb; > > > __u8 encrypt; > > > > > > if (conn->state == BT_CONFIG) { > > > @@ -2105,40 +2139,38 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) > > > conn->sec_level = conn->pending_sec_level; > > > } > > > > > > - mutex_lock(&hci_cb_list_lock); > > > - list_for_each_entry(cb, &hci_cb_list, list) { > > > - if (cb->security_cfm) > > > - cb->security_cfm(conn, status, encrypt); > > > - } > > > - mutex_unlock(&hci_cb_list_lock); > > > - > > > - if (conn->security_cfm_cb) > > > - conn->security_cfm_cb(conn, status); > > > + hci_security_cfm(conn, status, encrypt); > > > } > > > > > > static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status) > > > { > > > - struct hci_cb *cb; > > > + struct list_head list; > > > + struct hci_cb *cb, *tmp; > > > > > > - mutex_lock(&hci_cb_list_lock); > > > - list_for_each_entry(cb, &hci_cb_list, list) { > > > + INIT_LIST_HEAD(&list); > > > + hci_cb_lookup(conn, &list); > > > + > > > + list_for_each_entry_safe(cb, tmp, &list, list) { > > > if (cb->key_change_cfm) > > > cb->key_change_cfm(conn, status); > > > + kfree(cb); > > > } > > > - mutex_unlock(&hci_cb_list_lock); > > > } > > > > > > static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status, > > > __u8 role) > > > { > > > - struct hci_cb *cb; > > > + struct list_head list; > > > + struct hci_cb *cb, *tmp; > > > > > > - mutex_lock(&hci_cb_list_lock); > > > - list_for_each_entry(cb, &hci_cb_list, list) { > > > + INIT_LIST_HEAD(&list); > > > + hci_cb_lookup(conn, &list); > > > + > > > + list_for_each_entry_safe(cb, tmp, &list, list) { > > > if (cb->role_switch_cfm) > > > cb->role_switch_cfm(conn, status, role); > > > + kfree(cb); > > > } > > > - mutex_unlock(&hci_cb_list_lock); > > > } > > > > > > static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type) > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > > > index f9e19f9cb5a3..25d180d225c1 100644 > > > --- a/net/bluetooth/hci_core.c > > > +++ b/net/bluetooth/hci_core.c > > > @@ -2993,9 +2993,7 @@ int hci_register_cb(struct hci_cb *cb) > > > { > > > BT_DBG("%p name %s", cb, cb->name); > > > > > > - mutex_lock(&hci_cb_list_lock); > > > - list_add_tail(&cb->list, &hci_cb_list); > > > - mutex_unlock(&hci_cb_list_lock); > > > + list_add_tail_rcu(&cb->list, &hci_cb_list); > > > > > > return 0; > > > } > > > @@ -3005,9 +3003,8 @@ int hci_unregister_cb(struct hci_cb *cb) > > > { > > > BT_DBG("%p name %s", cb, cb->name); > > > > > > - mutex_lock(&hci_cb_list_lock); > > > - list_del(&cb->list); > > > - mutex_unlock(&hci_cb_list_lock); > > > + list_del_rcu(&cb->list); > > > + synchronize_rcu(); > > > > > > return 0; > > > } > > > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c > > > index 8ed818254dc8..eccdaa16cba0 100644 > > > --- a/net/bluetooth/iso.c > > > +++ b/net/bluetooth/iso.c > > > @@ -2154,6 +2154,11 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) > > > return HCI_LM_ACCEPT; > > > } > > > > > > +static bool iso_match(struct hci_conn *hcon) > > > +{ > > > + return hcon->type == ISO_LINK || hcon->type == LE_LINK; > > > +} > > > + > > > static void iso_connect_cfm(struct hci_conn *hcon, __u8 status) > > > { > > > if (hcon->type != ISO_LINK) { > > > @@ -2335,6 +2340,7 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags) > > > > > > static struct hci_cb iso_cb = { > > > .name = "ISO", > > > + .match = iso_match, > > > .connect_cfm = iso_connect_cfm, > > > .disconn_cfm = iso_disconn_cfm, > > > }; > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > > index 6544c1ed7143..27b4c4a2ba1f 100644 > > > --- a/net/bluetooth/l2cap_core.c > > > +++ b/net/bluetooth/l2cap_core.c > > > @@ -7217,6 +7217,11 @@ static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c, > > > return NULL; > > > } > > > > > > +static bool l2cap_match(struct hci_conn *hcon) > > > +{ > > > + return hcon->type == ACL_LINK || hcon->type == LE_LINK; > > > +} > > > + > > > static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) > > > { > > > struct hci_dev *hdev = hcon->hdev; > > > @@ -7224,9 +7229,6 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) > > > struct l2cap_chan *pchan; > > > u8 dst_type; > > > > > > - if (hcon->type != ACL_LINK && hcon->type != LE_LINK) > > > - return; > > > - > > > BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status); > > > > > > if (status) { > > > @@ -7291,9 +7293,6 @@ int l2cap_disconn_ind(struct hci_conn *hcon) > > > > > > static void l2cap_disconn_cfm(struct hci_conn *hcon, u8 reason) > > > { > > > - if (hcon->type != ACL_LINK && hcon->type != LE_LINK) > > > - return; > > > - > > > BT_DBG("hcon %p reason %d", hcon, reason); > > > > > > l2cap_conn_del(hcon, bt_to_errno(reason)); > > > @@ -7572,6 +7571,7 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags) > > > > > > static struct hci_cb l2cap_cb = { > > > .name = "L2CAP", > > > + .match = l2cap_match, > > > .connect_cfm = l2cap_connect_cfm, > > > .disconn_cfm = l2cap_disconn_cfm, > > > .security_cfm = l2cap_security_cfm, > > > diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c > > > index ad5177e3a69b..4c56ca5a216c 100644 > > > --- a/net/bluetooth/rfcomm/core.c > > > +++ b/net/bluetooth/rfcomm/core.c > > > @@ -2134,6 +2134,11 @@ static int rfcomm_run(void *unused) > > > return 0; > > > } > > > > > > +static bool rfcomm_match(struct hci_conn *hcon) > > > +{ > > > + return hcon->type == ACL_LINK; > > > +} > > > + > > > static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt) > > > { > > > struct rfcomm_session *s; > > > @@ -2180,6 +2185,7 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt) > > > > > > static struct hci_cb rfcomm_cb = { > > > .name = "RFCOMM", > > > + .match = rfcomm_match, > > > .security_cfm = rfcomm_security_cfm > > > }; > > > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c > > > index 7eb8d3e04ec4..40c4957cfc0b 100644 > > > --- a/net/bluetooth/sco.c > > > +++ b/net/bluetooth/sco.c > > > @@ -1397,11 +1397,13 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) > > > return lm; > > > } > > > > > > +static bool sco_match(struct hci_conn *hcon) > > > +{ > > > + return hcon->type == SCO_LINK || hcon->type == ESCO_LINK; > > > +} > > > + > > > static void sco_connect_cfm(struct hci_conn *hcon, __u8 status) > > > { > > > - if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK) > > > - return; > > > - > > > BT_DBG("hcon %p bdaddr %pMR status %u", hcon, &hcon->dst, status); > > > > > > if (!status) { > > > @@ -1416,9 +1418,6 @@ static void sco_connect_cfm(struct hci_conn *hcon, __u8 status) > > > > > > static void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason) > > > { > > > - if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK) > > > - return; > > > - > > > BT_DBG("hcon %p reason %d", hcon, reason); > > > > > > sco_conn_del(hcon, bt_to_errno(reason)); > > > @@ -1444,6 +1443,7 @@ void sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb) > > > > > > static struct hci_cb sco_cb = { > > > .name = "SCO", > > > + .match = sco_match, > > > .connect_cfm = sco_connect_cfm, > > > .disconn_cfm = sco_disconn_cfm, > > > }; > > > > > -- > Luiz Augusto von Dentz
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
Tested-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
Tested on:
commit: feffde68 Merge tag 'for-6.13-rc1-tag' of git://git.ker..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=144e1330580000
kernel config: https://syzkaller.appspot.com/x/.config?x=773e6b07384b3087
dashboard link: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=178490f8580000
Note: testing is done by a robot and is best-effort only.
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Wed, 4 Dec 2024 11:58:49 -0500 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This reworks hci_cb_list to not use mutex hci_cb_list_lock to avoid bugs > like the bellow: > > BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585 > in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2 > preempt_count: 0, expected: 0 > RCU nest depth: 1, expected: 0 > 4 locks held by kworker/u9:2/5070: > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline] > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335 > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline] > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335 > #2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914 > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline] > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline] > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915 > CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024 > Workqueue: hci0 hci_rx_work > Call Trace: > <TASK> > __dump_stack lib/dump_stack.c:88 [inline] > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114 > __might_resched+0x5d4/0x780 kernel/sched/core.c:10187 > __mutex_lock_common kernel/locking/mutex.c:585 [inline] > __mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752 > hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline] > hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939 > hci_event_func net/bluetooth/hci_event.c:7514 [inline] > hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569 > hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171 > process_one_work kernel/workqueue.c:3254 [inline] > process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335 > worker_thread+0x86d/0xd70 kernel/workqueue.c:3416 > kthread+0x2f0/0x390 kernel/kthread.c:388 > ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147 > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243 > </TASK> > > [...] Here is the summary with links: - [v2] Bluetooth: hci_core: Fix sleeping function called from invalid context https://git.kernel.org/bluetooth/bluetooth-next/c/20c9afdfb287 You are awesome, thank you!
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index ea798f07c5a2..ca22ead85dbe 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -804,7 +804,6 @@ struct hci_conn_params { extern struct list_head hci_dev_list; extern struct list_head hci_cb_list; extern rwlock_t hci_dev_list_lock; -extern struct mutex hci_cb_list_lock; #define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags) #define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags) @@ -2017,24 +2016,47 @@ struct hci_cb { char *name; + bool (*match) (struct hci_conn *conn); void (*connect_cfm) (struct hci_conn *conn, __u8 status); void (*disconn_cfm) (struct hci_conn *conn, __u8 status); void (*security_cfm) (struct hci_conn *conn, __u8 status, - __u8 encrypt); + __u8 encrypt); void (*key_change_cfm) (struct hci_conn *conn, __u8 status); void (*role_switch_cfm) (struct hci_conn *conn, __u8 status, __u8 role); }; +static inline void hci_cb_lookup(struct hci_conn *conn, struct list_head *list) +{ + struct hci_cb *cb, *cpy; + + rcu_read_lock(); + list_for_each_entry_rcu(cb, &hci_cb_list, list) { + if (cb->match && cb->match(conn)) { + cpy = kmalloc(sizeof(*cpy), GFP_ATOMIC); + if (!cpy) + break; + + *cpy = *cb; + INIT_LIST_HEAD(&cpy->list); + list_add_rcu(&cpy->list, list); + } + } + rcu_read_unlock(); +} + static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status) { - struct hci_cb *cb; + struct list_head list; + struct hci_cb *cb, *tmp; - mutex_lock(&hci_cb_list_lock); - list_for_each_entry(cb, &hci_cb_list, list) { + INIT_LIST_HEAD(&list); + hci_cb_lookup(conn, &list); + + list_for_each_entry_safe(cb, tmp, &list, list) { if (cb->connect_cfm) cb->connect_cfm(conn, status); + kfree(cb); } - mutex_unlock(&hci_cb_list_lock); if (conn->connect_cfm_cb) conn->connect_cfm_cb(conn, status); @@ -2042,22 +2064,43 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status) static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason) { - struct hci_cb *cb; + struct list_head list; + struct hci_cb *cb, *tmp; - mutex_lock(&hci_cb_list_lock); - list_for_each_entry(cb, &hci_cb_list, list) { + INIT_LIST_HEAD(&list); + hci_cb_lookup(conn, &list); + + list_for_each_entry_safe(cb, tmp, &list, list) { if (cb->disconn_cfm) cb->disconn_cfm(conn, reason); + kfree(cb); } - mutex_unlock(&hci_cb_list_lock); if (conn->disconn_cfm_cb) conn->disconn_cfm_cb(conn, reason); } +static inline void hci_security_cfm(struct hci_conn *conn, __u8 status, + __u8 encrypt) +{ + struct list_head list; + struct hci_cb *cb, *tmp; + + INIT_LIST_HEAD(&list); + hci_cb_lookup(conn, &list); + + list_for_each_entry_safe(cb, tmp, &list, list) { + if (cb->security_cfm) + cb->security_cfm(conn, status, encrypt); + kfree(cb); + } + + if (conn->security_cfm_cb) + conn->security_cfm_cb(conn, status); +} + static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) { - struct hci_cb *cb; __u8 encrypt; if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags)) @@ -2065,20 +2108,11 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00; - mutex_lock(&hci_cb_list_lock); - list_for_each_entry(cb, &hci_cb_list, list) { - if (cb->security_cfm) - cb->security_cfm(conn, status, encrypt); - } - mutex_unlock(&hci_cb_list_lock); - - if (conn->security_cfm_cb) - conn->security_cfm_cb(conn, status); + hci_security_cfm(conn, status, encrypt); } static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) { - struct hci_cb *cb; __u8 encrypt; if (conn->state == BT_CONFIG) { @@ -2105,40 +2139,38 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) conn->sec_level = conn->pending_sec_level; } - mutex_lock(&hci_cb_list_lock); - list_for_each_entry(cb, &hci_cb_list, list) { - if (cb->security_cfm) - cb->security_cfm(conn, status, encrypt); - } - mutex_unlock(&hci_cb_list_lock); - - if (conn->security_cfm_cb) - conn->security_cfm_cb(conn, status); + hci_security_cfm(conn, status, encrypt); } static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status) { - struct hci_cb *cb; + struct list_head list; + struct hci_cb *cb, *tmp; - mutex_lock(&hci_cb_list_lock); - list_for_each_entry(cb, &hci_cb_list, list) { + INIT_LIST_HEAD(&list); + hci_cb_lookup(conn, &list); + + list_for_each_entry_safe(cb, tmp, &list, list) { if (cb->key_change_cfm) cb->key_change_cfm(conn, status); + kfree(cb); } - mutex_unlock(&hci_cb_list_lock); } static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status, __u8 role) { - struct hci_cb *cb; + struct list_head list; + struct hci_cb *cb, *tmp; - mutex_lock(&hci_cb_list_lock); - list_for_each_entry(cb, &hci_cb_list, list) { + INIT_LIST_HEAD(&list); + hci_cb_lookup(conn, &list); + + list_for_each_entry_safe(cb, tmp, &list, list) { if (cb->role_switch_cfm) cb->role_switch_cfm(conn, status, role); + kfree(cb); } - mutex_unlock(&hci_cb_list_lock); } static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index f9e19f9cb5a3..25d180d225c1 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -2993,9 +2993,7 @@ int hci_register_cb(struct hci_cb *cb) { BT_DBG("%p name %s", cb, cb->name); - mutex_lock(&hci_cb_list_lock); - list_add_tail(&cb->list, &hci_cb_list); - mutex_unlock(&hci_cb_list_lock); + list_add_tail_rcu(&cb->list, &hci_cb_list); return 0; } @@ -3005,9 +3003,8 @@ int hci_unregister_cb(struct hci_cb *cb) { BT_DBG("%p name %s", cb, cb->name); - mutex_lock(&hci_cb_list_lock); - list_del(&cb->list); - mutex_unlock(&hci_cb_list_lock); + list_del_rcu(&cb->list); + synchronize_rcu(); return 0; } diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c index 8ed818254dc8..eccdaa16cba0 100644 --- a/net/bluetooth/iso.c +++ b/net/bluetooth/iso.c @@ -2154,6 +2154,11 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) return HCI_LM_ACCEPT; } +static bool iso_match(struct hci_conn *hcon) +{ + return hcon->type == ISO_LINK || hcon->type == LE_LINK; +} + static void iso_connect_cfm(struct hci_conn *hcon, __u8 status) { if (hcon->type != ISO_LINK) { @@ -2335,6 +2340,7 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags) static struct hci_cb iso_cb = { .name = "ISO", + .match = iso_match, .connect_cfm = iso_connect_cfm, .disconn_cfm = iso_disconn_cfm, }; diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 6544c1ed7143..27b4c4a2ba1f 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -7217,6 +7217,11 @@ static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c, return NULL; } +static bool l2cap_match(struct hci_conn *hcon) +{ + return hcon->type == ACL_LINK || hcon->type == LE_LINK; +} + static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) { struct hci_dev *hdev = hcon->hdev; @@ -7224,9 +7229,6 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) struct l2cap_chan *pchan; u8 dst_type; - if (hcon->type != ACL_LINK && hcon->type != LE_LINK) - return; - BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status); if (status) { @@ -7291,9 +7293,6 @@ int l2cap_disconn_ind(struct hci_conn *hcon) static void l2cap_disconn_cfm(struct hci_conn *hcon, u8 reason) { - if (hcon->type != ACL_LINK && hcon->type != LE_LINK) - return; - BT_DBG("hcon %p reason %d", hcon, reason); l2cap_conn_del(hcon, bt_to_errno(reason)); @@ -7572,6 +7571,7 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags) static struct hci_cb l2cap_cb = { .name = "L2CAP", + .match = l2cap_match, .connect_cfm = l2cap_connect_cfm, .disconn_cfm = l2cap_disconn_cfm, .security_cfm = l2cap_security_cfm, diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c index ad5177e3a69b..4c56ca5a216c 100644 --- a/net/bluetooth/rfcomm/core.c +++ b/net/bluetooth/rfcomm/core.c @@ -2134,6 +2134,11 @@ static int rfcomm_run(void *unused) return 0; } +static bool rfcomm_match(struct hci_conn *hcon) +{ + return hcon->type == ACL_LINK; +} + static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt) { struct rfcomm_session *s; @@ -2180,6 +2185,7 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt) static struct hci_cb rfcomm_cb = { .name = "RFCOMM", + .match = rfcomm_match, .security_cfm = rfcomm_security_cfm }; diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index 7eb8d3e04ec4..40c4957cfc0b 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -1397,11 +1397,13 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) return lm; } +static bool sco_match(struct hci_conn *hcon) +{ + return hcon->type == SCO_LINK || hcon->type == ESCO_LINK; +} + static void sco_connect_cfm(struct hci_conn *hcon, __u8 status) { - if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK) - return; - BT_DBG("hcon %p bdaddr %pMR status %u", hcon, &hcon->dst, status); if (!status) { @@ -1416,9 +1418,6 @@ static void sco_connect_cfm(struct hci_conn *hcon, __u8 status) static void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason) { - if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK) - return; - BT_DBG("hcon %p reason %d", hcon, reason); sco_conn_del(hcon, bt_to_errno(reason)); @@ -1444,6 +1443,7 @@ void sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb) static struct hci_cb sco_cb = { .name = "SCO", + .match = sco_match, .connect_cfm = sco_connect_cfm, .disconn_cfm = sco_disconn_cfm, };