All of lore.kernel.org
 help / color / mirror / Atom feed
From: oficerovas@altlinux.org
To: oficerovas@altlinux.org, stable@vger.kernel.org,
	linux-bluetooth@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Pauli Virtanen <pav@iki.fi>,
	Luiz Augusto von Dentz <luiz.von.dentz@intel.com>,
	Paolo Abeni <pabeni@redhat.com>, Jakub Kicinski <kuba@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	kovalev@altlinux.org, nickel@altlinux.org, dutyrok@altlinux.org
Subject: [PATCH 2/2] Bluetooth: SCO: fix sco_conn related locking and validity issues
Date: Mon, 12 Feb 2024 16:09:33 +0300	[thread overview]
Message-ID: <20240212130933.3856081-3-oficerovas@altlinux.org> (raw)
In-Reply-To: <20240212130933.3856081-1-oficerovas@altlinux.org>

From: Alexander Ofitserov <oficerovas@altlinux.org>

From: Pauli Virtanen <pav@iki.fi>

commit 3dcaa192ac21 ("Bluetooth: SCO: fix sco_conn related locking and validity issues")

Operations that check/update sk_state and access conn should hold
lock_sock, otherwise they can race.

The order of taking locks is hci_dev_lock > lock_sock > sco_conn_lock,
which is how it is in connect/disconnect_cfm -> sco_conn_del ->
sco_chan_del.

Fix locking in sco_connect to take lock_sock around updating sk_state
and conn.

sco_conn_del must not occur during sco_connect, as it frees the
sco_conn. Hold hdev->lock longer to prevent that.

sco_conn_add shall return sco_conn with valid hcon. Make it so also when
reusing an old SCO connection waiting for disconnect timeout (see
__sco_sock_close where conn->hcon is set to NULL).

This should not reintroduce the issue fixed in the earlier
commit 9a8ec9e8ebb5 ("Bluetooth: SCO: Fix possible circular locking
dependency on sco_connect_cfm"), the relevant fix of releasing lock_sock
in sco_sock_connect before acquiring hdev->lock is retained.

These changes mirror similar fixes earlier in ISO sockets.

Fixes: 9a8ec9e8ebb5 ("Bluetooth: SCO: Fix possible circular locking dependency on sco_connect_cfm")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Alexander Ofitserov <oficerovas@altlinux.org>
---
 net/bluetooth/sco.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 0e1f5dde7bfec..99b149261949a 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -126,8 +126,11 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
 	struct hci_dev *hdev = hcon->hdev;
 	struct sco_conn *conn = hcon->sco_data;
 
-	if (conn)
+	if (conn) {
+		if (!conn->hcon)
+			conn->hcon = hcon;
 		return conn;
+	}
 
 	conn = kzalloc(sizeof(struct sco_conn), GFP_KERNEL);
 	if (!conn)
@@ -268,21 +271,21 @@ static int sco_connect(struct sock *sk)
 		goto unlock;
 	}
 
-	hci_dev_unlock(hdev);
-	hci_dev_put(hdev);
-
 	conn = sco_conn_add(hcon);
 	if (!conn) {
 		hci_conn_drop(hcon);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto unlock;
 	}
 
-	err = sco_chan_add(conn, sk, NULL);
-	if (err)
-		return err;
-
 	lock_sock(sk);
 
+	err = sco_chan_add(conn, sk, NULL);
+	if (err) {
+		release_sock(sk);
+		goto unlock;
+	}
+
 	/* Update source addr of the socket */
 	bacpy(&sco_pi(sk)->src, &hcon->src);
 
@@ -296,8 +299,6 @@ static int sco_connect(struct sock *sk)
 
 	release_sock(sk);
 
-	return err;
-
 unlock:
 	hci_dev_unlock(hdev);
 	hci_dev_put(hdev);
-- 
2.42.1


  parent reply	other threads:[~2024-02-12 13:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-12 13:09 [PATCH 0/2] Bluetooth: SCO: possible deadlock in sco_conn_del oficerovas
2024-02-12 13:09 ` [PATCH 1/2] Bluetooth: SCO: Fix possible circular locking dependency on sco_connect_cfm oficerovas
2024-02-12 13:14   ` kernel test robot
2024-02-12 13:36   ` Bluetooth: SCO: possible deadlock in sco_conn_del bluez.test.bot
2024-02-12 13:09 ` oficerovas [this message]
2024-02-26 13:28 [PATCH 0/2] " Alexander Ofitserov
2024-02-26 13:28 ` [PATCH 2/2] Bluetooth: SCO: fix sco_conn related locking and validity issues Alexander Ofitserov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240212130933.3856081-3-oficerovas@altlinux.org \
    --to=oficerovas@altlinux.org \
    --cc=dutyrok@altlinux.org \
    --cc=edumazet@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kovalev@altlinux.org \
    --cc=kuba@kernel.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=luiz.von.dentz@intel.com \
    --cc=nickel@altlinux.org \
    --cc=pabeni@redhat.com \
    --cc=pav@iki.fi \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.