[PW_SID:1168560] [v3] Bluetooth: af_bluetooth: Fix double list_del and UAF in accept_q - #790
BluezTestBot wants to merge 1 commit into
Conversation
bt_sk(sk)->parent is a raw pointer with no refcount backing it, so a child sitting in the listening socket's accept_q can outlive it. When the child's channel is torn down independently (e.g. hci_error_reset() -> l2cap_conn_del() -> l2cap_sock_teardown_cb()), bt_accept_unlink() dereferences the freed parent, corrupting its accept_q_lock and list. Additionally, if multiple threads concurrently tear down the child socket, bt_accept_unlink() can be called multiple times. Without checking if the socket is still in the list, the second thread will perform a double list_del_init(), causing list_del corruption (kernel BUG). Fix this by: 1. Taking a reference on parent in bt_accept_enqueue() and dropping it only when actually unlinked in bt_accept_unlink(). 2. Checking !list_empty(&bt_sk(sk)->accept_q) inside the accept_q_lock to prevent double unlinking. 3. Moving sock_put() outside of the spinlock to avoid sleeping in an atomic context. Reported-by: syzbot+534002670dd34a114fdc@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=534002670dd34a114fdc Fixes: 1da177e ("Linux-2.6.12-rc2") Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
|
CheckPatch |
|
VerifyFixes |
|
VerifySignedoff |
|
GitLint |
|
SubjectPrefix |
|
BuildKernel |
|
CheckAllWarning |
|
CheckSparse |
|
BuildKernel32 |
|
CheckKernelLLVM |
|
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 |
|
TestRunner_6lowpan-tester |
|
IncrementalBuild |
2 participants
bt_sk(sk)->parent is a raw pointer with no refcount backing it, so a
child sitting in the listening socket's accept_q can outlive it. When
the child's channel is torn down independently (e.g. hci_error_reset()
-> l2cap_conn_del() -> l2cap_sock_teardown_cb()), bt_accept_unlink()
dereferences the freed parent, corrupting its accept_q_lock and list.
Additionally, if multiple threads concurrently tear down the child
socket, bt_accept_unlink() can be called multiple times. Without
checking if the socket is still in the list, the second thread will
perform a double list_del_init(), causing list_del corruption (kernel BUG).
Fix this by:
only when actually unlinked in bt_accept_unlink().
to prevent double unlinking.
atomic context.
Reported-by: syzbot+534002670dd34a114fdc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=534002670dd34a114fdc
Fixes: 1da177e ("Linux-2.6.12-rc2")
Signed-off-by: Nguyen Ngoc Thang ngocthang2710.1999@gmail.com
net/bluetooth/af_bluetooth.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)