Bluetooth: ISO-related concurrency fixes
Subsystems (2)
PATCH v3 2/3 Bluetooth: hci_event: call disconnect callback before deleting conn
In reply to Pauli VirtanenIn hci_cs_disconnect, we do hci_conn_del even if disconnection failed.
ISO, L2CAP and SCO connections refer to the hci_conn without hci_conn_get, so disconn_cfm must be called so they can clean up their conn, otherwise use-after-free occurs.
ISO:
==========================================================
iso_sock_connect:880: sk 00000000eabd6557 iso_connect_cis:356: 70:1a:b8:98:ff:a2 -> 28:3d:c2:4a:7e:da
...
iso_conn_add:140: hcon 000000001696f1fd conn 00000000b6251073 hci_dev_put:1487: hci0 orig refcnt 17 __iso_chan_add:214: conn 00000000b6251073 iso_sock_clear_timer:117: sock 00000000eabd6557 state 3
...
hci_rx_work:4085: hci0 Event packet hci_event_packet:7601: hci0: event 0x0f hci_cmd_status_evt:4346: hci0: opcode 0x0406 hci_cs_disconnect:2760: hci0: status 0x0c hci_sent_cmd_data:3107: hci0 opcode 0x0406 hci_conn_del:1151: hci0 hcon 000000001696f1fd handle 2560 hci_conn_unlink:1102: hci0: hcon 000000001696f1fd hci_conn_drop:1451: hcon 00000000d8521aaf orig refcnt 2 hci_chan_list_flush:2780: hcon 000000001696f1fd hci_dev_put:1487: hci0 orig refcnt 21
hci_dev_put:1487: hci0 orig refcnt 20 hci_req_cmd_complete:3978: opcode 0x0406 status 0x0c ... <no iso_* activity on sk/conn> ... iso_sock_sendmsg:1098: sock 00000000dea5e2e0, sk 00000000eabd6557
BUG: kernel NULL pointer dereference, address: 0000000000000668 PGD 0 P4D 0 Oops: 0000 [#1] PREEMPT SMP PTI Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014 RIP: 0010:iso_sock_sendmsg (net/bluetooth/iso.c:1112) bluetooth ==========================================================
L2CAP:
==================================================================
hci_cmd_status_evt:4359: hci0: opcode 0x0406 hci_cs_disconnect:2760: hci0: status 0x0c hci_sent_cmd_data:3085: hci0 opcode 0x0406 hci_conn_del:1151: hci0 hcon ffff88800c999000 handle 3585 hci_conn_unlink:1102: hci0: hcon ffff88800c999000 hci_chan_list_flush:2780: hcon ffff88800c999000 hci_chan_del:2761: hci0 hcon ffff88800c999000 chan ffff888018ddd280
...
BUG: KASAN: slab-use-after-free in hci_send_acl+0x2d/0x540 [bluetooth] Read of size 8 at addr ffff888018ddd298 by task bluetoothd/1175 CPU: 0 PID: 1175 Comm: bluetoothd Tainted: G E 6.4.0-rc4+ #2 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0x5b/0x90 print_report+0xcf/0x670 ? __virt_addr_valid+0xf8/0x180 ? hci_send_acl+0x2d/0x540 [bluetooth] kasan_report+0xa8/0xe0 ? hci_send_acl+0x2d/0x540 [bluetooth] hci_send_acl+0x2d/0x540 [bluetooth] ? __pfx___lock_acquire+0x10/0x10 l2cap_chan_send+0x1fd/0x1300 [bluetooth] ? l2cap_sock_sendmsg+0xf2/0x170 [bluetooth] ? __pfx_l2cap_chan_send+0x10/0x10 [bluetooth] ? lock_release+0x1d5/0x3c0 ? mark_held_locks+0x1a/0x90 l2cap_sock_sendmsg+0x100/0x170 [bluetooth] sock_write_iter+0x275/0x280 ? __pfx_sock_write_iter+0x10/0x10 ? __pfx___lock_acquire+0x10/0x10 do_iter_readv_writev+0x176/0x220 ? __pfx_do_iter_readv_writev+0x10/0x10 ? find_held_lock+0x83/0xa0 ? selinux_file_permission+0x13e/0x210 do_iter_write+0xda/0x340 vfs_writev+0x1b4/0x400 ? __pfx_vfs_writev+0x10/0x10 ? __seccomp_filter+0x112/0x750 ? populate_seccomp_data+0x182/0x220 ? __fget_light+0xdf/0x100 ? do_writev+0x19d/0x210 do_writev+0x19d/0x210 ? __pfx_do_writev+0x10/0x10 ? mark_held_locks+0x1a/0x90 do_syscall_64+0x60/0x90 ? lockdep_hardirqs_on_prepare+0x149/0x210 ? do_syscall_64+0x6c/0x90 ? lockdep_hardirqs_on_prepare+0x149/0x210 entry_SYSCALL_64_after_hwframe+0x72/0xdc RIP: 0033:0x7ff45cb23e64 Code: 15 d1 1f 0d 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 80 3d 9d a7 0d 00 00 74 13 b8 14 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 48 83 ec 28 89 54 24 1c 48 89 RSP: 002b:00007fff21ae09b8 EFLAGS: 00000202 ORIG_RAX: 0000000000000014 RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007ff45cb23e64 RDX: 0000000000000001 RSI: 00007fff21ae0aa0 RDI: 0000000000000017 RBP: 00007fff21ae0aa0 R08: 000000000095a8a0 R09: 0000607000053f40 R10: 0000000000000001 R11: 0000000000000202 R12: 00007fff21ae0ac0 R13: 00000fffe435c150 R14: 00007fff21ae0a80 R15: 000060f000000040 </TASK> Allocated by task 771: kasan_save_stack+0x33/0x60 kasan_set_track+0x25/0x30 __kasan_kmalloc+0xaa/0xb0 hci_chan_create+0x67/0x1b0 [bluetooth] l2cap_conn_add.part.0+0x17/0x590 [bluetooth] l2cap_connect_cfm+0x266/0x6b0 [bluetooth] hci_le_remote_feat_complete_evt+0x167/0x310 [bluetooth] hci_event_packet+0x38d/0x800 [bluetooth] hci_rx_work+0x287/0xb20 [bluetooth] process_one_work+0x4f7/0x970 worker_thread+0x8f/0x620 kthread+0x17f/0x1c0 ret_from_fork+0x2c/0x50 Freed by task 771: kasan_save_stack+0x33/0x60 kasan_set_track+0x25/0x30 kasan_save_free_info+0x2e/0x50 ____kasan_slab_free+0x169/0x1c0 slab_free_freelist_hook+0x9e/0x1c0 __kmem_cache_free+0xc0/0x310 hci_chan_list_flush+0x46/0x90 [bluetooth] hci_conn_cleanup+0x7d/0x330 [bluetooth] hci_cs_disconnect+0x35d/0x530 [bluetooth] hci_cmd_status_evt+0xef/0x2b0 [bluetooth] hci_event_packet+0x38d/0x800 [bluetooth] hci_rx_work+0x287/0xb20 [bluetooth] process_one_work+0x4f7/0x970 worker_thread+0x8f/0x620 kthread+0x17f/0x1c0 ret_from_fork+0x2c/0x50 ==================================================================
---
Notes:
v3: no changes
v2: call disconn_cfm for all conn types, as L2CAP also crashes.
Correct commit in Fixes tag.
net/bluetooth/hci_event.c | 3 +++ 1 file changed, 3 insertions(+)
net/bluetooth/hci_event.c +3 -0
@@ -2784,6 +2784,9 @@ static void hci_cs_disconnect(struct hci_dev *hdev, u8 status)hci_enable_advertising (hdev); } +/* Inform sockets conn is gone before we delete it */ +hci_disconn_cfm (conn,HCI_ERROR_UNSPECIFIED ); + goto done; }
Signature · 1 line
2.41.0
v3 cover Bluetooth: ISO-related concurrency fixes
This series addresses some concurrency issues (NULL / GPF) in ISO sockets or related.
v3:
- Copy the pend_le_* lists, iterate over copy to make it simple.
- Rename to hci_pend_le_list_*
v2:
- Use RCU for the pend_le_* lists, avoid using hci_dev_lock.
- Always call disconn_cfm before hci_conn_del.
These were found while testing patches that make hci_le_set_cig_params check the validity of the configuration and return false if incorrect. This causes dropping of hci_conn just created, which apparently makes hitting race conditions easier.
The test setup was primitive
while true; do bluetoothctl power on; sleep 12; bluetoothctl power off; sleep 1.5; bluetoothctl power off; sleep 2.5; done; while true; do sudo systemctl restart bluetooth; sleep 110; done
while true; do systemctl --user restart pipewire wireplumber pipewire-pulse; sleep 91; done
while true; do paplay sample.flac & sleep 2; kill %1; sleep 0.7; done
and equivalent operations manually, on VM + connect to TWS earbuds. This eventually hit the NULL / GFP errors here, but they are hard to reproduce aside from the first one that appears in iso-tester.
Pauli Virtanen (3):
Bluetooth: use RCU for hci_conn_params and iterate safely in hci_sync
Bluetooth: hci_event: call disconnect callback before deleting conn
Bluetooth: ISO: fix iso_conn related locking and validity issues
include/net/bluetooth/hci_core.h | 5 ++ net/bluetooth/hci_conn.c | 10 +-- net/bluetooth/hci_core.c | 38 ++++++++-- net/bluetooth/hci_event.c | 15 ++-- net/bluetooth/hci_sync.c | 117 ++++++++++++++++++++++++++++--- net/bluetooth/iso.c | 53 ++++++++------ net/bluetooth/mgmt.c | 26 +++---- 7 files changed, 198 insertions(+), 66 deletions(-)
Signature · 1 line
2.41.0
PATCH v3 3/3 Bluetooth: ISO: fix iso_conn related locking and validity issues
sk->sk_state indicates whether iso_pi(sk)->conn is valid. Operations that check/update sk_state and access conn should hold lock_sock, otherwise they can race.
The order of taking locks is hci_dev_lock > lock_sock > iso_conn_lock, which is how it is in connect/disconnect_cfm -> iso_conn_del -> iso_chan_del.
Fix locking in iso_connect_cis/bis and sendmsg/recvmsg to take lock_sock around updating sk_state and conn.
iso_conn_del must not occur during iso_connect_cis/bis, as it frees the iso_conn. Hold hdev->lock longer to prevent that.
This should not reintroduce the issue fixed in commit 241f51931c35 ("Bluetooth: ISO: Avoid circular locking dependency"), since the we acquire locks in order. We retain the fix in iso_sock_connect to release lock_sock before iso_connect_* acquires hdev->lock.
Similarly for commit 6a5ad251b7cd ("Bluetooth: ISO: Fix possible circular locking dependency"). We retain the fix in iso_conn_ready to not acquire iso_conn_lock before lock_sock.
iso_conn_add shall return iso_conn with valid hcon. Make it so also when reusing an old CIS connection waiting for disconnect timeout (see __iso_sock_close where conn->hcon is set to NULL).
Trace with iso_conn_del after iso_chan_add in iso_connect_cis:
===============================================================
iso_sock_create:771: sock 00000000be9b69b7
iso_sock_init:693: sk 000000004dff667e
iso_sock_bind:827: sk 000000004dff667e 70:1a:b8:98:ff:a2 type 1 iso_sock_setsockopt:1289: sk 000000004dff667e iso_sock_setsockopt:1289: sk 000000004dff667e iso_sock_setsockopt:1289: sk 000000004dff667e iso_sock_connect:875: sk 000000004dff667e
iso_connect_cis:353: 70:1a:b8:98:ff:a2 -> 28:3d:c2:4a:7e:da hci_get_route:1199: 70:1a:b8:98:ff:a2 -> 28:3d:c2:4a:7e:da hci_conn_add:1005: hci0 dst 28:3d:c2:4a:7e:da
iso_conn_add:140: hcon 000000007b65d182 conn 00000000daf8625e __iso_chan_add:214: conn 00000000daf8625e
iso_connect_cfm:1700: hcon 000000007b65d182 bdaddr 28:3d:c2:4a:7e:da status 12 iso_conn_del:187: hcon 000000007b65d182 conn 00000000daf8625e, err 16 iso_sock_clear_timer:117: sock 000000004dff667e state 3
<Note: sk_state is BT_BOUND (3), so iso_connect_cis is still running at this point>
iso_chan_del:153: sk 000000004dff667e, conn 00000000daf8625e, err 16 hci_conn_del:1151: hci0 hcon 000000007b65d182 handle 65535 hci_conn_unlink:1102: hci0: hcon 000000007b65d182 hci_chan_list_flush:2780: hcon 000000007b65d182 iso_sock_getsockopt:1376: sk 000000004dff667e iso_sock_getname:1070: sock 00000000be9b69b7, sk 000000004dff667e iso_sock_getname:1070: sock 00000000be9b69b7, sk 000000004dff667e iso_sock_getsockopt:1376: sk 000000004dff667e iso_sock_getname:1070: sock 00000000be9b69b7, sk 000000004dff667e iso_sock_getname:1070: sock 00000000be9b69b7, sk 000000004dff667e iso_sock_shutdown:1434: sock 00000000be9b69b7, sk 000000004dff667e, how 1 __iso_sock_close:632: sk 000000004dff667e state 5 socket 00000000be9b69b7
<Note: sk_state is BT_CONNECT (5), even though iso_chan_del sets BT_CLOSED (6). Only iso_connect_cis sets it to BT_CONNECT, so it must be that iso_chan_del occurred between iso_chan_add and end of iso_connect_cis.>
BUG: kernel NULL pointer dereference, address: 0000000000000000 PGD 8000000006467067 P4D 8000000006467067 PUD 3f5f067 PMD 0 Oops: 0000 [#1] PREEMPT SMP PTI Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014 RIP: 0010:__iso_sock_close (net/bluetooth/iso.c:664) bluetooth ===============================================================
Trace with iso_conn_del before iso_chan_add in iso_connect_cis:
===============================================================
iso_connect_cis:356: 70:1a:b8:98:ff:a2 -> 28:3d:c2:4a:7e:da
...
iso_conn_add:140: hcon 0000000093bc551f conn 00000000768ae504 hci_dev_put:1487: hci0 orig refcnt 21
hci_event_packet:7607: hci0: event 0x0e hci_cmd_complete_evt:4231: hci0: opcode 0x2062 hci_cc_le_set_cig_params:3846: hci0: status 0x07 hci_sent_cmd_data:3107: hci0 opcode 0x2062
iso_connect_cfm:1703: hcon 0000000093bc551f bdaddr 28:3d:c2:4a:7e:da status 7 iso_conn_del:187: hcon 0000000093bc551f conn 00000000768ae504, err 12 hci_conn_del:1151: hci0 hcon 0000000093bc551f handle 65535 hci_conn_unlink:1102: hci0: hcon 0000000093bc551f hci_chan_list_flush:2780: hcon 0000000093bc551f __iso_chan_add:214: conn 00000000768ae504
<Note: this conn was already freed in iso_conn_del above>
iso_sock_clear_timer:117: sock 0000000098323f95 state 3
general protection fault, probably for non-canonical address 0x30b29c630930aec8: 0000 [#1] PREEMPT SMP PTI CPU: 1 PID: 1920 Comm: bluetoothd Tainted: G E 6.3.0-rc7+ #4 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014 RIP: 0010:detach_if_pending+0x28/0xd0 Code: 90 90 0f 1f 44 00 00 48 8b 47 08 48 85 c0 0f 84 ad 00 00 00 55 89 d5 53 48 83 3f 00 48 89 fb 74 7d 66 90 48 8b 03 48 8b 53 08 <> RSP: 0018:ffffb90841a67d08 EFLAGS: 00010007 RAX: 0000000000000000 RBX: ffff9141bd5061b8 RCX: 0000000000000000 RDX: 30b29c630930aec8 RSI: ffff9141fdd21e80 RDI: ffff9141bd5061b8 RBP: 0000000000000001 R08: 0000000000000000 R09: ffffb90841a67b88 R10: 0000000000000003 R11: ffffffff8613f558 R12: ffff9141fdd21e80 R13: 0000000000000000 R14: ffff9141b5976010 R15: ffff914185755338 FS: 00007f45768bd840(0000) GS:ffff9141fdd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000619000424074 CR3: 0000000009f5e005 CR4: 0000000000170ee0 Call Trace: <TASK> timer_delete+0x48/0x80 try_to_grab_pending+0xdf/0x170 __cancel_work+0x37/0xb0 iso_connect_cis+0x141/0x400 [bluetooth] ===============================================================
Trace with NULL conn->hcon in state BT_CONNECT:
===============================================================
__iso_sock_close:619: sk 00000000f7c71fc5 state 1 socket 00000000d90c5fe5
...
__iso_sock_close:619: sk 00000000f7c71fc5 state 8 socket 00000000d90c5fe5 iso_chan_del:153: sk 00000000f7c71fc5, conn 0000000022c03a7e, err 104
...
iso_sock_connect:862: sk 00000000129b56c3 iso_connect_cis:348: 70:1a:b8:98:ff:a2 -> 28:3d:c2:4a:7d:2a hci_get_route:1199: 70:1a:b8:98:ff:a2 -> 28:3d:c2:4a:7d:2a hci_dev_hold:1495: hci0 orig refcnt 19
__iso_chan_add:214: conn 0000000022c03a7e
<Note: reusing old conn>
iso_sock_clear_timer:117: sock 00000000129b56c3 state 3
...
iso_sock_ready:1485: sk 00000000129b56c3
...
iso_sock_sendmsg:1077: sock 00000000e5013966, sk 00000000129b56c3
BUG: kernel NULL pointer dereference, address: 00000000000006a8 PGD 0 P4D 0 Oops: 0000 [#1] PREEMPT SMP PTI CPU: 1 PID: 1403 Comm: wireplumber Tainted: G E 6.3.0-rc7+ #4 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014 RIP: 0010:iso_sock_sendmsg+0x63/0x2a0 [bluetooth] ===============================================================
---
Notes:
v3: no changes
v2: no changes
Don't have a clean reproducer, but these occur sometimes on bluetooth-next/master when sending data over ISO and disconnecting + reconnecting immediately, although the timing window required for these to occur is not so wide.
These appear to be somewhat more easily reproduced if hci_le_set_cig_params is changed to return errors if CIG is busy. Enabling printks also seems to help.
net/bluetooth/iso.c | 53 ++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 22 deletions(-)
net/bluetooth/iso.c +31 -22
@@ -123,8 +123,11 @@ static struct iso_conn *iso_conn_add(struct hci_conn *hcon) {struct iso_conn *conn = hcon->iso_data; -if (conn) +if (conn) { +if (!conn->hcon) + conn->hcon = hcon;return conn; + } conn =kzalloc (sizeof (*conn),GFP_KERNEL );if (!conn)
@@ -311,14 +314,13 @@ static int iso_connect_bis(struct sock *sk) goto unlock; } -hci_dev_unlock (hdev); -hci_dev_put (hdev); +lock_sock (sk); err =iso_chan_add (conn, sk,NULL ); -if (err) -return err; - -lock_sock (sk); +if (err) { +release_sock (sk); + goto unlock; + }/* Update source addr of the socket */ bacpy (&iso_pi (sk)->src, &hcon->src);
@@ -335,7 +337,6 @@ static int iso_connect_bis(struct sock *sk) }release_sock (sk); -return err; unlock:hci_dev_unlock (hdev);
@@ -403,14 +404,13 @@ static int iso_connect_cis(struct sock *sk) goto unlock; } -hci_dev_unlock (hdev); -hci_dev_put (hdev); +lock_sock (sk); err =iso_chan_add (conn, sk,NULL ); -if (err) -return err; - -lock_sock (sk); +if (err) { +release_sock (sk); + goto unlock; + }/* Update source addr of the socket */ bacpy (&iso_pi (sk)->src, &hcon->src);
@@ -427,7 +427,6 @@ static int iso_connect_cis(struct sock *sk) }release_sock (sk); -return err; unlock:hci_dev_unlock (hdev);
@@ -1078,8 +1077,8 @@ static int iso_sock_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) {struct sock *sk = sock->sk; -struct iso_conn *conn =iso_pi (sk)->conn;struct sk_buff *skb, **frag; +size_t mtu;int err;BT_DBG ("sock %p, sk %p" , sock, sk);
@@ -1091,11 +1090,18 @@ static int iso_sock_sendmsg(struct socket *sock, struct msghdr *msg,if (msg->msg_flags &MSG_OOB )return -EOPNOTSUPP ; -if (sk->sk_state !=BT_CONNECTED ) +lock_sock (sk); + +if (sk->sk_state !=BT_CONNECTED ) { +release_sock (sk);return -ENOTCONN ; + } + + mtu =iso_pi (sk)->conn->hcon->hdev->iso_mtu; + +release_sock (sk); - skb =bt_skb_sendmsg (sk, msg, len, conn->hcon->hdev->iso_mtu, -HCI_ISO_DATA_HDR_SIZE ,0 ); + skb =bt_skb_sendmsg (sk, msg, len, mtu,HCI_ISO_DATA_HDR_SIZE ,0 );if (IS_ERR (skb))return PTR_ERR (skb);
@@ -1108,8 +1114,7 @@ static int iso_sock_sendmsg(struct socket *sock, struct msghdr *msg,while (len) {struct sk_buff *tmp; - tmp =bt_skb_sendmsg (sk, msg, len, conn->hcon->hdev->iso_mtu, -0 ,0 ); + tmp =bt_skb_sendmsg (sk, msg, len, mtu,0 ,0 );if (IS_ERR (tmp)) {kfree_skb (skb);return PTR_ERR (tmp);
@@ -1164,15 +1169,19 @@ static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,BT_DBG ("sk %p" , sk);if (test_and_clear_bit (BT_SK_DEFER_SETUP , &bt_sk (sk)->flags)) { +lock_sock (sk);switch (sk->sk_state) {case BT_CONNECT2 : -lock_sock (sk);iso_conn_defer_accept (pi->conn->hcon); sk->sk_state =BT_CONFIG ;release_sock (sk);return 0 ;case BT_CONNECT : +release_sock (sk);return iso_connect_cis (sk); +default : +release_sock (sk); +break ; } }
Signature · 1 line
2.41.0
PATCH v3 1/3 Bluetooth: use RCU for hci_conn_params and iterate safely in hci_sync
In reply to Pauli Virtanenhci_update_accept_list_sync iterates over hdev->pend_le_conns and hdev->pend_le_reports, and waits for controller events in the loop body, without holding hdev lock.
Meanwhile, these lists and the items may be modified e.g. by le_scan_cleanup. This can invalidate the list cursor or any other item in the list, resulting to invalid behavior (eg use-after-free).
Use RCU for the hci_conn_params action lists. Since the loop bodies in hci_sync block and we cannot use RCU or hdev->lock for the whole loop, copy list items first and then iterate on the copy. Only the flags field is written from elsewhere, so READ_ONCE/WRITE_ONCE should guarantee we read valid values.
Free params everywhere with hci_conn_params_free so the cleanup is guaranteed to be done properly.
This fixes the following, which can be triggered e.g. by BlueZ new mgmt-tester case "Add + Remove Device Nowait - Success", or by changing hci_le_set_cig_params to always return false, and running iso-tester:
==================================================================
BUG: KASAN: slab-use-after-free in hci_update_passive_scan_sync (net/bluetooth/hci_sync.c:2536 net/bluetooth/hci_sync.c:2723 net/bluetooth/hci_sync.c:2841) Read of size 8 at addr ffff888001265018 by task kworker/u3:0/32 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014 Workqueue: hci0 hci_cmd_sync_work Call Trace: <TASK> dump_stack_lvl (./arch/x86/include/asm/irqflags.h:134 lib/dump_stack.c:107) print_report (mm/kasan/report.c:320 mm/kasan/report.c:430) ? __virt_addr_valid (./include/linux/mmzone.h:1915 ./include/linux/mmzone.h:2011 arch/x86/mm/physaddr.c:65) ? hci_update_passive_scan_sync (net/bluetooth/hci_sync.c:2536 net/bluetooth/hci_sync.c:2723 net/bluetooth/hci_sync.c:2841) kasan_report (mm/kasan/report.c:538) ? hci_update_passive_scan_sync (net/bluetooth/hci_sync.c:2536 net/bluetooth/hci_sync.c:2723 net/bluetooth/hci_sync.c:2841) hci_update_passive_scan_sync (net/bluetooth/hci_sync.c:2536 net/bluetooth/hci_sync.c:2723 net/bluetooth/hci_sync.c:2841)
? __pfx_hci_update_passive_scan_sync (net/bluetooth/hci_sync.c:2780) ? mutex_lock (kernel/locking/mutex.c:282)
? __pfx_mutex_lock (kernel/locking/mutex.c:282)
? __pfx_mutex_unlock (kernel/locking/mutex.c:538)
? __pfx_update_passive_scan_sync (net/bluetooth/hci_sync.c:2861) hci_cmd_sync_work (net/bluetooth/hci_sync.c:306)
process_one_work (./arch/x86/include/asm/preempt.h:27 kernel/workqueue.c:2399) worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2538)
? __pfx_worker_thread (kernel/workqueue.c:2480)
kthread (kernel/kthread.c:376)
? __pfx_kthread (kernel/kthread.c:331)ret_from_fork (arch/x86/entry/entry_64.S:314)
</TASK> Allocated by task 31: kasan_save_stack (mm/kasan/common.c:46) kasan_set_track (mm/kasan/common.c:52) __kasan_kmalloc (mm/kasan/common.c:374 mm/kasan/common.c:383) hci_conn_params_add (./include/linux/slab.h:580 ./include/linux/slab.h:720 net/bluetooth/hci_core.c:2277) hci_connect_le_scan (net/bluetooth/hci_conn.c:1419 net/bluetooth/hci_conn.c:1589)
hci_connect_cis (net/bluetooth/hci_conn.c:2266) iso_connect_cis (net/bluetooth/iso.c:390)iso_sock_connect (net/bluetooth/iso.c:899)__sys_connect (net/socket.c:2003 net/socket.c:2020) __x64_sys_connect (net/socket.c:2027)do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
Freed by task 15: kasan_save_stack (mm/kasan/common.c:46) kasan_set_track (mm/kasan/common.c:52) kasan_save_free_info (mm/kasan/generic.c:523) __kasan_slab_free (mm/kasan/common.c:238 mm/kasan/common.c:200 mm/kasan/common.c:244) __kmem_cache_free (mm/slub.c:1807 mm/slub.c:3787 mm/slub.c:3800) hci_conn_params_del (net/bluetooth/hci_core.c:2323) le_scan_cleanup (net/bluetooth/hci_conn.c:202) process_one_work (./arch/x86/include/asm/preempt.h:27 kernel/workqueue.c:2399) worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2538)
kthread (kernel/kthread.c:376)ret_from_fork (arch/x86/entry/entry_64.S:314)
==================================================================
---
Notes:
v3:
- Make copy of pend le list, iterate over copy, for simplicity.
- Rename pend_le_* convenience accessors to hci_pend_le_list_add/del_init. These are mainly to avoid needing to think about RCU elsewhere.
v2: use RCU
include/net/bluetooth/hci_core.h | 5 ++ net/bluetooth/hci_conn.c | 10 +-- net/bluetooth/hci_core.c | 38 ++++++++-- net/bluetooth/hci_event.c | 12 ++-- net/bluetooth/hci_sync.c | 117 ++++++++++++++++++++++++++++--- net/bluetooth/mgmt.c | 26 +++---- 6 files changed, 164 insertions(+), 44 deletions(-)
include/net/bluetooth/hci_core.h +5 -0
@@ -822,6 +822,7 @@ struct hci_conn_params {struct hci_conn *conn;bool explicit_connect; +/* Accessed without hdev->lock: */ hci_conn_flags_t flags;u8 privacy_mode; };
@@ -1605,7 +1606,11 @@ struct hci_conn_params *hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);void hci_conn_params_del (struct hci_dev *hdev,bdaddr_t *addr,u8 addr_type);void hci_conn_params_clear_disabled (struct hci_dev *hdev); +void hci_conn_params_free (struct hci_conn_params *param); +void hci_pend_le_list_del_init (struct hci_conn_params *param); +void hci_pend_le_list_add (struct hci_conn_params *param, +struct list_head *list);struct hci_conn_params *hci_pend_le_action_lookup (struct list_head *list,bdaddr_t *addr,u8 addr_type);
net/bluetooth/hci_conn.c +5 -5
@@ -118,7 +118,7 @@ static void hci_connect_le_scan_cleanup(struct hci_conn *conn, u8 status)*/ params->explicit_connect = false; -list_del_init (¶ms->action); +hci_pend_le_list_del_init (params);switch (params->auto_connect) {case HCI_AUTO_CONN_EXPLICIT :
@@ -127,10 +127,10 @@ static void hci_connect_le_scan_cleanup(struct hci_conn *conn, u8 status)return ;case HCI_AUTO_CONN_DIRECT :case HCI_AUTO_CONN_ALWAYS : -list_add (¶ms->action, &hdev->pend_le_conns); +hci_pend_le_list_add (params, &hdev->pend_le_conns);break ;case HCI_AUTO_CONN_REPORT : -list_add (¶ms->action, &hdev->pend_le_reports); +hci_pend_le_list_add (params, &hdev->pend_le_reports);break ;default :break ;
@@ -1435,8 +1435,8 @@ static int hci_explicit_conn_params_set(struct hci_dev *hdev,if (params->auto_connect ==HCI_AUTO_CONN_DISABLED || params->auto_connect ==HCI_AUTO_CONN_REPORT || params->auto_connect ==HCI_AUTO_CONN_EXPLICIT ) { -list_del_init (¶ms->action); -list_add (¶ms->action, &hdev->pend_le_conns); +hci_pend_le_list_del_init (params); +hci_pend_le_list_add (params, &hdev->pend_le_conns); } params->explicit_connect = true;
net/bluetooth/hci_core.c +31 -7
@@ -2249,21 +2249,45 @@ struct hci_conn_params *hci_conn_params_lookup(struct hci_dev *hdev,return NULL ; } -/* This function requires the caller holds hdev->lock */ +/* This function requires the caller holds hdev->lock or rcu_read_lock */ struct hci_conn_params *hci_pend_le_action_lookup (struct list_head *list,bdaddr_t *addr,u8 addr_type) {struct hci_conn_params *param; -list_for_each_entry (param, list, action) { +rcu_read_lock (); + +list_for_each_entry_rcu (param, list, action) {if (bacmp (¶m->addr, addr) ==0 && - param->addr_type == addr_type) + param->addr_type == addr_type) { +rcu_read_unlock ();return param; + } } +rcu_read_unlock (); +return NULL ; } +/* This function requires the caller holds hdev->lock */ +void hci_pend_le_list_del_init (struct hci_conn_params *param) +{ +if (list_empty (¶m->action)) +return ; + +list_del_rcu (¶m->action); +synchronize_rcu (); +INIT_LIST_HEAD (¶m->action); +} + +/* This function requires the caller holds hdev->lock */ +void hci_pend_le_list_add (struct hci_conn_params *param, +struct list_head *list) +{ +list_add_rcu (¶m->action, list); +} +/* This function requires the caller holds hdev->lock */ struct hci_conn_params *hci_conn_params_add (struct hci_dev *hdev,bdaddr_t *addr,u8 addr_type)
@@ -2297,14 +2321,15 @@ struct hci_conn_params *hci_conn_params_add(struct hci_dev *hdev,return params; } -static void hci_conn_params_free (struct hci_conn_params *params) +void hci_conn_params_free (struct hci_conn_params *params) { +hci_pend_le_list_del_init (params); +if (params->conn) {hci_conn_drop (params->conn);hci_conn_put (params->conn); } -list_del (¶ms->action);list_del (¶ms->list);kfree (params); }
@@ -2342,8 +2367,7 @@ void hci_conn_params_clear_disabled(struct hci_dev *hdev)continue ; } -list_del (¶ms->list); -kfree (params); +hci_conn_params_free (params); }BT_DBG ("All LE disabled connection parameters were removed" );
net/bluetooth/hci_event.c +6 -6
@@ -1564,7 +1564,7 @@ static u8 hci_cc_le_set_privacy_mode(struct hci_dev *hdev, void *data, params =hci_conn_params_lookup (hdev, &cp->bdaddr, cp->bdaddr_type);if (params) - params->privacy_mode = cp->mode; +WRITE_ONCE (params->privacy_mode, cp->mode);hci_dev_unlock (hdev);
@@ -2804,8 +2804,8 @@ static void hci_cs_disconnect(struct hci_dev *hdev, u8 status)case HCI_AUTO_CONN_DIRECT :case HCI_AUTO_CONN_ALWAYS : -list_del_init (¶ms->action); -list_add (¶ms->action, &hdev->pend_le_conns); +hci_pend_le_list_del_init (params); +hci_pend_le_list_add (params, &hdev->pend_le_conns);break ;default :
@@ -3423,8 +3423,8 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, void *data,case HCI_AUTO_CONN_DIRECT :case HCI_AUTO_CONN_ALWAYS : -list_del_init (¶ms->action); -list_add (¶ms->action, &hdev->pend_le_conns); +hci_pend_le_list_del_init (params); +hci_pend_le_list_add (params, &hdev->pend_le_conns);hci_update_passive_scan (hdev);break ;
@@ -5972,7 +5972,7 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status, params =hci_pend_le_action_lookup (&hdev->pend_le_conns, &conn->dst, conn->dst_type);if (params) { -list_del_init (¶ms->action); +hci_pend_le_list_del_init (params);if (params->conn) {hci_conn_drop (params->conn);hci_conn_put (params->conn);
net/bluetooth/hci_sync.c +106 -11
205 lines not on this page · raw message
net/bluetooth/mgmt.c +11 -15
@@ -1303,15 +1303,15 @@ static void restart_le_actions(struct hci_dev *hdev)/* Needed for AUTO_OFF case where might not "really" * have been powered off. */ -list_del_init (&p->action); +hci_pend_le_list_del_init (p);switch (p->auto_connect) {case HCI_AUTO_CONN_DIRECT :case HCI_AUTO_CONN_ALWAYS : -list_add (&p->action, &hdev->pend_le_conns); +hci_pend_le_list_add (p, &hdev->pend_le_conns);break ;case HCI_AUTO_CONN_REPORT : -list_add (&p->action, &hdev->pend_le_reports); +hci_pend_le_list_add (p, &hdev->pend_le_reports);break ;default :break ;
@@ -5175,7 +5175,7 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data, goto unlock; } - params->flags = current_flags; +WRITE_ONCE (params->flags, current_flags); status =MGMT_STATUS_SUCCESS ;/* Update passive scan if HCI_CONN_FLAG_DEVICE_PRIVACY
@@ -7586,7 +7586,7 @@ static int hci_conn_params_set(struct hci_dev *hdev, bdaddr_t *addr,if (params->auto_connect == auto_connect)return 0 ; -list_del_init (¶ms->action); +hci_pend_le_list_del_init (params);switch (auto_connect) {case HCI_AUTO_CONN_DISABLED :
@@ -7595,18 +7595,18 @@ static int hci_conn_params_set(struct hci_dev *hdev, bdaddr_t *addr,* connect to device, keep connecting. */ if (params->explicit_connect) -list_add (¶ms->action, &hdev->pend_le_conns); +hci_pend_le_list_add (params, &hdev->pend_le_conns);break ;case HCI_AUTO_CONN_REPORT :if (params->explicit_connect) -list_add (¶ms->action, &hdev->pend_le_conns); +hci_pend_le_list_add (params, &hdev->pend_le_conns);else -list_add (¶ms->action, &hdev->pend_le_reports); +hci_pend_le_list_add (params, &hdev->pend_le_reports);break ;case HCI_AUTO_CONN_DIRECT :case HCI_AUTO_CONN_ALWAYS :if (!is_connected (hdev, addr, addr_type)) -list_add (¶ms->action, &hdev->pend_le_conns); +hci_pend_le_list_add (params, &hdev->pend_le_conns);break ; }
@@ -7829,9 +7829,7 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev, goto unlock; } -list_del (¶ms->action); -list_del (¶ms->list); -kfree (params); +hci_conn_params_free (params);device_removed (sk, hdev, &cp->addr.bdaddr, cp->addr.type); }else {
@@ -7862,9 +7860,7 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev, p->auto_connect =HCI_AUTO_CONN_EXPLICIT ;continue ; } -list_del (&p->action); -list_del (&p->list); -kfree (p); +hci_conn_params_free (p); }bt_dev_dbg (hdev,"All LE connection parameters were removed" );
Signature · 1 line
2.41.0
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=758199
---Test result---
Test Summary:
CheckPatch FAIL 3.82 seconds GitLint FAIL 1.35 seconds SubjectPrefix PASS 0.43 seconds BuildKernel PASS 32.47 seconds CheckAllWarning PASS 35.94 seconds CheckSparse WARNING 40.85 seconds CheckSmatch WARNING 110.76 seconds BuildKernel32 PASS 31.56 seconds TestRunnerSetup PASS 448.68 seconds TestRunner_l2cap-tester PASS 17.64 seconds TestRunner_iso-tester PASS 24.63 seconds TestRunner_bnep-tester PASS 5.84 seconds TestRunner_mgmt-tester PASS 134.92 seconds TestRunner_rfcomm-tester PASS 9.48 seconds TestRunner_sco-tester PASS 8.68 seconds TestRunner_ioctl-tester PASS 10.01 seconds TestRunner_mesh-tester PASS 7.41 seconds TestRunner_smp-tester PASS 8.55 seconds TestRunner_userchan-tester PASS 6.15 seconds IncrementalBuild PASS 41.46 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[v3,1/3] Bluetooth: use RCU for hci_conn_params and iterate safely in hci_sync
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #104:
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014
total: 0 errors, 1 warnings, 0 checks, 439 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13283914.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[v3,2/3] Bluetooth: hci_event: call disconnect callback before deleting conn
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #130:
CPU: 0 PID: 1175 Comm: bluetoothd Tainted: G E 6.4.0-rc4+ #2
total: 0 errors, 1 warnings, 0 checks, 9 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13283911.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[v3,3/3] Bluetooth: ISO: fix iso_conn related locking and validity issues
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #120:
iso_connect_cfm:1700: hcon 000000007b65d182 bdaddr 28:3d:c2:4a:7e:da status 12
total: 0 errors, 1 warnings, 0 checks, 123 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13283913.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v3,1/3] Bluetooth: use RCU for hci_conn_params and iterate safely in hci_sync
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
25: B1 Line exceeds max length (155>80): "BUG: KASAN: slab-use-after-free in hci_update_passive_scan_sync (net/bluetooth/hci_sync.c:2536 net/bluetooth/hci_sync.c:2723 net/bluetooth/hci_sync.c:2841)"
28: B1 Line exceeds max length (81>80): "Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014"
34: B1 Line exceeds max length (107>80): "? __virt_addr_valid (./include/linux/mmzone.h:1915 ./include/linux/mmzone.h:2011 arch/x86/mm/physaddr.c:65)"
35: B1 Line exceeds max length (122>80): "? hci_update_passive_scan_sync (net/bluetooth/hci_sync.c:2536 net/bluetooth/hci_sync.c:2723 net/bluetooth/hci_sync.c:2841)"
37: B1 Line exceeds max length (122>80): "? hci_update_passive_scan_sync (net/bluetooth/hci_sync.c:2536 net/bluetooth/hci_sync.c:2723 net/bluetooth/hci_sync.c:2841)"
38: B1 Line exceeds max length (120>80): "hci_update_passive_scan_sync (net/bluetooth/hci_sync.c:2536 net/bluetooth/hci_sync.c:2723 net/bluetooth/hci_sync.c:2841)"
57: B1 Line exceeds max length (105>80): "hci_conn_params_add (./include/linux/slab.h:580 ./include/linux/slab.h:720 net/bluetooth/hci_core.c:2277)"
58: B1 Line exceeds max length (81>80): "hci_connect_le_scan (net/bluetooth/hci_conn.c:1419 net/bluetooth/hci_conn.c:1589)"
71: B1 Line exceeds max length (85>80): "__kasan_slab_free (mm/kasan/common.c:238 mm/kasan/common.c:200 mm/kasan/common.c:244)"
89: B2 Line has trailing whitespace: " "
[v3,2/3] Bluetooth: hci_event: call disconnect callback before deleting conn
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
36: B1 Line exceeds max length (81>80): "Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014"
54: B1 Line exceeds max length (81>80): "Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014"
94: B1 Line exceeds max length (199>80): "Code: 15 d1 1f 0d 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 80 3d 9d a7 0d 00 00 74 13 b8 14 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 48 83 ec 28 89 54 24 1c 48 89"
[v3,3/3] Bluetooth: ISO: fix iso_conn related locking and validity issues
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
68: B1 Line exceeds max length (81>80): "Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014"
90: B1 Line exceeds max length (106>80): "general protection fault, probably for non-canonical address 0x30b29c630930aec8: 0000 [#1] PREEMPT SMP PTI"
92: B1 Line exceeds max length (81>80): "Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014"
94: B1 Line exceeds max length (134>80): "Code: 90 90 0f 1f 44 00 00 48 8b 47 08 48 85 c0 0f 84 ad 00 00 00 55 89 d5 53 48 83 3f 00 48 89 fb 74 7d 66 90 48 8b 03 48 8b 53 08 <>"
134: B1 Line exceeds max length (81>80): "Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014"
144: B2 Line has trailing whitespace: " "
149: B2 Line has trailing whitespace: " "
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
---
Regards,
Linux Bluetooth
This series was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <[email protected]>:
Hello:
This series was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <[email protected]>:
Quoted 8 lines · On Mon, 19 Jun 2023 01:04:30 +0300 you wrote:
This series addresses some concurrency issues (NULL / GPF) in ISO sockets or related.
v3:
- Copy the pend_le_* lists, iterate over copy to make it simple.
- Rename to hci_pend_le_list_*[...]
Here is the summary with links:
- [v3,1/3] Bluetooth: use RCU for hci_conn_params and iterate safely in hci_sync
https://git.kernel.org/bluetooth/bluetooth-next/c/7b48b117e516 - [v3,2/3] Bluetooth: hci_event: call disconnect callback before deleting conn https://git.kernel.org/bluetooth/bluetooth-next/c/e61a5be1d421 - [v3,3/3] Bluetooth: ISO: fix iso_conn related locking and validity issues https://git.kernel.org/bluetooth/bluetooth-next/c/d9687da53dab
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html