[PATCH v3] Bluetooth: af_bluetooth: Fix double list_del and UAF in accept_q
Nguyen Ngoc Thang <[email protected]>
| Newsgroups | org.kernel.vger.linux-kernel,org.kernel.vger.linux-bluetooth |
|---|---|
| Message-ID | <[email protected]> |
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: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=534002670dd34a114fdc Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Nguyen Ngoc Thang <[email protected]> --- net/bluetooth/af_bluetooth.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c index 411d66f24393..7797b1052808 100644 --- a/net/bluetooth/af_bluetooth.c +++ b/net/bluetooth/af_bluetooth.c @@ -224,6 +224,10 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh) else lock_sock_nested(sk, SINGLE_DEPTH_NESTING); + /* Hold a reference on parent so it cannot be freed while sk keeps + * a raw pointer to it via bt_sk(sk)->parent. + */ + sock_hold(parent); bt_sk(sk)->parent = parent; spin_lock_bh(&par->accept_q_lock); @@ -257,15 +261,24 @@ EXPORT_SYMBOL(bt_accept_enqueue); void bt_accept_unlink(struct sock *sk) { struct sock *parent = bt_sk(sk)->parent; + bool unlinked = false; - BT_DBG("sk %p state %d", sk, sk->sk_state); + if (!parent) + return; spin_lock_bh(&bt_sk(parent)->accept_q_lock); - list_del_init(&bt_sk(sk)->accept_q); - sk_acceptq_removed(parent); + if (!list_empty(&bt_sk(sk)->accept_q)) { + list_del_init(&bt_sk(sk)->accept_q); + sk_acceptq_removed(parent); + unlinked = true; + } spin_unlock_bh(&bt_sk(parent)->accept_q_lock); - bt_sk(sk)->parent = NULL; - sock_put(sk); + + if (unlinked) { + bt_sk(sk)->parent = NULL; + sock_put(parent); + sock_put(sk); + } } EXPORT_SYMBOL(bt_accept_unlink); -- 2.43.0