[PW_SID:1031774] [RFC,v2,1/2] Bluetooth: fix locking in hci_conn_request_evt() with HCI_PROTO_DEFER #3211
Conversation
This patch adds workflow files for ci: [sync.yml] - The workflow file for scheduled work - Sync the repo with upstream repo and rebase the workflow branch - Review the patches in the patchwork and creates the PR if needed [ci.yml] - The workflow file for CI tasks - Run CI tests when PR is created Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
When protocol sets HCI_PROTO_DEFER, hci_conn_request_evt() calls hci_connect_cfm(conn) without hdev->lock. Only SCO and ISO set HCI_PROTO_DEFER (for listening socket with defer setup). Nothing guarantees conn remains alive after unlock. In all other code paths (also listening socket without defer setup), hci_connect_cfm(conn) is called with hdev->lock held. Fix by holding the lock. Fixes: 70c4642 ("Bluetooth: Refactor connection request handling") Signed-off-by: Pauli Virtanen <pav@iki.fi>
hci_conn::sco_data does not own a refcount to sco_conn and the field is updated also without hdev->lock, so it cannot be safely accessed in sco_connect_cfm(). UAF observed due to wrong refcounting is reported. Revise the sco_conn ownership and locking: - hci_conn::sco_data owns refcount; field protected by hdev->lock - sco_pi(sk)::conn owns refcount; field protected by lock_sock() - sco_conn::hcon and sk own no refcount; fields protected by conn->lock Use lockdep + Sparse to try to enforce proper locking. Add locks where they were missing. Use separate functions that detach sco_conn from hcon and sk. Don't do operations that take locks in sco_conn_free() so that sco_conn_put() is safe to use with locks. Handle the race when hcon obtains locked sk via sco_conn, which requires sco_conn_unlock due to lock ordering. In sco_conn_ready() fix calling sleeping functions under spinlock. Fixes: ecb9a84 ("Bluetooth: SCO: Fix UAF on sco_conn_free") Reported-by: Cen Zhang <rollkingzzc@gmail.com> Closes: https://lore.kernel.org/linux-bluetooth/44091d60.3570.19a40a89dd8.Coremail.zzzccc427@163.com/ Signed-off-by: Pauli Virtanen <pav@iki.fi>
|
CheckPatch |
|
GitLint |
|
SubjectPrefix |
|
BuildKernel |
|
CheckAllWarning |
|
CheckSparse |
|
BuildKernel32 |
|
TestRunnerSetup |
|
TestRunner_l2cap-tester |
|
TestRunner_iso-tester |
|
TestRunner_bnep-tester |
|
TestRunner_mgmt-tester |
|
TestRunner_rfcomm-tester |
|
TestRunner_sco-tester |
|
TestRunner_ioctl-tester |
|
TestRunner_mesh-tester |
|
TestRunner_smp-tester |
|
TestRunner_userchan-tester |
|
IncrementalBuild |
github-actions
bot
force-pushed
the
workflow
branch
4 times, most recently
from
dbdc24e to
7b93eea
Compare
github-actions
bot
force-pushed
the
workflow
branch
4 times, most recently
from
1e7b027 to
8eabaf6
Compare
github-actions
bot
force-pushed
the
workflow
branch
from
8eabaf6 to
b7ba526
Compare
When protocol sets HCI_PROTO_DEFER, hci_conn_request_evt() calls
hci_connect_cfm(conn) without hdev->lock. Only SCO and ISO set
HCI_PROTO_DEFER (for listening socket with defer setup).
Nothing guarantees conn remains alive after unlock. In all other code
paths (also listening socket without defer setup), hci_connect_cfm(conn)
is called with hdev->lock held.
Fix by holding the lock.
Fixes: 70c4642 ("Bluetooth: Refactor connection request handling")
Signed-off-by: Pauli Virtanen pav@iki.fi
Notes:
These two patches are pending some further testing in practice, but
lockdep is now happy about how this works.
net/bluetooth/hci_event.c | 3 ---
1 file changed, 3 deletions(-)