From: Luiz Augusto von Dentz <[email protected]>
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 <[email protected]>
---
net/bluetooth/hci_conn.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 17b946f9ba31..0cd339ba7858 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -88,7 +88,16 @@ 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;
/* The connection attempt was doing scan for new RPA, and is
@@ -1181,13 +1190,7 @@ 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;
- }
+ hci_connect_le_scan_cleanup(conn);
/* If the status indicates successful cancellation of
* the attempt (i.e. Unknown Connection Id) there's no point of
@@ -1200,11 +1203,6 @@ static void hci_le_conn_failed(struct hci_conn *conn, u8 status)
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);
-
/* Enable advertising in case this was a failed connection
* attempt as a peripheral.
*/
--
2.39.2
From: Luiz Augusto von Dentz <[email protected]>
This fixes errors like bellow when LE Connection times out since that
is actually not a controller error:
Bluetooth: hci0: Opcode 0x200d failed: -110
Bluetooth: hci0: request failed to create LE connection: err -110
Instead the code shall properly detect if -ETIMEDOUT is returned and
send HCI_OP_LE_CREATE_CONN_CANCEL to give up on the connection.
Link: https://github.com/bluez/bluez/issues/340
Fixes: Fixes: 8e8b92ee60de ("Bluetooth: hci_sync: Add hci_le_create_conn_sync")
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 7 +++++--
net/bluetooth/hci_event.c | 16 ++++++----------
net/bluetooth/hci_sync.c | 13 ++++++++++---
4 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 6ed9b4d546a7..d5311ceb21c6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -954,6 +954,7 @@ enum {
HCI_CONN_STK_ENCRYPT,
HCI_CONN_AUTH_INITIATOR,
HCI_CONN_DROP,
+ HCI_CONN_CANCEL,
HCI_CONN_PARAM_REMOVAL_PEND,
HCI_CONN_NEW_LINK_KEY,
HCI_CONN_SCANNING,
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0cd339ba7858..a51a6cfd5687 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1235,6 +1235,8 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
{
struct hci_conn *conn = data;
+ bt_dev_dbg(hdev, "err %d", err);
+
hci_dev_lock(hdev);
if (!err) {
@@ -1242,8 +1244,6 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
goto done;
}
- bt_dev_err(hdev, "request failed to create LE connection: err %d", err);
-
/* Check if connection is still pending */
if (conn != hci_lookup_le_connect(hdev))
goto done;
@@ -2773,6 +2773,9 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
{
int r = 0;
+ if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags))
+ return 0;
+
switch (conn->state) {
case BT_CONNECTED:
case BT_CONFIG:
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ad92a4be5851..e68f2a7d863a 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2881,16 +2881,6 @@ static void cs_le_create_conn(struct hci_dev *hdev, bdaddr_t *peer_addr,
conn->resp_addr_type = peer_addr_type;
bacpy(&conn->resp_addr, peer_addr);
-
- /* We don't want the connection attempt to stick around
- * indefinitely since LE doesn't have a page timeout concept
- * like BR/EDR. Set a timer for any connection that doesn't use
- * the accept list for connecting.
- */
- if (filter_policy == HCI_LE_USE_PEER_ADDR)
- queue_delayed_work(conn->hdev->workqueue,
- &conn->le_conn_timeout,
- conn->conn_timeout);
}
static void hci_cs_le_create_conn(struct hci_dev *hdev, u8 status)
@@ -5902,6 +5892,12 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
if (status)
goto unlock;
+ /* Drop the connection if it has been aborted */
+ if (test_bit(HCI_CONN_CANCEL, &conn->flags)) {
+ hci_conn_drop(conn);
+ goto unlock;
+ }
+
if (conn->dst_type == ADDR_LE_DEV_PUBLIC)
addr_type = BDADDR_LE_PUBLIC;
else
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 5a6aa1627791..632be1267288 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -246,8 +246,9 @@ int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
skb = __hci_cmd_sync_sk(hdev, opcode, plen, param, event, timeout, sk);
if (IS_ERR(skb)) {
- bt_dev_err(hdev, "Opcode 0x%4x failed: %ld", opcode,
- PTR_ERR(skb));
+ if (!event)
+ bt_dev_err(hdev, "Opcode 0x%4x failed: %ld", opcode,
+ PTR_ERR(skb));
return PTR_ERR(skb);
}
@@ -5126,8 +5127,11 @@ static int hci_le_connect_cancel_sync(struct hci_dev *hdev,
if (test_bit(HCI_CONN_SCANNING, &conn->flags))
return 0;
+ if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags))
+ return 0;
+
return __hci_cmd_sync_status(hdev, HCI_OP_LE_CREATE_CONN_CANCEL,
- 6, &conn->dst, HCI_CMD_TIMEOUT);
+ 0, NULL, HCI_CMD_TIMEOUT);
}
static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn)
@@ -6102,6 +6106,9 @@ 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);
+
/* Re-enable advertising after the connection attempt is finished. */
hci_resume_advertising_sync(hdev);
return err;
--
2.39.2
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=733699
---Test result---
Test Summary:
CheckPatch PASS 2.34 seconds
GitLint PASS 0.77 seconds
SubjectPrefix PASS 0.28 seconds
BuildKernel PASS 44.46 seconds
CheckAllWarning PASS 48.86 seconds
CheckSparse WARNING 55.16 seconds
CheckSmatch WARNING 146.37 seconds
BuildKernel32 PASS 43.09 seconds
TestRunnerSetup PASS 612.31 seconds
TestRunner_l2cap-tester PASS 20.91 seconds
TestRunner_iso-tester PASS 22.25 seconds
TestRunner_bnep-tester PASS 7.47 seconds
TestRunner_mgmt-tester PASS 136.93 seconds
TestRunner_rfcomm-tester PASS 11.60 seconds
TestRunner_sco-tester PASS 10.57 seconds
TestRunner_ioctl-tester PASS 12.56 seconds
TestRunner_mesh-tester PASS 9.31 seconds
TestRunner_smp-tester PASS 10.44 seconds
TestRunner_userchan-tester PASS 7.80 seconds
IncrementalBuild PASS 76.85 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
Hi Luiz,
I love your patch! Perhaps something to improve:
[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on bluetooth/master linus/master v6.3-rc3 next-20230324]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Luiz-Augusto-von-Dentz/Bluetooth-Fix-printing-errors-if-LE-Connection-times-out/20230325-044559
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
patch link: https://lore.kernel.org/r/20230324204525.3630188-1-luiz.dentz%40gmail.com
patch subject: [PATCH 1/2] Bluetooth: hci_conn: Fix not cleaning up on LE Connection failure
config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20230325/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/e15b8378a4f3221972483cf6bb52f0649341a55e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Luiz-Augusto-von-Dentz/Bluetooth-Fix-printing-errors-if-LE-Connection-times-out/20230325-044559
git checkout e15b8378a4f3221972483cf6bb52f0649341a55e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash net/bluetooth/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> net/bluetooth/hci_conn.c:1202:7: warning: variable 'params' is uninitialized when used here [-Wuninitialized]
(params && params->explicit_connect))
^~~~~~
net/bluetooth/hci_conn.c:1191:32: note: initialize the variable 'params' to silence this warning
struct hci_conn_params *params;
^
= NULL
1 warning generated.
vim +/params +1202 net/bluetooth/hci_conn.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 1186
9bb3c01fdb2201 Andre Guedes 2014-01-30 1187 /* This function requires the caller holds hdev->lock */
9b3628d79b46f0 Luiz Augusto von Dentz 2022-04-22 1188 static void hci_le_conn_failed(struct hci_conn *conn, u8 status)
9bb3c01fdb2201 Andre Guedes 2014-01-30 1189 {
9bb3c01fdb2201 Andre Guedes 2014-01-30 1190 struct hci_dev *hdev = conn->hdev;
f161dd4122ffa7 Johan Hedberg 2014-08-15 1191 struct hci_conn_params *params;
f161dd4122ffa7 Johan Hedberg 2014-08-15 1192
e15b8378a4f322 Luiz Augusto von Dentz 2023-03-24 1193 hci_connect_le_scan_cleanup(conn);
9bb3c01fdb2201 Andre Guedes 2014-01-30 1194
acb9f911ea1f82 Johan Hedberg 2015-12-03 1195 /* If the status indicates successful cancellation of
91641b79e1e153 Zheng Yongjun 2021-06-02 1196 * the attempt (i.e. Unknown Connection Id) there's no point of
acb9f911ea1f82 Johan Hedberg 2015-12-03 1197 * notifying failure since we'll go back to keep trying to
acb9f911ea1f82 Johan Hedberg 2015-12-03 1198 * connect. The only exception is explicit connect requests
acb9f911ea1f82 Johan Hedberg 2015-12-03 1199 * where a timeout + cancel does indicate an actual failure.
acb9f911ea1f82 Johan Hedberg 2015-12-03 1200 */
acb9f911ea1f82 Johan Hedberg 2015-12-03 1201 if (status != HCI_ERROR_UNKNOWN_CONN_ID ||
acb9f911ea1f82 Johan Hedberg 2015-12-03 @1202 (params && params->explicit_connect))
acb9f911ea1f82 Johan Hedberg 2015-12-03 1203 mgmt_connect_failed(hdev, &conn->dst, conn->type,
acb9f911ea1f82 Johan Hedberg 2015-12-03 1204 conn->dst_type, status);
9bb3c01fdb2201 Andre Guedes 2014-01-30 1205
abfeea476c68af Luiz Augusto von Dentz 2021-10-27 1206 /* Enable advertising in case this was a failed connection
3c857757ef6e5a Johan Hedberg 2014-03-25 1207 * attempt as a peripheral.
3c857757ef6e5a Johan Hedberg 2014-03-25 1208 */
abfeea476c68af Luiz Augusto von Dentz 2021-10-27 1209 hci_enable_advertising(hdev);
9bb3c01fdb2201 Andre Guedes 2014-01-30 1210 }
9bb3c01fdb2201 Andre Guedes 2014-01-30 1211
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi Luiz,
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Luiz-Augusto-von-Dentz/Bluetooth-Fix-printing-errors-if-LE-Connection-times-out/20230325-044559
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
patch link: https://lore.kernel.org/r/20230324204525.3630188-1-luiz.dentz%40gmail.com
patch subject: [PATCH 1/2] Bluetooth: hci_conn: Fix not cleaning up on LE Connection failure
config: riscv-randconfig-m031-20230326 (https://download.01.org/0day-ci/archive/20230327/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]/
smatch warnings:
net/bluetooth/hci_conn.c:1202 hci_le_conn_failed() error: uninitialized symbol 'params'.
vim +/params +1202 net/bluetooth/hci_conn.c
9b3628d79b46f0 Luiz Augusto von Dentz 2022-04-22 1188 static void hci_le_conn_failed(struct hci_conn *conn, u8 status)
9bb3c01fdb2201 Andre Guedes 2014-01-30 1189 {
9bb3c01fdb2201 Andre Guedes 2014-01-30 1190 struct hci_dev *hdev = conn->hdev;
f161dd4122ffa7 Johan Hedberg 2014-08-15 1191 struct hci_conn_params *params;
f161dd4122ffa7 Johan Hedberg 2014-08-15 1192
e15b8378a4f322 Luiz Augusto von Dentz 2023-03-24 1193 hci_connect_le_scan_cleanup(conn);
9bb3c01fdb2201 Andre Guedes 2014-01-30 1194
acb9f911ea1f82 Johan Hedberg 2015-12-03 1195 /* If the status indicates successful cancellation of
91641b79e1e153 Zheng Yongjun 2021-06-02 1196 * the attempt (i.e. Unknown Connection Id) there's no point of
acb9f911ea1f82 Johan Hedberg 2015-12-03 1197 * notifying failure since we'll go back to keep trying to
acb9f911ea1f82 Johan Hedberg 2015-12-03 1198 * connect. The only exception is explicit connect requests
acb9f911ea1f82 Johan Hedberg 2015-12-03 1199 * where a timeout + cancel does indicate an actual failure.
acb9f911ea1f82 Johan Hedberg 2015-12-03 1200 */
acb9f911ea1f82 Johan Hedberg 2015-12-03 1201 if (status != HCI_ERROR_UNKNOWN_CONN_ID ||
acb9f911ea1f82 Johan Hedberg 2015-12-03 @1202 (params && params->explicit_connect))
^^^^^^
params is uninitialized
acb9f911ea1f82 Johan Hedberg 2015-12-03 1203 mgmt_connect_failed(hdev, &conn->dst, conn->type,
acb9f911ea1f82 Johan Hedberg 2015-12-03 1204 conn->dst_type, status);
9bb3c01fdb2201 Andre Guedes 2014-01-30 1205
abfeea476c68af Luiz Augusto von Dentz 2021-10-27 1206 /* Enable advertising in case this was a failed connection
3c857757ef6e5a Johan Hedberg 2014-03-25 1207 * attempt as a peripheral.
3c857757ef6e5a Johan Hedberg 2014-03-25 1208 */
abfeea476c68af Luiz Augusto von Dentz 2021-10-27 1209 hci_enable_advertising(hdev);
9bb3c01fdb2201 Andre Guedes 2014-01-30 1210 }
9bb3c01fdb2201 Andre Guedes 2014-01-30 1211
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests