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 <[email protected]>
---
Notes:
Possibly related:
https://lore.kernel.org/linux-bluetooth/[email protected]/
https://lore.kernel.org/linux-bluetooth/HwByr9Ffw_HVVE5pnMu6CTIrQjKwBJpJcSskM0xOP0S-4XiIdmGFTLYAcgKJsOkrxt62je88hFJ1vkHzYtCgwsEATe0KFq3E4YX8LeN5Fdo=@protonmail.com/
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 bbf7072f5e80..ec6dce488a40 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.41.0
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=764106
---Test result---
Test Summary:
CheckPatch PASS 0.72 seconds
GitLint FAIL 0.58 seconds
SubjectPrefix PASS 0.12 seconds
BuildKernel PASS 33.87 seconds
CheckAllWarning PASS 37.12 seconds
CheckSparse WARNING 42.51 seconds
CheckSmatch WARNING 113.52 seconds
BuildKernel32 PASS 32.46 seconds
TestRunnerSetup PASS 500.07 seconds
TestRunner_l2cap-tester PASS 23.85 seconds
TestRunner_iso-tester PASS 42.86 seconds
TestRunner_bnep-tester PASS 10.82 seconds
TestRunner_mgmt-tester PASS 223.11 seconds
TestRunner_rfcomm-tester PASS 16.22 seconds
TestRunner_sco-tester PASS 17.16 seconds
TestRunner_ioctl-tester PASS 18.11 seconds
TestRunner_mesh-tester PASS 13.46 seconds
TestRunner_smp-tester PASS 14.88 seconds
TestRunner_userchan-tester PASS 11.35 seconds
IncrementalBuild PASS 31.27 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: SCO: fix sco_conn related locking and validity issues
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
32: B2 Line has trailing whitespace: " "
33: B1 Line exceeds max length (84>80): " https://lore.kernel.org/linux-bluetooth/[email protected]/"
34: B1 Line exceeds max length (168>80): " https://lore.kernel.org/linux-bluetooth/HwByr9Ffw_HVVE5pnMu6CTIrQjKwBJpJcSskM0xOP0S-4XiIdmGFTLYAcgKJsOkrxt62je88hFJ1vkHzYtCgwsEATe0KFq3E4YX8LeN5Fdo=@protonmail.com/"
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:150:35: warning: array of flexible structures
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
net/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:150:35: warning: array of flexible structures
---
Regards,
Linux Bluetooth
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Mon, 10 Jul 2023 19:48:19 +0300 you wrote:
> 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.
>
> [...]
Here is the summary with links:
- Bluetooth: SCO: fix sco_conn related locking and validity issues
https://git.kernel.org/bluetooth/bluetooth-next/c/23ba0a1d51bd
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html