2023-08-05 16:27:05

by Pauli Virtanen

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: hci_event: drop only unbound CIS if Set CIG Parameters fails

When user tries to connect a new CIS when its CIG is not configurable,
that connection shall fail, but pre-existing connections shall not be
affected. However, currently hci_cc_le_set_cig_params deletes all CIS
of the CIG on error so it doesn't work, even though controller shall not
change CIG/CIS configuration if the command fails.

Fix by failing on command error only the connections that are not yet
bound, so that we keep the previous CIS configuration like the
controller does.

Fixes: 26afbd826ee3 ("Bluetooth: Add initial implementation of CIS connections")
Signed-off-by: Pauli Virtanen <[email protected]>
---
net/bluetooth/hci_event.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 218da9b0fe8f..559b6080706c 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3799,6 +3799,22 @@ static u8 hci_cc_le_read_buffer_size_v2(struct hci_dev *hdev, void *data,
return rp->status;
}

+static void hci_unbound_cis_failed(struct hci_dev *hdev, u8 cig, u8 status)
+{
+ struct hci_conn *conn, *tmp;
+
+ lockdep_assert_held(&hdev->lock);
+
+ list_for_each_entry_safe(conn, tmp, &hdev->conn_hash.list, list) {
+ if (conn->type != ISO_LINK || !bacmp(&conn->dst, BDADDR_ANY) ||
+ conn->state == BT_OPEN || conn->iso_qos.ucast.cig != cig)
+ continue;
+
+ if (HCI_CONN_HANDLE_UNSET(conn->handle))
+ hci_conn_failed(conn, status);
+ }
+}
+
static u8 hci_cc_le_set_cig_params(struct hci_dev *hdev, void *data,
struct sk_buff *skb)
{
@@ -3820,12 +3836,15 @@ static u8 hci_cc_le_set_cig_params(struct hci_dev *hdev, void *data,

hci_dev_lock(hdev);

+ /* BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 4, Part E page 2554
+ *
+ * If the Status return parameter is non-zero, then the state of the CIG
+ * and its CIS configurations shall not be changed by the command. If
+ * the CIG did not already exist, it shall not be created.
+ */
if (status) {
- while ((conn = hci_conn_hash_lookup_cig(hdev, rp->cig_id))) {
- conn->state = BT_CLOSED;
- hci_connect_cfm(conn, status);
- hci_conn_del(conn);
- }
+ /* Keep current configuration, fail only the unbound CIS */
+ hci_unbound_cis_failed(hdev, rp->cig_id, status);
goto unlock;
}

--
2.41.0



2023-08-05 16:52:46

by bluez.test.bot

[permalink] [raw]
Subject: RE: [1/2] Bluetooth: hci_event: drop only unbound CIS if Set CIG Parameters fails

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=773373

---Test result---

Test Summary:
CheckPatch PASS 1.43 seconds
GitLint PASS 0.51 seconds
SubjectPrefix PASS 0.13 seconds
BuildKernel PASS 39.69 seconds
CheckAllWarning PASS 43.65 seconds
CheckSparse WARNING 49.58 seconds
CheckSmatch WARNING 132.40 seconds
BuildKernel32 PASS 38.51 seconds
TestRunnerSetup PASS 579.52 seconds
TestRunner_l2cap-tester PASS 27.76 seconds
TestRunner_iso-tester PASS 68.79 seconds
TestRunner_bnep-tester PASS 12.67 seconds
TestRunner_mgmt-tester FAIL 252.38 seconds
TestRunner_rfcomm-tester PASS 18.87 seconds
TestRunner_sco-tester PASS 21.89 seconds
TestRunner_ioctl-tester PASS 21.13 seconds
TestRunner_mesh-tester PASS 15.87 seconds
TestRunner_smp-tester PASS 17.16 seconds
TestRunner_userchan-tester PASS 13.29 seconds
IncrementalBuild PASS 68.10 seconds

Details
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 497, Passed: 496 (99.8%), Failed: 1, Not Run: 0

Failed Test Cases
LL Privacy - Unpair 1 Timed out 1.924 seconds


---
Regards,
Linux Bluetooth

2023-08-08 00:02:46

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH 1/2] Bluetooth: hci_event: drop only unbound CIS if Set CIG Parameters fails

Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Sat, 5 Aug 2023 19:08:41 +0300 you wrote:
> When user tries to connect a new CIS when its CIG is not configurable,
> that connection shall fail, but pre-existing connections shall not be
> affected. However, currently hci_cc_le_set_cig_params deletes all CIS
> of the CIG on error so it doesn't work, even though controller shall not
> change CIG/CIS configuration if the command fails.
>
> Fix by failing on command error only the connections that are not yet
> bound, so that we keep the previous CIS configuration like the
> controller does.
>
> [...]

Here is the summary with links:
- [1/2] Bluetooth: hci_event: drop only unbound CIS if Set CIG Parameters fails
https://git.kernel.org/bluetooth/bluetooth-next/c/f5669a036ae6
- [2/2] Bluetooth: hci_conn: avoid checking uninitialized CIG/CIS ids
https://git.kernel.org/bluetooth/bluetooth-next/c/6130cdd83d40

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html