2023-08-01 16:55:42

by Pauli Virtanen

[permalink] [raw]
Subject: [PATCH BlueZ v2 1/5] btdev: check error conditions for HCI_Create_Connection_Cancel

Create Connection Cancel shall return Command Complete with error status
when there is no Create Connection that can be canceled. In these
cases, we should not send a (spurious) Connection Complete event.

Fix by keeping a list of pending Create Connection commands, and
returning command errors if there is none pending at the moment.
---

Notes:
v2: emit Command_Complete (not Status) + fix compile

emulator/btdev.c | 86 +++++++++++++++++++++++++++++++++++++++++-------
monitor/bt.h | 4 +++
2 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index 637f0bb98..8658b4121 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -62,6 +62,7 @@ struct hook {

#define MAX_HOOK_ENTRIES 16
#define MAX_EXT_ADV_SETS 3
+#define MAX_PENDING_CONN 16

struct btdev_conn {
uint16_t handle;
@@ -223,6 +224,8 @@ struct btdev {
uint8_t le_rl_enable;
uint16_t le_rl_timeout;

+ struct btdev *pending_conn[MAX_PENDING_CONN];
+
uint8_t le_local_sk256[32];

uint16_t sync_train_interval;
@@ -1211,10 +1214,36 @@ static struct btdev_conn *conn_link_bis(struct btdev *dev, struct btdev *remote,
return conn;
}

+static void pending_conn_add(struct btdev *btdev, struct btdev *remote)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(btdev->pending_conn); ++i) {
+ if (!btdev->pending_conn[i]) {
+ btdev->pending_conn[i] = remote;
+ return;
+ }
+ }
+}
+
+static bool pending_conn_del(struct btdev *btdev, struct btdev *remote)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(btdev->pending_conn); ++i) {
+ if (btdev->pending_conn[i] == remote) {
+ btdev->pending_conn[i] = NULL;
+ return true;
+ }
+ }
+ return false;
+}
+
static void conn_complete(struct btdev *btdev,
const uint8_t *bdaddr, uint8_t status)
{
struct bt_hci_evt_conn_complete cc;
+ struct btdev *remote = find_btdev_by_bdaddr(bdaddr);

if (!status) {
struct btdev_conn *conn;
@@ -1223,6 +1252,8 @@ static void conn_complete(struct btdev *btdev,
if (!conn)
return;

+ pending_conn_del(conn->link->dev, btdev);
+
cc.status = status;
memcpy(cc.bdaddr, btdev->bdaddr, 6);
cc.encr_mode = 0x00;
@@ -1240,6 +1271,8 @@ static void conn_complete(struct btdev *btdev,
cc.link_type = 0x01;
}

+ pending_conn_del(btdev, remote);
+
cc.status = status;
memcpy(cc.bdaddr, bdaddr, 6);
cc.encr_mode = 0x00;
@@ -1260,6 +1293,8 @@ static int cmd_create_conn_complete(struct btdev *dev, const void *data,
memcpy(cr.dev_class, dev->dev_class, 3);
cr.link_type = 0x01;

+ pending_conn_add(dev, remote);
+
send_event(remote, BT_HCI_EVT_CONN_REQUEST, &cr, sizeof(cr));
} else {
conn_complete(dev, cmd->bdaddr, BT_HCI_ERR_PAGE_TIMEOUT);
@@ -1296,16 +1331,24 @@ static int cmd_add_sco_conn(struct btdev *dev, const void *data, uint8_t len)
cc.encr_mode = 0x00;

done:
+ pending_conn_del(dev, conn->link->dev);
+
send_event(dev, BT_HCI_EVT_CONN_COMPLETE, &cc, sizeof(cc));

return 0;
}

+static bool match_bdaddr(const void *data, const void *match_data)
+{
+ const struct btdev_conn *conn = data;
+ const uint8_t *bdaddr = match_data;
+
+ return !memcmp(conn->link->dev->bdaddr, bdaddr, 6);
+}
+
static int cmd_create_conn_cancel(struct btdev *dev, const void *data,
uint8_t len)
{
- cmd_status(dev, BT_HCI_ERR_SUCCESS, BT_HCI_CMD_CREATE_CONN_CANCEL);
-
return 0;
}

@@ -1313,8 +1356,37 @@ static int cmd_create_conn_cancel_complete(struct btdev *dev, const void *data,
uint8_t len)
{
const struct bt_hci_cmd_create_conn_cancel *cmd = data;
+ struct bt_hci_rsp_create_conn_cancel rp;
+ struct btdev *remote = find_btdev_by_bdaddr(cmd->bdaddr);
+ struct btdev_conn *conn;

- conn_complete(dev, cmd->bdaddr, BT_HCI_ERR_UNKNOWN_CONN_ID);
+ /* BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 4, Part E page 1848
+ *
+ * If the connection is already established, and the
+ * HCI_Connection_Complete event has been sent, then the Controller
+ * shall return an HCI_Command_Complete event with the error code
+ * Connection Already Exists (0x0B). If the HCI_Create_Connection_Cancel
+ * command is sent to the Controller without a preceding
+ * HCI_Create_Connection command to the same device, the BR/EDR
+ * Controller shall return an HCI_Command_Complete event with the error
+ * code Unknown Connection Identifier (0x02).
+ */
+ if (pending_conn_del(dev, remote)) {
+ rp.status = BT_HCI_ERR_SUCCESS;
+ } else {
+ conn = queue_find(dev->conns, match_bdaddr, cmd->bdaddr);
+ if (conn)
+ rp.status = BT_HCI_ERR_CONN_ALREADY_EXISTS;
+ else
+ rp.status = BT_HCI_ERR_UNKNOWN_CONN_ID;
+ }
+
+ memcpy(rp.bdaddr, cmd->bdaddr, sizeof(rp.bdaddr));
+
+ cmd_complete(dev, BT_HCI_CMD_CREATE_CONN_CANCEL, &rp, sizeof(rp));
+
+ if (!rp.status)
+ conn_complete(dev, cmd->bdaddr, BT_HCI_ERR_UNKNOWN_CONN_ID);

return 0;
}
@@ -1372,14 +1444,6 @@ static int cmd_link_key_reply(struct btdev *dev, const void *data, uint8_t len)
return 0;
}

-static bool match_bdaddr(const void *data, const void *match_data)
-{
- const struct btdev_conn *conn = data;
- const uint8_t *bdaddr = match_data;
-
- return !memcmp(conn->link->dev->bdaddr, bdaddr, 6);
-}
-
static void auth_complete(struct btdev_conn *conn, uint8_t status)
{
struct bt_hci_evt_auth_complete ev;
diff --git a/monitor/bt.h b/monitor/bt.h
index dca2dc8b8..6fb81abfe 100644
--- a/monitor/bt.h
+++ b/monitor/bt.h
@@ -590,6 +590,10 @@ struct bt_hci_cmd_add_sco_conn {
struct bt_hci_cmd_create_conn_cancel {
uint8_t bdaddr[6];
} __attribute__ ((packed));
+struct bt_hci_rsp_create_conn_cancel {
+ uint8_t status;
+ uint8_t bdaddr[6];
+} __attribute__ ((packed));

#define BT_HCI_CMD_ACCEPT_CONN_REQUEST 0x0409
struct bt_hci_cmd_accept_conn_request {
--
2.41.0



2023-08-01 19:19:36

by bluez.test.bot

[permalink] [raw]
Subject: RE: Additional tests for ISO and hci_sync

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

---Test result---

Test Summary:
CheckPatch FAIL 3.01 seconds
GitLint FAIL 1.95 seconds
BuildEll PASS 26.83 seconds
BluezMake PASS 785.29 seconds
MakeCheck PASS 12.26 seconds
MakeDistcheck PASS 156.24 seconds
CheckValgrind PASS 251.21 seconds
CheckSmatch WARNING 341.55 seconds
bluezmakeextell PASS 109.60 seconds
IncrementalBuild PASS 3312.84 seconds
ScanBuild WARNING 1019.24 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,v2,1/5] btdev: check error conditions for HCI_Create_Connection_Cancel
WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#277: FILE: monitor/bt.h:596:
+} __attribute__ ((packed));

/github/workspace/src/src/13337000.patch total: 0 errors, 1 warnings, 163 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/13337000.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

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:
[BlueZ,v2,3/5] sco-tester: test local and remote disconnecting simultaneously

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
6: B3 Line contains hard tab characters (\t): " [controller] > HCI Synchronous Connect Complete"
7: B3 Line contains hard tab characters (\t): " [controller] > HCI Disconnection Complete (from remote)"
8: B3 Line contains hard tab characters (\t): " [user] shutdown(sco_socket)"
9: B3 Line contains hard tab characters (\t): " [kernel] hci_conn_abort(SCO handle)"
10: B3 Line contains hard tab characters (\t): " [kernel] > HCI Create Connection Cancel"
11: B3 Line contains hard tab characters (\t): " [kernel] < HCI Synchronous Connect Complete"
12: B3 Line contains hard tab characters (\t): " [kernel] < HCI Disconnect Complete"
13: B3 Line contains hard tab characters (\t): " [controller] < HCI Create Connection Cancel"
14: B3 Line contains hard tab characters (\t): " [controller] > HCI Command Status (Create Connection Cancel)"
15: B3 Line contains hard tab characters (\t): " [kernel] < HCI Command Status (Create Connection Cancel)"
31: B2 Line has trailing whitespace: " "
35: B2 Line has trailing whitespace: " "
36: B1 Line exceeds max length (84>80): " CPU: 0 PID: 35 Comm: kworker/u3:2 Not tainted 6.5.0-rc1-00520-gf57f797eebfe #152"
37: B1 Line exceeds max length (85>80): " Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014"
64: B2 Line has trailing whitespace: " "
76: B2 Line has trailing whitespace: " "
92: B2 Line has trailing whitespace: " "
105: B2 Line has trailing whitespace: " "
110: B2 Line has trailing whitespace: " "
112: B1 Line exceeds max length (93>80): " page:ffffea00000a7800 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x29e0"
119: B2 Line has trailing whitespace: " "
[BlueZ,v2,4/5] iso-tester: test with large CIS_ID and invalid CIG_ID/CIS_ID

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
19: B2 Line has trailing whitespace: " "
21: B2 Line has trailing whitespace: " "
22: B1 Line exceeds max length (83>80): " ISO QoS CIG 0xF0 - Invalid Timed out 2.301 seconds"
23: B1 Line exceeds max length (83>80): " ISO QoS CIS 0xF0 - Invalid Failed 0.117 seconds"
24: B1 Line exceeds max length (83>80): " ISO Connect2 CIG 0x01 - Success/Invalid Failed 0.189 seconds"
25: B1 Line exceeds max length (83>80): " ISO AC 6(ii) CIS 0xEF/auto - Success Failed 0.196 seconds"
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
emulator/btdev.c:420:29: warning: Variable length array is used.emulator/btdev.c:420:29: warning: Variable length array is used.tools/sco-tester.c: note: in included file:./lib/bluetooth.h:216:15: warning: array of flexible structures./lib/bluetooth.h:221:31: warning: array of flexible structures
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
emulator/btdev.c:1083:10: warning: Although the value stored to 'conn' is used in the enclosing expression, the value is never actually read from 'conn'
while ((conn = queue_find(dev->conns, match_handle,
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
emulator/btdev.c:1334:24: warning: Access to field 'link' results in a dereference of a null pointer (loaded from variable 'conn')
pending_conn_del(dev, conn->link->dev);
^~~~~~~~~~
emulator/btdev.c:1456:13: warning: Access to field 'dev' results in a dereference of a null pointer (loaded from variable 'conn')
send_event(conn->dev, BT_HCI_EVT_AUTH_COMPLETE, &ev, sizeof(ev));
^~~~~~~~~
3 warnings generated.



---
Regards,
Linux Bluetooth