[PW_SID:1103047] Bluetooth: Fix data-race on dst/src in connect paths#257
Open
BluezTestBot wants to merge 2 commits into
Open
[PW_SID:1103047] Bluetooth: Fix data-race on dst/src in connect paths#257BluezTestBot wants to merge 2 commits into
BluezTestBot wants to merge 2 commits into
Conversation
iso_connect_bis(), iso_connect_cis(), iso_listen_bis(), and iso_conn_big_sync() all call hci_get_route() reading iso_pi(sk)->dst, iso_pi(sk)->src, and iso_pi(sk)->src_type without holding lock_sock. These fields can be concurrently written by another thread calling connect() or setsockopt() on the same socket, leading to torn reads or TOCTOU mismatches. Fix by snapshotting dst, src, and src_type into local variables under lock_sock before calling hci_get_route() in all four functions. BUG: KCSAN: data-race in memcmp+0x45/0xb0 race at unknown origin, with read to 0xffff8880122135cf of 1 bytes by task 333 on cpu 1: memcmp+0x45/0xb0 hci_get_route+0x27e/0x490 iso_connect_cis+0x4c/0xa10 iso_sock_connect+0x60e/0xb30 __sys_connect_file+0xbd/0xe0 __sys_connect+0xe0/0x110 __x64_sys_connect+0x40/0x50 x64_sys_call+0xcad/0x1c60 do_syscall_64+0x133/0x590 entry_SYSCALL_64_after_hwframe+0x77/0x7f Fixes: 241f519 ("Bluetooth: ISO: Avoid circular locking dependency") Signed-off-by: SeungJu Cheon <suunj1331@gmail.com>
sco_sock_connect() copies the destination address into sco_pi(sk)->dst under lock_sock, then releases the lock and calls sco_connect(), which reads dst back without holding any lock in hci_get_route() and hci_connect_sco(). If two threads call connect() on the same socket concurrently with different addresses, one thread can overwrite dst before the other thread's sco_connect() reads it. Fix by snapshotting dst into a local variable under lock_sock at the start of sco_connect(), matching the approach used for ISO in the previous patch. BUG: KCSAN: data-race in memcmp+0x45/0xb0 race at unknown origin, with read to 0xffff88800e6b0dd0 of 1 bytes by task 315 on cpu 0: memcmp+0x45/0xb0 hci_connect_acl+0x1b7/0x6b0 hci_connect_sco+0x4d/0xb30 sco_sock_connect+0x27b/0xd60 __sys_connect_file+0xbd/0xe0 __sys_connect+0xe0/0x110 __x64_sys_connect+0x40/0x50 x64_sys_call+0xcad/0x1c60 do_syscall_64+0x133/0x590 entry_SYSCALL_64_after_hwframe+0x77/0x7f Fixes: 9a8ec9e ("Bluetooth: Fix three socket race condition bugs in sco.c") Signed-off-by: SeungJu Cheon <suunj1331@gmail.com>
|
CheckPatch |
|
VerifyFixes |
|
VerifySignedoff |
|
GitLint |
|
SubjectPrefix |
|
BuildKernel |
|
CheckAllWarning |
|
CheckSparse |
|
BuildKernel32 |
|
TestRunnerSetup |
|
TestRunner_iso-tester |
|
TestRunner_sco-tester |
|
IncrementalBuild |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
iso_connect_bis(), iso_connect_cis(), iso_listen_bis(), and
iso_conn_big_sync() all call hci_get_route() reading iso_pi(sk)->dst,
iso_pi(sk)->src, and iso_pi(sk)->src_type without holding lock_sock.
These fields can be concurrently written by another thread calling
connect() or setsockopt() on the same socket, leading to torn reads
or TOCTOU mismatches.
Fix by snapshotting dst, src, and src_type into local variables under
lock_sock before calling hci_get_route() in all four functions.
BUG: KCSAN: data-race in memcmp+0x45/0xb0
race at unknown origin, with read to 0xffff8880122135cf of 1 bytes by task 333 on cpu 1:
memcmp+0x45/0xb0
hci_get_route+0x27e/0x490
iso_connect_cis+0x4c/0xa10
iso_sock_connect+0x60e/0xb30
__sys_connect_file+0xbd/0xe0
__sys_connect+0xe0/0x110
__x64_sys_connect+0x40/0x50
x64_sys_call+0xcad/0x1c60
do_syscall_64+0x133/0x590
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Fixes: 241f519 ("Bluetooth: ISO: Avoid circular locking dependency")
Signed-off-by: SeungJu Cheon suunj1331@gmail.com
net/bluetooth/iso.c | 51 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 39 insertions(+), 12 deletions(-)