[PATCH] Bluetooth: af_bluetooth: hold a reference on parent in accept_q

Nguyen Ngoc Thang posted 1 patch 1 week, 1 day ago
net/bluetooth/af_bluetooth.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH] Bluetooth: af_bluetooth: hold a reference on parent in accept_q
Posted by Nguyen Ngoc Thang 1 week, 1 day ago
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.

Take a reference on parent in bt_accept_enqueue() and drop it in
bt_accept_unlink() to keep it alive for as long as a child points to
it.

Reported-by: syzbot+534002670dd34a114fdc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=534002670dd34a114fdc
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
 net/bluetooth/af_bluetooth.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 411d66f24393..f9aae18c6733 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);
@@ -265,6 +269,7 @@ void bt_accept_unlink(struct sock *sk)
 	sk_acceptq_removed(parent);
 	spin_unlock_bh(&bt_sk(parent)->accept_q_lock);
 	bt_sk(sk)->parent = NULL;
+	sock_put(parent);
 	sock_put(sk);
 }
 EXPORT_SYMBOL(bt_accept_unlink);
-- 
2.43.0
Re: [PATCH] Bluetooth: af_bluetooth: hold a reference on parent in accept_q
Posted by Pauli Virtanen 1 week, 1 day ago
Hi,

ke, 2026-09-16 kello 22:38 +0700, Nguyen Ngoc Thang kirjoitti:
> 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.
>
> Take a reference on parent in bt_accept_enqueue() and drop it in
> bt_accept_unlink() to keep it alive for as long as a child points to
> it.

AFAIK the UAF described above cannot occur in current kernel:

https://lore.kernel.org/linux-bluetooth/3674f5d34d731403bfa0e680c9e5718237612315.camel@iki.fi/

> Reported-by: syzbot+534002670dd34a114fdc@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=534002670dd34a114fdc
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
>  net/bluetooth/af_bluetooth.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index 411d66f24393..f9aae18c6733 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);
> @@ -265,6 +269,7 @@ void bt_accept_unlink(struct sock *sk)
>  	sk_acceptq_removed(parent);
>  	spin_unlock_bh(&bt_sk(parent)->accept_q_lock);
>  	bt_sk(sk)->parent = NULL;
> +	sock_put(parent);
>  	sock_put(sk);
>  }
>  EXPORT_SYMBOL(bt_accept_unlink);

-- 
Pauli Virtanen