public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Alexander Ofitserov <oficerovas@altlinux.org>
To: oficerovas@altlinux.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: lvc-project@linuxtesting.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,
	stable@vger.kernel.org
Subject: [PATCH 2/2] Bluetooth: SCO: fix sco_conn related locking and validity issues
Date: Mon, 26 Feb 2024 16:28:37 +0300	[thread overview]
Message-ID: <20240226132837.8404-3-oficerovas@altlinux.org> (raw)
In-Reply-To: <20240226132837.8404-1-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>
Cc: stable@vger.kernel.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-26 13:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-26 13:28 [PATCH 0/2] possible deadlock in sco_conn_del Alexander Ofitserov
2024-02-26 13:28 ` [PATCH 1/2] Bluetooth: SCO: Fix possible circular locking dependency on sco_connect_cfm Alexander Ofitserov
2024-02-26 13:28 ` Alexander Ofitserov [this message]
  -- strict thread matches above, loose matches on Subject: below --
2024-02-12 13:09 [PATCH 0/2] Bluetooth: SCO: possible deadlock in sco_conn_del oficerovas
2024-02-12 13:09 ` [PATCH 2/2] Bluetooth: SCO: fix sco_conn related locking and validity issues oficerovas

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=20240226132837.8404-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=luiz.von.dentz@intel.com \
    --cc=lvc-project@linuxtesting.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox