Skip to content

Commit 4d94f05

Browse files
VudentzVudentz
committed
Bluetooth: hci_core: Fix sleeping function called from invalid context
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: [email protected] Tested-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614 Signed-off-by: Luiz Augusto von Dentz <[email protected]>
1 parent 3e643e4 commit 4d94f05

File tree

6 files changed

+97
-57
lines changed

6 files changed

+97
-57
lines changed

‎include/net/bluetooth/hci_core.h

Lines changed: 70 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,6 @@ struct hci_conn_params {
804804
extern struct list_head hci_dev_list;
805805
extern struct list_head hci_cb_list;
806806
extern rwlock_t hci_dev_list_lock;
807-
extern struct mutex hci_cb_list_lock;
808807

809808
#define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags)
810809
#define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags)
@@ -2017,68 +2016,103 @@ struct hci_cb {
20172016

20182017
char *name;
20192018

2019+
bool (*match) (struct hci_conn *conn);
20202020
void (*connect_cfm) (struct hci_conn *conn, __u8 status);
20212021
void (*disconn_cfm) (struct hci_conn *conn, __u8 status);
20222022
void (*security_cfm) (struct hci_conn *conn, __u8 status,
2023-
__u8 encrypt);
2023+
__u8 encrypt);
20242024
void (*key_change_cfm) (struct hci_conn *conn, __u8 status);
20252025
void (*role_switch_cfm) (struct hci_conn *conn, __u8 status, __u8 role);
20262026
};
20272027

2028+
static inline void hci_cb_lookup(struct hci_conn *conn, struct list_head *list)
2029+
{
2030+
struct hci_cb *cb, *cpy;
2031+
2032+
rcu_read_lock();
2033+
list_for_each_entry_rcu(cb, &hci_cb_list, list) {
2034+
if (cb->match && cb->match(conn)) {
2035+
cpy = kmalloc(sizeof(*cpy), GFP_ATOMIC);
2036+
if (!cpy)
2037+
break;
2038+
2039+
*cpy = *cb;
2040+
INIT_LIST_HEAD(&cpy->list);
2041+
list_add_rcu(&cpy->list, list);
2042+
}
2043+
}
2044+
rcu_read_unlock();
2045+
}
2046+
20282047
static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status)
20292048
{
2030-
struct hci_cb *cb;
2049+
struct list_head list;
2050+
struct hci_cb *cb, *tmp;
2051+
2052+
INIT_LIST_HEAD(&list);
2053+
hci_cb_lookup(conn, &list);
20312054

2032-
mutex_lock(&hci_cb_list_lock);
2033-
list_for_each_entry(cb, &hci_cb_list, list) {
2055+
list_for_each_entry_safe(cb, tmp, &list, list) {
20342056
if (cb->connect_cfm)
20352057
cb->connect_cfm(conn, status);
2058+
kfree(cb);
20362059
}
2037-
mutex_unlock(&hci_cb_list_lock);
20382060

20392061
if (conn->connect_cfm_cb)
20402062
conn->connect_cfm_cb(conn, status);
20412063
}
20422064

20432065
static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason)
20442066
{
2045-
struct hci_cb *cb;
2067+
struct list_head list;
2068+
struct hci_cb *cb, *tmp;
2069+
2070+
INIT_LIST_HEAD(&list);
2071+
hci_cb_lookup(conn, &list);
20462072

2047-
mutex_lock(&hci_cb_list_lock);
2048-
list_for_each_entry(cb, &hci_cb_list, list) {
2073+
list_for_each_entry_safe(cb, tmp, &list, list) {
20492074
if (cb->disconn_cfm)
20502075
cb->disconn_cfm(conn, reason);
2076+
kfree(cb);
20512077
}
2052-
mutex_unlock(&hci_cb_list_lock);
20532078

20542079
if (conn->disconn_cfm_cb)
20552080
conn->disconn_cfm_cb(conn, reason);
20562081
}
20572082

2058-
static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
2083+
static inline void hci_security_cfm(struct hci_conn *conn, __u8 status,
2084+
__u8 encrypt)
20592085
{
2060-
struct hci_cb *cb;
2061-
__u8 encrypt;
2062-
2063-
if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags))
2064-
return;
2086+
struct list_head list;
2087+
struct hci_cb *cb, *tmp;
20652088

2066-
encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00;
2089+
INIT_LIST_HEAD(&list);
2090+
hci_cb_lookup(conn, &list);
20672091

2068-
mutex_lock(&hci_cb_list_lock);
2069-
list_for_each_entry(cb, &hci_cb_list, list) {
2092+
list_for_each_entry_safe(cb, tmp, &list, list) {
20702093
if (cb->security_cfm)
20712094
cb->security_cfm(conn, status, encrypt);
2095+
kfree(cb);
20722096
}
2073-
mutex_unlock(&hci_cb_list_lock);
20742097

20752098
if (conn->security_cfm_cb)
20762099
conn->security_cfm_cb(conn, status);
20772100
}
20782101

2102+
static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
2103+
{
2104+
__u8 encrypt;
2105+
2106+
if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags))
2107+
return;
2108+
2109+
encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00;
2110+
2111+
hci_security_cfm(conn, status, encrypt);
2112+
}
2113+
20792114
static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
20802115
{
2081-
struct hci_cb *cb;
20822116
__u8 encrypt;
20832117

20842118
if (conn->state == BT_CONFIG) {
@@ -2105,40 +2139,38 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
21052139
conn->sec_level = conn->pending_sec_level;
21062140
}
21072141

2108-
mutex_lock(&hci_cb_list_lock);
2109-
list_for_each_entry(cb, &hci_cb_list, list) {
2110-
if (cb->security_cfm)
2111-
cb->security_cfm(conn, status, encrypt);
2112-
}
2113-
mutex_unlock(&hci_cb_list_lock);
2114-
2115-
if (conn->security_cfm_cb)
2116-
conn->security_cfm_cb(conn, status);
2142+
hci_security_cfm(conn, status, encrypt);
21172143
}
21182144

21192145
static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
21202146
{
2121-
struct hci_cb *cb;
2147+
struct list_head list;
2148+
struct hci_cb *cb, *tmp;
2149+
2150+
INIT_LIST_HEAD(&list);
2151+
hci_cb_lookup(conn, &list);
21222152

2123-
mutex_lock(&hci_cb_list_lock);
2124-
list_for_each_entry(cb, &hci_cb_list, list) {
2153+
list_for_each_entry_safe(cb, tmp, &list, list) {
21252154
if (cb->key_change_cfm)
21262155
cb->key_change_cfm(conn, status);
2156+
kfree(cb);
21272157
}
2128-
mutex_unlock(&hci_cb_list_lock);
21292158
}
21302159

21312160
static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
21322161
__u8 role)
21332162
{
2134-
struct hci_cb *cb;
2163+
struct list_head list;
2164+
struct hci_cb *cb, *tmp;
2165+
2166+
INIT_LIST_HEAD(&list);
2167+
hci_cb_lookup(conn, &list);
21352168

2136-
mutex_lock(&hci_cb_list_lock);
2137-
list_for_each_entry(cb, &hci_cb_list, list) {
2169+
list_for_each_entry_safe(cb, tmp, &list, list) {
21382170
if (cb->role_switch_cfm)
21392171
cb->role_switch_cfm(conn, status, role);
2172+
kfree(cb);
21402173
}
2141-
mutex_unlock(&hci_cb_list_lock);
21422174
}
21432175

21442176
static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)

‎net/bluetooth/hci_core.c

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ DEFINE_RWLOCK(hci_dev_list_lock);
5757

5858
/* HCI callback list */
5959
LIST_HEAD(hci_cb_list);
60-
DEFINE_MUTEX(hci_cb_list_lock);
6160

6261
/* HCI ID Numbering */
6362
static DEFINE_IDA(hci_index_ida);
@@ -2993,9 +2992,7 @@ int hci_register_cb(struct hci_cb *cb)
29932992
{
29942993
BT_DBG("%p name %s", cb, cb->name);
29952994

2996-
mutex_lock(&hci_cb_list_lock);
2997-
list_add_tail(&cb->list, &hci_cb_list);
2998-
mutex_unlock(&hci_cb_list_lock);
2995+
list_add_tail_rcu(&cb->list, &hci_cb_list);
29992996

30002997
return 0;
30012998
}
@@ -3005,9 +3002,8 @@ int hci_unregister_cb(struct hci_cb *cb)
30053002
{
30063003
BT_DBG("%p name %s", cb, cb->name);
30073004

3008-
mutex_lock(&hci_cb_list_lock);
3009-
list_del(&cb->list);
3010-
mutex_unlock(&hci_cb_list_lock);
3005+
list_del_rcu(&cb->list);
3006+
synchronize_rcu();
30113007

30123008
return 0;
30133009
}

‎net/bluetooth/iso.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2118,6 +2118,11 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
21182118
return HCI_LM_ACCEPT;
21192119
}
21202120

2121+
static bool iso_match(struct hci_conn *hcon)
2122+
{
2123+
return hcon->type == ISO_LINK || hcon->type == LE_LINK;
2124+
}
2125+
21212126
static void iso_connect_cfm(struct hci_conn *hcon, __u8 status)
21222127
{
21232128
if (hcon->type != ISO_LINK) {
@@ -2299,6 +2304,7 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
22992304

23002305
static struct hci_cb iso_cb = {
23012306
.name = "ISO",
2307+
.match = iso_match,
23022308
.connect_cfm = iso_connect_cfm,
23032309
.disconn_cfm = iso_disconn_cfm,
23042310
};

‎net/bluetooth/l2cap_core.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7217,16 +7217,18 @@ static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c,
72177217
return NULL;
72187218
}
72197219

7220+
static bool l2cap_match(struct hci_conn *hcon)
7221+
{
7222+
return hcon->type == ACL_LINK || hcon->type == LE_LINK;
7223+
}
7224+
72207225
static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
72217226
{
72227227
struct hci_dev *hdev = hcon->hdev;
72237228
struct l2cap_conn *conn;
72247229
struct l2cap_chan *pchan;
72257230
u8 dst_type;
72267231

7227-
if (hcon->type != ACL_LINK && hcon->type != LE_LINK)
7228-
return;
7229-
72307232
BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
72317233

72327234
if (status) {
@@ -7291,9 +7293,6 @@ int l2cap_disconn_ind(struct hci_conn *hcon)
72917293

72927294
static void l2cap_disconn_cfm(struct hci_conn *hcon, u8 reason)
72937295
{
7294-
if (hcon->type != ACL_LINK && hcon->type != LE_LINK)
7295-
return;
7296-
72977296
BT_DBG("hcon %p reason %d", hcon, reason);
72987297

72997298
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)
75727571

75737572
static struct hci_cb l2cap_cb = {
75747573
.name = "L2CAP",
7574+
.match = l2cap_match,
75757575
.connect_cfm = l2cap_connect_cfm,
75767576
.disconn_cfm = l2cap_disconn_cfm,
75777577
.security_cfm = l2cap_security_cfm,

‎net/bluetooth/rfcomm/core.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2134,6 +2134,11 @@ static int rfcomm_run(void *unused)
21342134
return 0;
21352135
}
21362136

2137+
static bool rfcomm_match(struct hci_conn *hcon)
2138+
{
2139+
return hcon->type == ACL_LINK;
2140+
}
2141+
21372142
static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt)
21382143
{
21392144
struct rfcomm_session *s;
@@ -2180,6 +2185,7 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt)
21802185

21812186
static struct hci_cb rfcomm_cb = {
21822187
.name = "RFCOMM",
2188+
.match = rfcomm_match,
21832189
.security_cfm = rfcomm_security_cfm
21842190
};
21852191

‎net/bluetooth/sco.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1397,11 +1397,13 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
13971397
return lm;
13981398
}
13991399

1400-
static void sco_connect_cfm(struct hci_conn *hcon, __u8 status)
1400+
static bool sco_match(struct hci_conn *hcon)
14011401
{
1402-
if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK)
1403-
return;
1402+
return hcon->type == SCO_LINK || hcon->type == ESCO_LINK;
1403+
}
14041404

1405+
static void sco_connect_cfm(struct hci_conn *hcon, __u8 status)
1406+
{
14051407
BT_DBG("hcon %p bdaddr %pMR status %u", hcon, &hcon->dst, status);
14061408

14071409
if (!status) {
@@ -1416,9 +1418,6 @@ static void sco_connect_cfm(struct hci_conn *hcon, __u8 status)
14161418

14171419
static void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason)
14181420
{
1419-
if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK)
1420-
return;
1421-
14221421
BT_DBG("hcon %p reason %d", hcon, reason);
14231422

14241423
sco_conn_del(hcon, bt_to_errno(reason));
@@ -1444,6 +1443,7 @@ void sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb)
14441443

14451444
static struct hci_cb sco_cb = {
14461445
.name = "SCO",
1446+
.match = sco_match,
14471447
.connect_cfm = sco_connect_cfm,
14481448
.disconn_cfm = sco_disconn_cfm,
14491449
};

0 commit comments

Comments
 (0)