[PATCH v2 1/1] Bluetooth: serialize accept_q access

[PATCH v2 1/1] Bluetooth: serialize accept_q access

[Ren Wei]

From: Jiexun Wang <wangjiexun2025@gmail.com>

bt_sock_poll() walks the accept queue without synchronization, while
child teardown can unlink the same socket and drop its last reference.

Protect accept_q with a dedicated lock for queue updates and polling.
Also rework bt_accept_dequeue() to take temporary child references under
the queue lock before dropping it and locking the child socket.

Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Co-developed-by: Yuan Tan <yuantan098@gmail.com>
Signed-off-by: Yuan Tan <yuantan098@gmail.com>
Suggested-by: Xin Liu <bird@lzu.edu.cn>
Tested-by: Ren Wei <enjou1224z@gmail.com>
Signed-off-by: Jiexun Wang <wangjiexun2025@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
Changes in v2:
- add Tested-by: Ren Wei <enjou1224z@gmail.com>
- resend to the public Bluetooth/netdev lists

 include/net/bluetooth/bluetooth.h |  1 +
 net/bluetooth/af_bluetooth.c      | 85 +++++++++++++++++++++++--------
 2 files changed, 66 insertions(+), 20 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 69eed69f7f26..3faea66b1979 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -398,6 +398,7 @@ void baswap(bdaddr_t *dst, const bdaddr_t *src);
 struct bt_sock {
 	struct sock sk;
 	struct list_head accept_q;
+	spinlock_t accept_q_lock; /* protects accept_q */
 	struct sock *parent;
 	unsigned long flags;
 	void (*skb_msg_name)(struct sk_buff *, void *, int *);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 2b94e2077203..f44e1ecc83d8 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -154,6 +154,7 @@ struct sock *bt_sock_alloc(struct net *net, struct socket *sock,
 
 	sock_init_data(sock, sk);
 	INIT_LIST_HEAD(&bt_sk(sk)->accept_q);
+	spin_lock_init(&bt_sk(sk)->accept_q_lock);
 
 	sock_reset_flag(sk, SOCK_ZAPPED);
 
@@ -214,6 +215,7 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
 {
 	const struct cred *old_cred;
 	struct pid *old_pid;
+	struct bt_sock *par = bt_sk(parent);
 
 	BT_DBG("parent %p, sk %p", parent, sk);
 
@@ -224,9 +226,12 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
 	else
 		lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
 
-	list_add_tail(&bt_sk(sk)->accept_q, &bt_sk(parent)->accept_q);
 	bt_sk(sk)->parent = parent;
 
+	spin_lock_bh(&par->accept_q_lock);
+	list_add_tail(&bt_sk(sk)->accept_q, &par->accept_q);
+	spin_unlock_bh(&par->accept_q_lock);
+
 	/* Copy credentials from parent since for incoming connections the
 	 * socket is allocated by the kernel.
 	 */
@@ -254,45 +259,73 @@ EXPORT_SYMBOL(bt_accept_enqueue);
  */
 void bt_accept_unlink(struct sock *sk)
 {
+	struct sock *parent = bt_sk(sk)->parent;
+
 	BT_DBG("sk %p state %d", sk, sk->sk_state);
 
+	spin_lock_bh(&bt_sk(parent)->accept_q_lock);
 	list_del_init(&bt_sk(sk)->accept_q);
-	sk_acceptq_removed(bt_sk(sk)->parent);
+	spin_unlock_bh(&bt_sk(parent)->accept_q_lock);
+
+	sk_acceptq_removed(parent);
 	bt_sk(sk)->parent = NULL;
 	sock_put(sk);
 }
 EXPORT_SYMBOL(bt_accept_unlink);
 
+static struct sock *bt_accept_get(struct sock *parent, struct sock *sk)
+{
+	struct bt_sock *bt = bt_sk(parent);
+	struct sock *next = NULL;
+
+	/* accept_q is modified from child teardown paths too, so take a
+	 * temporary reference before dropping the queue lock.
+	 */
+	spin_lock_bh(&bt->accept_q_lock);
+
+	if (sk) {
+		if (bt_sk(sk)->parent != parent)
+			goto out;
+
+		if (!list_is_last(&bt_sk(sk)->accept_q, &bt->accept_q)) {
+			next = &list_next_entry(bt_sk(sk), accept_q)->sk;
+			sock_hold(next);
+		}
+	} else if (!list_empty(&bt->accept_q)) {
+		next = &list_first_entry(&bt->accept_q,
+					 struct bt_sock, accept_q)->sk;
+		sock_hold(next);
+	}
+
+out:
+	spin_unlock_bh(&bt->accept_q_lock);
+	return next;
+}
+
 struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
 {
-	struct bt_sock *s, *n;
-	struct sock *sk;
+	struct sock *sk, *next;
 
 	BT_DBG("parent %p", parent);
 
 restart:
-	list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
-		sk = (struct sock *)s;
-
+	for (sk = bt_accept_get(parent, NULL); sk; sk = next) {
 		/* Prevent early freeing of sk due to unlink and sock_kill */
-		sock_hold(sk);
 		lock_sock(sk);
 
 		/* Check sk has not already been unlinked via
 		 * bt_accept_unlink() due to serialisation caused by sk locking
 		 */
-		if (!bt_sk(sk)->parent) {
+		if (bt_sk(sk)->parent != parent) {
 			BT_DBG("sk %p, already unlinked", sk);
 			release_sock(sk);
 			sock_put(sk);
 
-			/* Restart the loop as sk is no longer in the list
-			 * and also avoid a potential infinite loop because
-			 * list_for_each_entry_safe() is not thread safe.
-			 */
 			goto restart;
 		}
 
+		next = bt_accept_get(parent, sk);
+
 		/* sk is safely in the parent list so reduce reference count */
 		sock_put(sk);
 
@@ -310,6 +343,8 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
 				sock_graft(sk, newsock);
 
 			release_sock(sk);
+			if (next)
+				sock_put(next);
 			return sk;
 		}
 
@@ -518,18 +553,28 @@ EXPORT_SYMBOL(bt_sock_stream_recvmsg);
 
 static inline __poll_t bt_accept_poll(struct sock *parent)
 {
-	struct bt_sock *s, *n;
+	struct bt_sock *bt = bt_sk(parent);
+	struct bt_sock *s;
 	struct sock *sk;
+	__poll_t mask = 0;
+
+	spin_lock_bh(&bt->accept_q_lock);
+	list_for_each_entry(s, &bt->accept_q, accept_q) {
+		int state;
 
-	list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
 		sk = (struct sock *)s;
-		if (sk->sk_state == BT_CONNECTED ||
-		    (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags) &&
-		     sk->sk_state == BT_CONNECT2))
-			return EPOLLIN | EPOLLRDNORM;
+		state = READ_ONCE(sk->sk_state);
+
+		if (state == BT_CONNECTED ||
+		    (test_bit(BT_SK_DEFER_SETUP, &bt->flags) &&
+		     state == BT_CONNECT2)) {
+			mask = EPOLLIN | EPOLLRDNORM;
+			break;
+		}
 	}
+	spin_unlock_bh(&bt->accept_q_lock);
 
-	return 0;
+	return mask;
 }
 
 __poll_t bt_sock_poll(struct file *file, struct socket *sock,
-- 
2.34.1

Re: [PATCH v2 1/1] Bluetooth: serialize accept_q access

[Paul Menzel]

Dear Ren,


Thank you for your patch.

Am 03.04.26 um 10:05 schrieb Ren Wei:
> From: Jiexun Wang <wangjiexun2025@gmail.com>
> 
> bt_sock_poll() walks the accept queue without synchronization, while
> child teardown can unlink the same socket and drop its last reference.
> 
> Protect accept_q with a dedicated lock for queue updates and polling.
> Also rework bt_accept_dequeue() to take temporary child references under
> the queue lock before dropping it and locking the child socket.
> 
> Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Ren Wei <enjou1224z@gmail.com>
> Signed-off-by: Jiexun Wang <wangjiexun2025@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
> Changes in v2:
> - add Tested-by: Ren Wei <enjou1224z@gmail.com>
> - resend to the public Bluetooth/netdev lists
> 
>   include/net/bluetooth/bluetooth.h |  1 +
>   net/bluetooth/af_bluetooth.c      | 85 +++++++++++++++++++++++--------
>   2 files changed, 66 insertions(+), 20 deletions(-)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 69eed69f7f26..3faea66b1979 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -398,6 +398,7 @@ void baswap(bdaddr_t *dst, const bdaddr_t *src);
>   struct bt_sock {
>   	struct sock sk;
>   	struct list_head accept_q;
> +	spinlock_t accept_q_lock; /* protects accept_q */
>   	struct sock *parent;
>   	unsigned long flags;
>   	void (*skb_msg_name)(struct sk_buff *, void *, int *);
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index 2b94e2077203..f44e1ecc83d8 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -154,6 +154,7 @@ struct sock *bt_sock_alloc(struct net *net, struct socket *sock,
>   
>   	sock_init_data(sock, sk);
>   	INIT_LIST_HEAD(&bt_sk(sk)->accept_q);
> +	spin_lock_init(&bt_sk(sk)->accept_q_lock);
>   
>   	sock_reset_flag(sk, SOCK_ZAPPED);
>   
> @@ -214,6 +215,7 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
>   {
>   	const struct cred *old_cred;
>   	struct pid *old_pid;
> +	struct bt_sock *par = bt_sk(parent);
>   
>   	BT_DBG("parent %p, sk %p", parent, sk);
>   
> @@ -224,9 +226,12 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
>   	else
>   		lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
>   
> -	list_add_tail(&bt_sk(sk)->accept_q, &bt_sk(parent)->accept_q);
>   	bt_sk(sk)->parent = parent;
>   
> +	spin_lock_bh(&par->accept_q_lock);
> +	list_add_tail(&bt_sk(sk)->accept_q, &par->accept_q);
> +	spin_unlock_bh(&par->accept_q_lock);
> +
>   	/* Copy credentials from parent since for incoming connections the
>   	 * socket is allocated by the kernel.
>   	 */
> @@ -254,45 +259,73 @@ EXPORT_SYMBOL(bt_accept_enqueue);
>    */
>   void bt_accept_unlink(struct sock *sk)
>   {
> +	struct sock *parent = bt_sk(sk)->parent;
> +
>   	BT_DBG("sk %p state %d", sk, sk->sk_state);
>   
> +	spin_lock_bh(&bt_sk(parent)->accept_q_lock);
>   	list_del_init(&bt_sk(sk)->accept_q);
> -	sk_acceptq_removed(bt_sk(sk)->parent);
> +	spin_unlock_bh(&bt_sk(parent)->accept_q_lock);
> +
> +	sk_acceptq_removed(parent);
>   	bt_sk(sk)->parent = NULL;
>   	sock_put(sk);
>   }
>   EXPORT_SYMBOL(bt_accept_unlink);
>   
> +static struct sock *bt_accept_get(struct sock *parent, struct sock *sk)
> +{
> +	struct bt_sock *bt = bt_sk(parent);
> +	struct sock *next = NULL;
> +
> +	/* accept_q is modified from child teardown paths too, so take a
> +	 * temporary reference before dropping the queue lock.
> +	 */
> +	spin_lock_bh(&bt->accept_q_lock);
> +
> +	if (sk) {
> +		if (bt_sk(sk)->parent != parent)
> +			goto out;
> +
> +		if (!list_is_last(&bt_sk(sk)->accept_q, &bt->accept_q)) {
> +			next = &list_next_entry(bt_sk(sk), accept_q)->sk;
> +			sock_hold(next);
> +		}
> +	} else if (!list_empty(&bt->accept_q)) {
> +		next = &list_first_entry(&bt->accept_q,
> +					 struct bt_sock, accept_q)->sk;
> +		sock_hold(next);
> +	}
> +
> +out:
> +	spin_unlock_bh(&bt->accept_q_lock);
> +	return next;
> +}
> +
>   struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
>   {
> -	struct bt_sock *s, *n;
> -	struct sock *sk;
> +	struct sock *sk, *next;
>   
>   	BT_DBG("parent %p", parent);
>   
>   restart:
> -	list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
> -		sk = (struct sock *)s;
> -
> +	for (sk = bt_accept_get(parent, NULL); sk; sk = next) {
>   		/* Prevent early freeing of sk due to unlink and sock_kill */
> -		sock_hold(sk);
>   		lock_sock(sk);
>   
>   		/* Check sk has not already been unlinked via
>   		 * bt_accept_unlink() due to serialisation caused by sk locking
>   		 */
> -		if (!bt_sk(sk)->parent) {
> +		if (bt_sk(sk)->parent != parent) {
>   			BT_DBG("sk %p, already unlinked", sk);
>   			release_sock(sk);
>   			sock_put(sk);
>   
> -			/* Restart the loop as sk is no longer in the list
> -			 * and also avoid a potential infinite loop because
> -			 * list_for_each_entry_safe() is not thread safe.
> -			 */
>   			goto restart;
>   		}
>   
> +		next = bt_accept_get(parent, sk);
> +
>   		/* sk is safely in the parent list so reduce reference count */
>   		sock_put(sk);
>   
> @@ -310,6 +343,8 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
>   				sock_graft(sk, newsock);
>   
>   			release_sock(sk);
> +			if (next)
> +				sock_put(next);
>   			return sk;
>   		}
>   
> @@ -518,18 +553,28 @@ EXPORT_SYMBOL(bt_sock_stream_recvmsg);
>   
>   static inline __poll_t bt_accept_poll(struct sock *parent)
>   {
> -	struct bt_sock *s, *n;
> +	struct bt_sock *bt = bt_sk(parent);
> +	struct bt_sock *s;
>   	struct sock *sk;
> +	__poll_t mask = 0;
> +
> +	spin_lock_bh(&bt->accept_q_lock);
> +	list_for_each_entry(s, &bt->accept_q, accept_q) {
> +		int state;
>   
> -	list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
>   		sk = (struct sock *)s;
> -		if (sk->sk_state == BT_CONNECTED ||
> -		    (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags) &&
> -		     sk->sk_state == BT_CONNECT2))
> -			return EPOLLIN | EPOLLRDNORM;
> +		state = READ_ONCE(sk->sk_state);
> +
> +		if (state == BT_CONNECTED ||
> +		    (test_bit(BT_SK_DEFER_SETUP, &bt->flags) &&
> +		     state == BT_CONNECT2)) {
> +			mask = EPOLLIN | EPOLLRDNORM;
> +			break;
> +		}
>   	}
> +	spin_unlock_bh(&bt->accept_q_lock);
>   
> -	return 0;
> +	return mask;
>   }
>   
>   __poll_t bt_sock_poll(struct file *file, struct socket *sock,

Google’s Gemini review service has a comment [1]. Do you think it’s 
valid and related?


Kind regards,

Paul


[1]: 
https://sashiko.dev/#/patchset/06a6b4549acba207847ce532dedbf1c95ab22d13.1774925231.git.wangjiexun2025%40gmail.com

Re: [PATCH v2 1/1] Bluetooth: serialize accept_q access

[Luiz Augusto von Dentz]

Hi Ren,

On Fri, Apr 3, 2026 at 4:05 AM Ren Wei <n05ec@lzu.edu.cn> wrote:
>
> From: Jiexun Wang <wangjiexun2025@gmail.com>
>
> bt_sock_poll() walks the accept queue without synchronization, while
> child teardown can unlink the same socket and drop its last reference.
>
> Protect accept_q with a dedicated lock for queue updates and polling.
> Also rework bt_accept_dequeue() to take temporary child references under
> the queue lock before dropping it and locking the child socket.
>
> Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Ren Wei <enjou1224z@gmail.com>
> Signed-off-by: Jiexun Wang <wangjiexun2025@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
> Changes in v2:
> - add Tested-by: Ren Wei <enjou1224z@gmail.com>
> - resend to the public Bluetooth/netdev lists
>
>  include/net/bluetooth/bluetooth.h |  1 +
>  net/bluetooth/af_bluetooth.c      | 85 +++++++++++++++++++++++--------
>  2 files changed, 66 insertions(+), 20 deletions(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 69eed69f7f26..3faea66b1979 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -398,6 +398,7 @@ void baswap(bdaddr_t *dst, const bdaddr_t *src);
>  struct bt_sock {
>         struct sock sk;
>         struct list_head accept_q;
> +       spinlock_t accept_q_lock; /* protects accept_q */
>         struct sock *parent;
>         unsigned long flags;
>         void (*skb_msg_name)(struct sk_buff *, void *, int *);
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index 2b94e2077203..f44e1ecc83d8 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -154,6 +154,7 @@ struct sock *bt_sock_alloc(struct net *net, struct socket *sock,
>
>         sock_init_data(sock, sk);
>         INIT_LIST_HEAD(&bt_sk(sk)->accept_q);
> +       spin_lock_init(&bt_sk(sk)->accept_q_lock);
>
>         sock_reset_flag(sk, SOCK_ZAPPED);
>
> @@ -214,6 +215,7 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
>  {
>         const struct cred *old_cred;
>         struct pid *old_pid;
> +       struct bt_sock *par = bt_sk(parent);
>
>         BT_DBG("parent %p, sk %p", parent, sk);
>
> @@ -224,9 +226,12 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
>         else
>                 lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
>
> -       list_add_tail(&bt_sk(sk)->accept_q, &bt_sk(parent)->accept_q);
>         bt_sk(sk)->parent = parent;
>
> +       spin_lock_bh(&par->accept_q_lock);
> +       list_add_tail(&bt_sk(sk)->accept_q, &par->accept_q);
> +       spin_unlock_bh(&par->accept_q_lock);
> +
>         /* Copy credentials from parent since for incoming connections the
>          * socket is allocated by the kernel.
>          */
> @@ -254,45 +259,73 @@ EXPORT_SYMBOL(bt_accept_enqueue);
>   */
>  void bt_accept_unlink(struct sock *sk)
>  {
> +       struct sock *parent = bt_sk(sk)->parent;
> +
>         BT_DBG("sk %p state %d", sk, sk->sk_state);
>
> +       spin_lock_bh(&bt_sk(parent)->accept_q_lock);
>         list_del_init(&bt_sk(sk)->accept_q);
> -       sk_acceptq_removed(bt_sk(sk)->parent);
> +       spin_unlock_bh(&bt_sk(parent)->accept_q_lock);
> +
> +       sk_acceptq_removed(parent);
>         bt_sk(sk)->parent = NULL;
>         sock_put(sk);
>  }
>  EXPORT_SYMBOL(bt_accept_unlink);
>
> +static struct sock *bt_accept_get(struct sock *parent, struct sock *sk)
> +{
> +       struct bt_sock *bt = bt_sk(parent);
> +       struct sock *next = NULL;
> +
> +       /* accept_q is modified from child teardown paths too, so take a
> +        * temporary reference before dropping the queue lock.
> +        */
> +       spin_lock_bh(&bt->accept_q_lock);
> +
> +       if (sk) {
> +               if (bt_sk(sk)->parent != parent)
> +                       goto out;
> +
> +               if (!list_is_last(&bt_sk(sk)->accept_q, &bt->accept_q)) {
> +                       next = &list_next_entry(bt_sk(sk), accept_q)->sk;
> +                       sock_hold(next);
> +               }
> +       } else if (!list_empty(&bt->accept_q)) {
> +               next = &list_first_entry(&bt->accept_q,
> +                                        struct bt_sock, accept_q)->sk;
> +               sock_hold(next);
> +       }
> +
> +out:
> +       spin_unlock_bh(&bt->accept_q_lock);
> +       return next;
> +}
> +
>  struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
>  {
> -       struct bt_sock *s, *n;
> -       struct sock *sk;
> +       struct sock *sk, *next;
>
>         BT_DBG("parent %p", parent);
>
>  restart:
> -       list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
> -               sk = (struct sock *)s;
> -
> +       for (sk = bt_accept_get(parent, NULL); sk; sk = next) {
>                 /* Prevent early freeing of sk due to unlink and sock_kill */
> -               sock_hold(sk);
>                 lock_sock(sk);
>
>                 /* Check sk has not already been unlinked via
>                  * bt_accept_unlink() due to serialisation caused by sk locking
>                  */
> -               if (!bt_sk(sk)->parent) {
> +               if (bt_sk(sk)->parent != parent) {
>                         BT_DBG("sk %p, already unlinked", sk);
>                         release_sock(sk);
>                         sock_put(sk);
>
> -                       /* Restart the loop as sk is no longer in the list
> -                        * and also avoid a potential infinite loop because
> -                        * list_for_each_entry_safe() is not thread safe.
> -                        */
>                         goto restart;
>                 }
>
> +               next = bt_accept_get(parent, sk);
> +
>                 /* sk is safely in the parent list so reduce reference count */
>                 sock_put(sk);
>
> @@ -310,6 +343,8 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
>                                 sock_graft(sk, newsock);
>
>                         release_sock(sk);
> +                       if (next)
> +                               sock_put(next);
>                         return sk;
>                 }
>
> @@ -518,18 +553,28 @@ EXPORT_SYMBOL(bt_sock_stream_recvmsg);
>
>  static inline __poll_t bt_accept_poll(struct sock *parent)
>  {
> -       struct bt_sock *s, *n;
> +       struct bt_sock *bt = bt_sk(parent);
> +       struct bt_sock *s;
>         struct sock *sk;
> +       __poll_t mask = 0;
> +
> +       spin_lock_bh(&bt->accept_q_lock);
> +       list_for_each_entry(s, &bt->accept_q, accept_q) {
> +               int state;
>
> -       list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
>                 sk = (struct sock *)s;
> -               if (sk->sk_state == BT_CONNECTED ||
> -                   (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags) &&
> -                    sk->sk_state == BT_CONNECT2))
> -                       return EPOLLIN | EPOLLRDNORM;
> +               state = READ_ONCE(sk->sk_state);
> +
> +               if (state == BT_CONNECTED ||
> +                   (test_bit(BT_SK_DEFER_SETUP, &bt->flags) &&
> +                    state == BT_CONNECT2)) {
> +                       mask = EPOLLIN | EPOLLRDNORM;
> +                       break;
> +               }
>         }
> +       spin_unlock_bh(&bt->accept_q_lock);
>
> -       return 0;
> +       return mask;
>  }
>
>  __poll_t bt_sock_poll(struct file *file, struct socket *sock,
> --
> 2.34.1

https://sashiko.dev/#/patchset/06a6b4549acba207847ce532dedbf1c95ab22d13.1774925231.git.wangjiexun2025%40gmail.com

Seem valid to me.

-- 
Luiz Augusto von Dentz

[PATCH v3 1/1] Bluetooth: serialize accept_q access

[Ren Wei]

From: Jiexun Wang <wangjiexun2025@gmail.com>

bt_sock_poll() walks the accept queue without synchronization, while
child teardown can unlink the same socket and drop its last reference.

Protect accept_q with a dedicated lock for queue updates and polling.
Also rework bt_accept_dequeue() to take temporary child references under
the queue lock before dropping it and locking the child socket.

Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Co-developed-by: Yuan Tan <yuantan098@gmail.com>
Signed-off-by: Yuan Tan <yuantan098@gmail.com>
Suggested-by: Xin Liu <bird@lzu.edu.cn>
Tested-by: Ren Wei <enjou1224z@gmail.com>
Signed-off-by: Jiexun Wang <wangjiexun2025@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
Changes in v3:
- move sk_acceptq_added()/sk_acceptq_removed() inside accept_q_lock
  critical sections to serialize sk_ack_backlog updates with accept_q
  operations

Changes in v2:
- add Tested-by: Ren Wei <enjou1224z@gmail.com>
- resend to the public Bluetooth/netdev lists

 include/net/bluetooth/bluetooth.h |  1 +
 net/bluetooth/af_bluetooth.c      | 87 +++++++++++++++++++++++--------
 2 files changed, 66 insertions(+), 22 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 69eed69f7f26..3faea66b1979 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -398,6 +398,7 @@ void baswap(bdaddr_t *dst, const bdaddr_t *src);
 struct bt_sock {
 	struct sock sk;
 	struct list_head accept_q;
+	spinlock_t accept_q_lock; /* protects accept_q */
 	struct sock *parent;
 	unsigned long flags;
 	void (*skb_msg_name)(struct sk_buff *, void *, int *);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 2b94e2077203..fa14b9a915eb 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -154,6 +154,7 @@ struct sock *bt_sock_alloc(struct net *net, struct socket *sock,
 
 	sock_init_data(sock, sk);
 	INIT_LIST_HEAD(&bt_sk(sk)->accept_q);
+	spin_lock_init(&bt_sk(sk)->accept_q_lock);
 
 	sock_reset_flag(sk, SOCK_ZAPPED);
 
@@ -214,6 +215,7 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
 {
 	const struct cred *old_cred;
 	struct pid *old_pid;
+	struct bt_sock *par = bt_sk(parent);
 
 	BT_DBG("parent %p, sk %p", parent, sk);
 
@@ -224,9 +226,13 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
 	else
 		lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
 
-	list_add_tail(&bt_sk(sk)->accept_q, &bt_sk(parent)->accept_q);
 	bt_sk(sk)->parent = parent;
 
+	spin_lock_bh(&par->accept_q_lock);
+	list_add_tail(&bt_sk(sk)->accept_q, &par->accept_q);
+	sk_acceptq_added(parent);
+	spin_unlock_bh(&par->accept_q_lock);
+
 	/* Copy credentials from parent since for incoming connections the
 	 * socket is allocated by the kernel.
 	 */
@@ -244,8 +250,6 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
 		bh_unlock_sock(sk);
 	else
 		release_sock(sk);
-
-	sk_acceptq_added(parent);
 }
 EXPORT_SYMBOL(bt_accept_enqueue);
 
@@ -254,45 +258,72 @@ EXPORT_SYMBOL(bt_accept_enqueue);
  */
 void bt_accept_unlink(struct sock *sk)
 {
+	struct sock *parent = bt_sk(sk)->parent;
+
 	BT_DBG("sk %p state %d", sk, sk->sk_state);
 
+	spin_lock_bh(&bt_sk(parent)->accept_q_lock);
 	list_del_init(&bt_sk(sk)->accept_q);
-	sk_acceptq_removed(bt_sk(sk)->parent);
+	sk_acceptq_removed(parent);
+	spin_unlock_bh(&bt_sk(parent)->accept_q_lock);
 	bt_sk(sk)->parent = NULL;
 	sock_put(sk);
 }
 EXPORT_SYMBOL(bt_accept_unlink);
 
+static struct sock *bt_accept_get(struct sock *parent, struct sock *sk)
+{
+	struct bt_sock *bt = bt_sk(parent);
+	struct sock *next = NULL;
+
+	/* accept_q is modified from child teardown paths too, so take a
+	 * temporary reference before dropping the queue lock.
+	 */
+	spin_lock_bh(&bt->accept_q_lock);
+
+	if (sk) {
+		if (bt_sk(sk)->parent != parent)
+			goto out;
+
+		if (!list_is_last(&bt_sk(sk)->accept_q, &bt->accept_q)) {
+			next = &list_next_entry(bt_sk(sk), accept_q)->sk;
+			sock_hold(next);
+		}
+	} else if (!list_empty(&bt->accept_q)) {
+		next = &list_first_entry(&bt->accept_q,
+					 struct bt_sock, accept_q)->sk;
+		sock_hold(next);
+	}
+
+out:
+	spin_unlock_bh(&bt->accept_q_lock);
+	return next;
+}
+
 struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
 {
-	struct bt_sock *s, *n;
-	struct sock *sk;
+	struct sock *sk, *next;
 
 	BT_DBG("parent %p", parent);
 
 restart:
-	list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
-		sk = (struct sock *)s;
-
+	for (sk = bt_accept_get(parent, NULL); sk; sk = next) {
 		/* Prevent early freeing of sk due to unlink and sock_kill */
-		sock_hold(sk);
 		lock_sock(sk);
 
 		/* Check sk has not already been unlinked via
 		 * bt_accept_unlink() due to serialisation caused by sk locking
 		 */
-		if (!bt_sk(sk)->parent) {
+		if (bt_sk(sk)->parent != parent) {
 			BT_DBG("sk %p, already unlinked", sk);
 			release_sock(sk);
 			sock_put(sk);
 
-			/* Restart the loop as sk is no longer in the list
-			 * and also avoid a potential infinite loop because
-			 * list_for_each_entry_safe() is not thread safe.
-			 */
 			goto restart;
 		}
 
+		next = bt_accept_get(parent, sk);
+
 		/* sk is safely in the parent list so reduce reference count */
 		sock_put(sk);
 
@@ -310,6 +341,8 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
 				sock_graft(sk, newsock);
 
 			release_sock(sk);
+			if (next)
+				sock_put(next);
 			return sk;
 		}
 
@@ -518,18 +551,28 @@ EXPORT_SYMBOL(bt_sock_stream_recvmsg);
 
 static inline __poll_t bt_accept_poll(struct sock *parent)
 {
-	struct bt_sock *s, *n;
+	struct bt_sock *bt = bt_sk(parent);
+	struct bt_sock *s;
 	struct sock *sk;
+	__poll_t mask = 0;
+
+	spin_lock_bh(&bt->accept_q_lock);
+	list_for_each_entry(s, &bt->accept_q, accept_q) {
+		int state;
 
-	list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
 		sk = (struct sock *)s;
-		if (sk->sk_state == BT_CONNECTED ||
-		    (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags) &&
-		     sk->sk_state == BT_CONNECT2))
-			return EPOLLIN | EPOLLRDNORM;
+		state = READ_ONCE(sk->sk_state);
+
+		if (state == BT_CONNECTED ||
+		    (test_bit(BT_SK_DEFER_SETUP, &bt->flags) &&
+		     state == BT_CONNECT2)) {
+			mask = EPOLLIN | EPOLLRDNORM;
+			break;
+		}
 	}
+	spin_unlock_bh(&bt->accept_q_lock);
 
-	return 0;
+	return mask;
 }
 
 __poll_t bt_sock_poll(struct file *file, struct socket *sock,
-- 
2.34.1