Commit e83f5e24 authored by Jiexun Wang's avatar Jiexun Wang Committed by Luiz Augusto von Dentz
Browse files

Bluetooth: serialize accept_q access 



bt_sock_poll() walks the accept queue without synchronization, while
child teardown can unlink the same socket and drop its last reference.
The unsynchronized accept queue walk has existed since the initial
Bluetooth import.

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: 1da177e4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: default avatarJann Horn <jannh@google.com>
Reported-by: default avatarYuan Tan <yuantan098@gmail.com>
Reported-by: default avatarYifan Wu <yifanwucs@gmail.com>
Reported-by: default avatarJuefei Pu <tomapufckgml@gmail.com>
Reported-by: default avatarXin Liu <bird@lzu.edu.cn>
Signed-off-by: default avatarJiexun Wang <wangjiexun2025@gmail.com>
Signed-off-by: default avatarRen Wei <n05ec@lzu.edu.cn>
Signed-off-by: default avatarJiexun Wang <wangjiexun2025@gmail.com>
Reviewed-by: default avatarJann Horn <jannh@google.com>
Signed-off-by: default avatarLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
parent c78bdba7

include/net/bluetooth/bluetooth.h

+1 −0
Original line number Diff line number Diff line
@@ -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 *);

net/bluetooth/af_bluetooth.c

+65 −22
Original line number Diff line number Diff line
@@ -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,