Skip to content

Commit bef3334

Browse files
Vudentzgregkh
Vudentz
authored and
gregkh
committed
Bluetooth: hci_core: Fix sleeping function called from invalid context
[ Upstream commit 4d94f05 ] 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]> Signed-off-by: Sasha Levin <[email protected]>
1 parent d8ecb24 commit bef3334

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
@@ -800,7 +800,6 @@ struct hci_conn_params {
800800
extern struct list_head hci_dev_list;
801801
extern struct list_head hci_cb_list;
802802
extern rwlock_t hci_dev_list_lock;
803-
extern struct mutex hci_cb_list_lock;
804803

805804
#define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags)
806805
#define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags)
@@ -1949,68 +1948,103 @@ struct hci_cb {
19491948

19501949
char *name;
19511950

1951+
bool (*match) (struct hci_conn *conn);
19521952
void (*connect_cfm) (struct hci_conn *conn, __u8 status);
19531953
void (*disconn_cfm) (struct hci_conn *conn, __u8 status);
19541954
void (*security_cfm) (struct hci_conn *conn, __u8 status,
1955-
__u8 encrypt);
1955+
__u8 encrypt);
19561956
void (*key_change_cfm) (struct hci_conn *conn, __u8 status);
19571957
void (*role_switch_cfm) (struct hci_conn *conn, __u8 status, __u8 role);
19581958
};
19591959

1960+
static inline void hci_cb_lookup(struct hci_conn *conn, struct list_head *list)
1961+
{
1962+
struct hci_cb *cb, *cpy;
1963+
1964+
rcu_read_lock();
1965+
list_for_each_entry_rcu(cb, &hci_cb_list, list) {
1966+
if (cb->match && cb->match(conn)) {
1967+
cpy = kmalloc(sizeof(*cpy), GFP_ATOMIC);
1968+
if (!cpy)
1969+
break;
1970+
1971+
*cpy = *cb;
1972+
INIT_LIST_HEAD(&cpy->list);
1973+
list_add_rcu(&cpy->list, list);
1974+
}
1975+
}
1976+
rcu_read_unlock();
1977+
}
1978+
19601979
static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status)
19611980
{
1962-
struct hci_cb *cb;
1981+
struct list_head list;
1982+
struct hci_cb *cb, *tmp;
1983+
1984+
INIT_LIST_HEAD(&list);
1985+
hci_cb_lookup(conn, &list);
19631986

1964-
mutex_lock(&hci_cb_list_lock);
1965-
list_for_each_entry(cb, &hci_cb_list, list) {
1987+
list_for_each_entry_safe(cb, tmp, &list, list) {
19661988
if (cb->connect_cfm)
19671989
cb->connect_cfm(conn, status);
1990+
kfree(cb);
19681991
}
1969-
mutex_unlock(&hci_cb_list_lock);
19701992

19711993
if (conn->connect_cfm_cb)
19721994
conn->connect_cfm_cb(conn, status);
19731995
}
19741996

19751997
static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason)
19761998
{
1977-
struct hci_cb *cb;
1999+
struct list_head list;
2000+
struct hci_cb *cb, *tmp;
2001+
2002+
INIT_LIST_HEAD(&list);
2003+
hci_cb_lookup(conn, &list);
19782004

1979-
mutex_lock(&hci_cb_list_lock);
1980-
list_for_each_entry(cb, &hci_cb_list, list) {
2005+
list_for_each_entry_safe(cb, tmp, &list, list) {
19812006
if (cb->disconn_cfm)
19822007
cb->disconn_cfm(conn, reason);
2008+
kfree(cb);
19832009
}
1984-
mutex_unlock(&hci_cb_list_lock);
19852010

19862011
if (conn->disconn_cfm_cb)
19872012
conn->disconn_cfm_cb(conn, reason);
19882013
}
19892014

1990-
static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
2015+
static inline void hci_security_cfm(struct hci_conn *conn, __u8 status,
2016+
__u8 encrypt)
19912017
{
1992-
struct hci_cb *cb;
1993-
__u8 encrypt;
1994-
1995-
if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags))
1996-
return;
2018+
struct list_head list;
2019+
struct hci_cb *cb, *tmp;
19972020

1998-
encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00;
2021+
INIT_LIST_HEAD(&list);
2022+
hci_cb_lookup(conn, &list);
19992023

2000-
mutex_lock(&hci_cb_list_lock);
2001-
list_for_each_entry(cb, &hci_cb_list, list) {
2024+
list_for_each_entry_safe(cb, tmp, &list, list) {
20022025
if (cb->security_cfm)
20032026
cb->security_cfm(conn, status, encrypt);
2027+
kfree(cb);
20042028
}
2005-
mutex_unlock(&hci_cb_list_lock);
20062029

20072030
if (conn->security_cfm_cb)
20082031
conn->security_cfm_cb(conn, status);
20092032
}
20102033

2034+
static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
2035+
{
2036+
__u8 encrypt;
2037+
2038+
if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags))
2039+
return;
2040+
2041+
encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00;
2042+
2043+
hci_security_cfm(conn, status, encrypt);
2044+
}
2045+
20112046
static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
20122047
{
2013-
struct hci_cb *cb;
20142048
__u8 encrypt;
20152049

20162050
if (conn->state == BT_CONFIG) {
@@ -2037,40 +2071,38 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
20372071
conn->sec_level = conn->pending_sec_level;
20382072
}
20392073

2040-
mutex_lock(&hci_cb_list_lock);
2041-
list_for_each_entry(cb, &hci_cb_list, list) {
2042-
if (cb->security_cfm)
2043-
cb->security_cfm(conn, status, encrypt);
2044-
}
2045-
mutex_unlock(&hci_cb_list_lock);
2046-
2047-
if (conn->security_cfm_cb)
2048-
conn->security_cfm_cb(conn, status);
2074+
hci_security_cfm(conn, status, encrypt);
20492075
}
20502076

20512077
static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
20522078
{
2053-
struct hci_cb *cb;
2079+
struct list_head list;
2080+
struct hci_cb *cb, *tmp;
2081+
2082+
INIT_LIST_HEAD(&list);
2083+
hci_cb_lookup(conn, &list);
20542084

2055-
mutex_lock(&hci_cb_list_lock);
2056-
list_for_each_entry(cb, &hci_cb_list, list) {
2085+
list_for_each_entry_safe(cb, tmp, &list, list) {
20572086
if (cb->key_change_cfm)
20582087
cb->key_change_cfm(conn, status);
2088+
kfree(cb);
20592089
}
2060-
mutex_unlock(&hci_cb_list_lock);
20612090
}
20622091

20632092
static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
20642093
__u8 role)
20652094
{
2066-
struct hci_cb *cb;
2095+
struct list_head list;
2096+
struct hci_cb *cb, *tmp;
2097+
2098+
INIT_LIST_HEAD(&list);
2099+
hci_cb_lookup(conn, &list);
20672100

2068-
mutex_lock(&hci_cb_list_lock);
2069-
list_for_each_entry(cb, &hci_cb_list, list) {
2101+
list_for_each_entry_safe(cb, tmp, &list, list) {
20702102
if (cb->role_switch_cfm)
20712103
cb->role_switch_cfm(conn, status, role);
2104+
kfree(cb);
20722105
}
2073-
mutex_unlock(&hci_cb_list_lock);
20742106
}
20752107

20762108
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
@@ -58,7 +58,6 @@ DEFINE_RWLOCK(hci_dev_list_lock);
5858

5959
/* HCI callback list */
6060
LIST_HEAD(hci_cb_list);
61-
DEFINE_MUTEX(hci_cb_list_lock);
6261

6362
/* HCI ID Numbering */
6463
static DEFINE_IDA(hci_index_ida);
@@ -2957,9 +2956,7 @@ int hci_register_cb(struct hci_cb *cb)
29572956
{
29582957
BT_DBG("%p name %s", cb, cb->name);
29592958

2960-
mutex_lock(&hci_cb_list_lock);
2961-
list_add_tail(&cb->list, &hci_cb_list);
2962-
mutex_unlock(&hci_cb_list_lock);
2959+
list_add_tail_rcu(&cb->list, &hci_cb_list);
29632960

29642961
return 0;
29652962
}
@@ -2969,9 +2966,8 @@ int hci_unregister_cb(struct hci_cb *cb)
29692966
{
29702967
BT_DBG("%p name %s", cb, cb->name);
29712968

2972-
mutex_lock(&hci_cb_list_lock);
2973-
list_del(&cb->list);
2974-
mutex_unlock(&hci_cb_list_lock);
2969+
list_del_rcu(&cb->list);
2970+
synchronize_rcu();
29752971

29762972
return 0;
29772973
}

‎net/bluetooth/iso.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1929,6 +1929,11 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
19291929
return lm;
19301930
}
19311931

1932+
static bool iso_match(struct hci_conn *hcon)
1933+
{
1934+
return hcon->type == ISO_LINK || hcon->type == LE_LINK;
1935+
}
1936+
19321937
static void iso_connect_cfm(struct hci_conn *hcon, __u8 status)
19331938
{
19341939
if (hcon->type != ISO_LINK) {
@@ -2110,6 +2115,7 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
21102115

21112116
static struct hci_cb iso_cb = {
21122117
.name = "ISO",
2118+
.match = iso_match,
21132119
.connect_cfm = iso_connect_cfm,
21142120
.disconn_cfm = iso_disconn_cfm,
21152121
};

‎net/bluetooth/l2cap_core.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7223,16 +7223,18 @@ static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c,
72237223
return NULL;
72247224
}
72257225

7226+
static bool l2cap_match(struct hci_conn *hcon)
7227+
{
7228+
return hcon->type == ACL_LINK || hcon->type == LE_LINK;
7229+
}
7230+
72267231
static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
72277232
{
72287233
struct hci_dev *hdev = hcon->hdev;
72297234
struct l2cap_conn *conn;
72307235
struct l2cap_chan *pchan;
72317236
u8 dst_type;
72327237

7233-
if (hcon->type != ACL_LINK && hcon->type != LE_LINK)
7234-
return;
7235-
72367238
BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
72377239

72387240
if (status) {
@@ -7297,9 +7299,6 @@ int l2cap_disconn_ind(struct hci_conn *hcon)
72977299

72987300
static void l2cap_disconn_cfm(struct hci_conn *hcon, u8 reason)
72997301
{
7300-
if (hcon->type != ACL_LINK && hcon->type != LE_LINK)
7301-
return;
7302-
73037302
BT_DBG("hcon %p reason %d", hcon, reason);
73047303

73057304
l2cap_conn_del(hcon, bt_to_errno(reason));
@@ -7578,6 +7577,7 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
75787577

75797578
static struct hci_cb l2cap_cb = {
75807579
.name = "L2CAP",
7580+
.match = l2cap_match,
75817581
.connect_cfm = l2cap_connect_cfm,
75827582
.disconn_cfm = l2cap_disconn_cfm,
75837583
.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
@@ -1353,11 +1353,13 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags)
13531353
return lm;
13541354
}
13551355

1356-
static void sco_connect_cfm(struct hci_conn *hcon, __u8 status)
1356+
static bool sco_match(struct hci_conn *hcon)
13571357
{
1358-
if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK)
1359-
return;
1358+
return hcon->type == SCO_LINK || hcon->type == ESCO_LINK;
1359+
}
13601360

1361+
static void sco_connect_cfm(struct hci_conn *hcon, __u8 status)
1362+
{
13611363
BT_DBG("hcon %p bdaddr %pMR status %u", hcon, &hcon->dst, status);
13621364

13631365
if (!status) {
@@ -1372,9 +1374,6 @@ static void sco_connect_cfm(struct hci_conn *hcon, __u8 status)
13721374

13731375
static void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason)
13741376
{
1375-
if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK)
1376-
return;
1377-
13781377
BT_DBG("hcon %p reason %d", hcon, reason);
13791378

13801379
sco_conn_del(hcon, bt_to_errno(reason));
@@ -1400,6 +1399,7 @@ void sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb)
14001399

14011400
static struct hci_cb sco_cb = {
14021401
.name = "SCO",
1402+
.match = sco_match,
14031403
.connect_cfm = sco_connect_cfm,
14041404
.disconn_cfm = sco_disconn_cfm,
14051405
};

0 commit comments

Comments
 (0)