In hci_abort_conn_sync, some errors from hci_le_connect_cancel_sync
indicate the connection can be terminated immediately. However,
hci_abort_conn_sync passes these errors through, making
hci_disconnect_all_sync stop processing in these cases.
Fix by using a separate flag to indicate if canceled connection can be
deleted directly.
Fixes: 0f2efc1d02d5 ("Bluetooth: hci_conn: Consolidate code for aborting connections")
Fixes: dd1f6778b17b ("Bluetooth: hci_sync: delete CIS in BT_OPEN/CONNECT/BOUND when aborting")
Signed-off-by: Pauli Virtanen <[email protected]>
---
net/bluetooth/hci_sync.c | 48 ++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 41a8e57d8267..51ff682f66e0 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5253,13 +5253,14 @@ static int hci_disconnect_sync(struct hci_dev *hdev, struct hci_conn *conn,
}
static int hci_le_connect_cancel_sync(struct hci_dev *hdev,
- struct hci_conn *conn, u8 reason)
+ struct hci_conn *conn, u8 reason,
+ bool *del)
{
- /* Return reason if scanning since the connection shall probably be
- * cleanup directly.
- */
- if (test_bit(HCI_CONN_SCANNING, &conn->flags))
- return reason;
+ /* If scanning, connection can be just deleted */
+ if (test_bit(HCI_CONN_SCANNING, &conn->flags)) {
+ *del = true;
+ return 0;
+ }
if (conn->role == HCI_ROLE_SLAVE ||
test_and_set_bit(HCI_CONN_CANCEL, &conn->flags))
@@ -5270,10 +5271,10 @@ static int hci_le_connect_cancel_sync(struct hci_dev *hdev,
}
static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn,
- u8 reason)
+ u8 reason, bool *del)
{
if (conn->type == LE_LINK)
- return hci_le_connect_cancel_sync(hdev, conn, reason);
+ return hci_le_connect_cancel_sync(hdev, conn, reason, del);
if (conn->type == ISO_LINK) {
/* BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
@@ -5287,9 +5288,9 @@ static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn,
if (test_bit(HCI_CONN_CREATE_CIS, &conn->flags))
return hci_disconnect_sync(hdev, conn, reason);
- /* CIS with no Create CIS sent have nothing to cancel */
+ /* CIS with no Create CIS sent can be just deleted */
if (bacmp(&conn->dst, BDADDR_ANY))
- return HCI_ERROR_LOCAL_HOST_TERM;
+ *del = true;
/* There is no way to cancel a BIS without terminating the BIG
* which is done later on connection cleanup.
@@ -5370,6 +5371,7 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
{
int err = 0;
u16 handle = conn->handle;
+ bool del = false;
switch (conn->state) {
case BT_CONNECTED:
@@ -5377,17 +5379,15 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
err = hci_disconnect_sync(hdev, conn, reason);
break;
case BT_CONNECT:
- err = hci_connect_cancel_sync(hdev, conn, reason);
+ err = hci_connect_cancel_sync(hdev, conn, reason, &del);
break;
case BT_CONNECT2:
err = hci_reject_conn_sync(hdev, conn, reason);
break;
case BT_OPEN:
case BT_BOUND:
- hci_dev_lock(hdev);
- hci_conn_failed(conn, reason);
- hci_dev_unlock(hdev);
- return 0;
+ del = true;
+ break;
default:
conn->state = BT_CLOSED;
return 0;
@@ -5398,18 +5398,15 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
* or in case of LE it was still scanning so it can be cleanup
* safely.
*/
- if (err) {
- struct hci_conn *c;
+ if (err || del) {
+ hci_dev_lock(hdev);
/* Check if the connection hasn't been cleanup while waiting
* commands to complete.
*/
- c = hci_conn_hash_lookup_handle(hdev, handle);
- if (!c || c != conn)
- return 0;
+ if (hci_conn_hash_lookup_handle(hdev, handle) == conn)
+ hci_conn_failed(conn, del ? reason : err);
- hci_dev_lock(hdev);
- hci_conn_failed(conn, err);
hci_dev_unlock(hdev);
}
@@ -6311,8 +6308,11 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn)
conn->conn_timeout, NULL);
done:
- if (err == -ETIMEDOUT)
- hci_le_connect_cancel_sync(hdev, conn, 0x00);
+ if (err == -ETIMEDOUT) {
+ bool __maybe_unused del;
+
+ hci_le_connect_cancel_sync(hdev, conn, 0x00, &del);
+ }
/* Re-enable advertising after the connection attempt is finished. */
hci_resume_advertising_sync(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=773436
---Test result---
Test Summary:
CheckPatch FAIL 2.27 seconds
GitLint FAIL 1.19 seconds
SubjectPrefix PASS 0.35 seconds
BuildKernel PASS 32.20 seconds
CheckAllWarning PASS 35.20 seconds
CheckSparse PASS 40.12 seconds
CheckSmatch PASS 111.99 seconds
BuildKernel32 PASS 31.53 seconds
TestRunnerSetup PASS 472.33 seconds
TestRunner_l2cap-tester PASS 22.17 seconds
TestRunner_iso-tester PASS 43.64 seconds
TestRunner_bnep-tester PASS 10.03 seconds
TestRunner_mgmt-tester PASS 211.21 seconds
TestRunner_rfcomm-tester PASS 15.03 seconds
TestRunner_sco-tester PASS 18.23 seconds
TestRunner_ioctl-tester PASS 17.04 seconds
TestRunner_mesh-tester PASS 12.45 seconds
TestRunner_smp-tester PASS 13.56 seconds
TestRunner_userchan-tester PASS 10.49 seconds
IncrementalBuild PASS 40.72 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[2/3] Bluetooth: hci_sync: fix use-after-free in hci_disconnect_all_sync
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#100:
CPU: 0 PID: 124 Comm: kworker/u9:0 Tainted: G W 6.5.0-rc1+ #10
total: 0 errors, 1 warnings, 0 checks, 50 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13342873.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[2/3] Bluetooth: hci_sync: fix use-after-free in hci_disconnect_all_sync
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
13: B1 Line exceeds max length (99>80): "BUG: KASAN: slab-use-after-free in hci_set_powered_sync (net/bluetooth/hci_sync.c:5424) [bluetooth]"
17: B1 Line exceeds max length (81>80): "Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014"
---
Regards,
Linux Bluetooth
When processing connections in abort_conn_sync, also check the
connection handle still refers to a connection that should be aborted.
There is a theoretical race condition where a connection handle is
reused, after hci_abort_conn but before abort_conn_sync is processed in
hci_sync. This change should avoid terminating a wrong connection in
this case.
Signed-off-by: Pauli Virtanen <[email protected]>
---
net/bluetooth/hci_conn.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index ae206eb551f7..b65a1e9b178b 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -2861,7 +2861,7 @@ static int abort_conn_sync(struct hci_dev *hdev, void *data)
u16 handle = PTR_ERR(data);
conn = hci_conn_hash_lookup_handle(hdev, handle);
- if (!conn)
+ if (!conn || !conn->abort_reason)
return 0;
return hci_abort_conn_sync(hdev, conn, conn->abort_reason);
@@ -2876,6 +2876,8 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
*/
if (conn->abort_reason)
return 0;
+ if (!reason)
+ return -EINVAL;
bt_dev_dbg(hdev, "handle 0x%2.2x reason 0x%2.2x", conn->handle, reason);
--
2.41.0