diff mbox series

[v2,1/2] Bluetooth: hci_conn: Fix not cleaning up on LE Connection failure

Message ID 20230327205347.51568-1-luiz.dentz@gmail.com
State New
Headers show
Series [v2,1/2] Bluetooth: hci_conn: Fix not cleaning up on LE Connection failure | expand

Commit Message

Luiz Augusto von Dentz March 27, 2023, 8:53 p.m. UTC
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

hci_connect_le_scan_cleanup shall always be invoked to cleanup the
states and re-enable passive scanning if necessary, otherwise it may
cause the pending action to stay active causing multiple attempts to
connect.

Fixes: 9b3628d79b46 ("Bluetooth: hci_sync: Cleanup hci_conn if it cannot be aborted")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/hci_conn.c | 52 +++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 28 deletions(-)

Comments

bluez.test.bot@gmail.com March 27, 2023, 9:38 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=734342

---Test result---

Test Summary:
CheckPatch                    PASS      1.92 seconds
GitLint                       PASS      0.50 seconds
SubjectPrefix                 PASS      0.15 seconds
BuildKernel                   PASS      44.56 seconds
CheckAllWarning               PASS      47.79 seconds
CheckSparse                   WARNING   52.51 seconds
CheckSmatch                   WARNING   139.93 seconds
BuildKernel32                 PASS      41.19 seconds
TestRunnerSetup               PASS      588.02 seconds
TestRunner_l2cap-tester       PASS      19.97 seconds
TestRunner_iso-tester         PASS      21.20 seconds
TestRunner_bnep-tester        PASS      6.99 seconds
TestRunner_mgmt-tester        PASS      131.45 seconds
TestRunner_rfcomm-tester      PASS      11.30 seconds
TestRunner_sco-tester         PASS      10.26 seconds
TestRunner_ioctl-tester       PASS      12.11 seconds
TestRunner_mesh-tester        PASS      8.99 seconds
TestRunner_smp-tester         PASS      9.94 seconds
TestRunner_userchan-tester    PASS      7.70 seconds
IncrementalBuild              PASS      74.35 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):


---
Regards,
Linux Bluetooth
patchwork-bot+bluetooth@kernel.org March 28, 2023, 8:30 p.m. UTC | #2
Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Mon, 27 Mar 2023 13:53:46 -0700 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> hci_connect_le_scan_cleanup shall always be invoked to cleanup the
> states and re-enable passive scanning if necessary, otherwise it may
> cause the pending action to stay active causing multiple attempts to
> connect.
> 
> [...]

Here is the summary with links:
  - [v2,1/2] Bluetooth: hci_conn: Fix not cleaning up on LE Connection failure
    https://git.kernel.org/bluetooth/bluetooth-next/c/501ba6f31a83
  - [v2,2/2] Bluetooth: Fix printing errors if LE Connection times out
    https://git.kernel.org/bluetooth/bluetooth-next/c/2731e038a76d

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 17b946f9ba31..5af3f6b011c9 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -68,7 +68,7 @@  static const struct sco_param esco_param_msbc[] = {
 };
 
 /* This function requires the caller holds hdev->lock */
-static void hci_connect_le_scan_cleanup(struct hci_conn *conn)
+static void hci_connect_le_scan_cleanup(struct hci_conn *conn, u8 status)
 {
 	struct hci_conn_params *params;
 	struct hci_dev *hdev = conn->hdev;
@@ -88,9 +88,28 @@  static void hci_connect_le_scan_cleanup(struct hci_conn *conn)
 
 	params = hci_pend_le_action_lookup(&hdev->pend_le_conns, bdaddr,
 					   bdaddr_type);
-	if (!params || !params->explicit_connect)
+	if (!params)
 		return;
 
+	if (params->conn) {
+		hci_conn_drop(params->conn);
+		hci_conn_put(params->conn);
+		params->conn = NULL;
+	}
+
+	if (!params->explicit_connect)
+		return;
+
+	/* If the status indicates successful cancellation of
+	 * the attempt (i.e. Unknown Connection Id) there's no point of
+	 * notifying failure since we'll go back to keep trying to
+	 * connect. The only exception is explicit connect requests
+	 * where a timeout + cancel does indicate an actual failure.
+	 */
+	if (status && status != HCI_ERROR_UNKNOWN_CONN_ID)
+		mgmt_connect_failed(hdev, &conn->dst, conn->type,
+				    conn->dst_type, status);
+
 	/* The connection attempt was doing scan for new RPA, and is
 	 * in scan phase. If params are not associated with any other
 	 * autoconnect action, remove them completely. If they are, just unmark
@@ -178,7 +197,7 @@  static void le_scan_cleanup(struct work_struct *work)
 	rcu_read_unlock();
 
 	if (c == conn) {
-		hci_connect_le_scan_cleanup(conn);
+		hci_connect_le_scan_cleanup(conn, 0x00);
 		hci_conn_cleanup(conn);
 	}
 
@@ -1179,31 +1198,8 @@  EXPORT_SYMBOL(hci_get_route);
 static void hci_le_conn_failed(struct hci_conn *conn, u8 status)
 {
 	struct hci_dev *hdev = conn->hdev;
-	struct hci_conn_params *params;
 
-	params = hci_pend_le_action_lookup(&hdev->pend_le_conns, &conn->dst,
-					   conn->dst_type);
-	if (params && params->conn) {
-		hci_conn_drop(params->conn);
-		hci_conn_put(params->conn);
-		params->conn = NULL;
-	}
-
-	/* If the status indicates successful cancellation of
-	 * the attempt (i.e. Unknown Connection Id) there's no point of
-	 * notifying failure since we'll go back to keep trying to
-	 * connect. The only exception is explicit connect requests
-	 * where a timeout + cancel does indicate an actual failure.
-	 */
-	if (status != HCI_ERROR_UNKNOWN_CONN_ID ||
-	    (params && params->explicit_connect))
-		mgmt_connect_failed(hdev, &conn->dst, conn->type,
-				    conn->dst_type, status);
-
-	/* Since we may have temporarily stopped the background scanning in
-	 * favor of connection establishment, we should restart it.
-	 */
-	hci_update_passive_scan(hdev);
+	hci_connect_le_scan_cleanup(conn, status);
 
 	/* Enable advertising in case this was a failed connection
 	 * attempt as a peripheral.
@@ -1240,7 +1236,7 @@  static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
 	hci_dev_lock(hdev);
 
 	if (!err) {
-		hci_connect_le_scan_cleanup(conn);
+		hci_connect_le_scan_cleanup(conn, 0x00);
 		goto done;
 	}