From: syzbot <syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com>
To: linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com
Subject: Re: [syzbot] Re: [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context
Date: Tue, 03 Dec 2024 14:30:12 -0800 [thread overview]
Message-ID: <674f8674.050a0220.17bd51.0053.GAE@google.com> (raw)
In-Reply-To: <000000000000dd84650615800e67@google.com>
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.
***
Subject: Re: [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context
Author: luiz.dentz@gmail.com
#syz test
On Tue, Dec 3, 2024 at 4:15 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> 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>
>
> 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 | 89 ++++++++++++++++++++++----------
> net/bluetooth/hci_core.c | 9 ++--
> 2 files changed, 65 insertions(+), 33 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ea798f07c5a2..95f11f04e24a 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)
> @@ -2029,12 +2028,18 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status)
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> - list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->connect_cfm)
> - cb->connect_cfm(conn, status);
> + rcu_read_lock();
> + list_for_each_entry_rcu(cb, &hci_cb_list, list) {
> + if (cb->connect_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.connect_cfm(conn, status);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->connect_cfm_cb)
> conn->connect_cfm_cb(conn, status);
> @@ -2044,12 +2049,18 @@ static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason)
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->disconn_cfm)
> - cb->disconn_cfm(conn, reason);
> + if (cb->disconn_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.disconn_cfm(conn, reason);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->disconn_cfm_cb)
> conn->disconn_cfm_cb(conn, reason);
> @@ -2065,12 +2076,18 @@ 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);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->security_cfm)
> - cb->security_cfm(conn, status, encrypt);
> + if (cb->security_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.security_cfm(conn, status, encrypt);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->security_cfm_cb)
> conn->security_cfm_cb(conn, status);
> @@ -2105,12 +2122,18 @@ 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);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->security_cfm)
> - cb->security_cfm(conn, status, encrypt);
> + if (cb->security_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.security_cfm(conn, status, encrypt);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->security_cfm_cb)
> conn->security_cfm_cb(conn, status);
> @@ -2120,12 +2143,18 @@ static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->key_change_cfm)
> - cb->key_change_cfm(conn, status);
> + if (cb->key_change_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.key_change_cfm(conn, status);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
> }
>
> static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
> @@ -2133,12 +2162,18 @@ static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> - list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->role_switch_cfm)
> - cb->role_switch_cfm(conn, status, role);
> + rcu_read_lock();
> + list_for_each_entry_rcu(cb, &hci_cb_list, list) {
> + if (cb->role_switch_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.role_switch_cfm(conn, status, role);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
> }
>
> 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;
> }
> --
> 2.47.1
>
--
Luiz Augusto von Dentz
next prev parent reply other threads:[~2024-12-03 22:30 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-07 11:33 [syzbot] [bluetooth?] BUG: sleeping function called from invalid context in hci_le_create_big_complete_evt syzbot
2024-04-07 12:16 ` Hillf Danton
2024-04-07 12:50 ` syzbot
2024-04-07 13:59 ` syzbot
2024-04-07 23:14 ` Hillf Danton
2024-04-08 1:50 ` syzbot
2024-12-03 22:30 ` syzbot [this message]
2024-12-05 15:13 ` [syzbot] Re: [PATCH v2] Bluetooth: hci_core: Fix sleeping function called from invalid context syzbot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=674f8674.050a0220.17bd51.0053.GAE@google.com \
--to=syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=syzkaller-bugs@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.