diff mbox series

[v2] Bluetooth: hci_core: Fix sleeping function called from invalid context

Message ID 20241204165849.392564-2-luiz.dentz@gmail.com (mailing list archive)
State New
Headers show
Series [v2] Bluetooth: hci_core: Fix sleeping function called from invalid context | expand

Checks

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

Commit Message

Luiz Augusto von Dentz Dec. 4, 2024, 4:58 p.m. UTC
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>

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(-)

Comments

bluez.test.bot@gmail.com Dec. 4, 2024, 5:31 p.m. UTC | #1
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
Pauli Virtanen Dec. 4, 2024, 6:44 p.m. UTC | #2
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,
>  };
Luiz Augusto von Dentz Dec. 4, 2024, 6:59 p.m. UTC | #3
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,
> >  };
>
diff mbox series

Patch

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,
 };