diff mbox series

[BlueZ,v2,1/5] btdev: check error conditions for HCI_Create_Connection_Cancel

Message ID 5eca51146b08a512052261b88ae5c8a7437af5fc.1690907478.git.pav@iki.fi (mailing list archive)
State Accepted
Commit 5815a92423c3d074e26974f4ca5de0cc4b596845
Headers show
Series Additional tests for ISO and hci_sync | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch warning 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.
tedd_an/GitLint success Gitlint PASS
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch warning CheckSparse WARNING 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
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/IncrementalBuild success Incremental Build PASS
tedd_an/ScanBuild warning ScanBuild: 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.

Commit Message

Pauli Virtanen Aug. 1, 2023, 4:38 p.m. UTC
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(-)

Comments

bluez.test.bot@gmail.com Aug. 1, 2023, 6:53 p.m. UTC | #1
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
diff mbox series

Patch

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 {