Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF
@ 2015-10-21 15:02 Johan Hedberg
  2015-10-21 15:02 ` [PATCH 01/13] Bluetooth: Add le_addr_type() helper function Johan Hedberg
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:02 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

Here's the left-over set of patches from the UPF which are not as
critical as the other ones that were already applied. I didn't (yet)
spend time sorting or splitting these more logically, but I decided to
send what I have now to get an early review round for them.

Johan

----------------------------------------------------------------
Johan Hedberg (13):
      Bluetooth: Add le_addr_type() helper function
      Bluetooth: Add hci_conn_hash_lookup_le() helper function
      Bluetooth: Use hci_conn_hash_lookup_le() when possible
      Bluetooth: 6lowpan: Use hci_conn_hash_lookup_le() when possible
      Bluetooth: Remove unnecessary indentation in unpair_device()
      Bluetooth: Disable auto-connection parameters when unpairing
      Bluetooth: Introduce hci_req helper to abort a connection
      Bluetooth: Take advantage of connection abort helpers
      Bluetooth: Remove redundant (and possibly wrong) flag clearing
      Bluetooth: Make hci_disconnect() behave correctly for all states
      Bluetooth: Add hdev helper variable to hci_le_create_connection_cancel
      Bluetooth: Remove unnecessary hci_explicit_connect_lookup function
      Bluetooth: Fix crash in SMP when unpairing

 include/net/bluetooth/hci_core.h |  27 ++++++-
 net/bluetooth/6lowpan.c          |   2 +-
 net/bluetooth/hci_conn.c         | 115 ++++++--------------------
 net/bluetooth/hci_core.c         |  17 ----
 net/bluetooth/hci_event.c        |   3 +-
 net/bluetooth/hci_request.c      |  86 ++++++++++++++++++++
 net/bluetooth/hci_request.h      |   4 +
 net/bluetooth/mgmt.c             | 164 ++++++++++++++++----------------------
 net/bluetooth/smp.c              |  27 ++++++-
 net/bluetooth/smp.h              |   1 +
 10 files changed, 236 insertions(+), 210 deletions(-)



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 01/13] Bluetooth: Add le_addr_type() helper function
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
@ 2015-10-21 15:02 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 02/13] Bluetooth: Add hci_conn_hash_lookup_le() " Johan Hedberg
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:02 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

The mgmt code needs to convert from mgmt/L2CAP address types to HCI in
many places. Having a dedicated helper function for this simplifies
code by shortening it and removing unnecessary 'addr_type' variables.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 56 +++++++++++++++++-----------------------------------
 1 file changed, 18 insertions(+), 38 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 98d8d20a05c1..de338d576907 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -268,6 +268,14 @@ static int mgmt_event(u16 event, struct hci_dev *hdev, void *data, u16 len,
 			       HCI_SOCK_TRUSTED, skip_sk);
 }
 
+static u8 le_addr_type(u8 mgmt_addr_type)
+{
+	if (mgmt_addr_type == BDADDR_LE_PUBLIC)
+		return ADDR_LE_DEV_PUBLIC;
+	else
+		return ADDR_LE_DEV_RANDOM;
+}
+
 static int read_version(struct sock *sk, struct hci_dev *hdev, void *data,
 			u16 data_len)
 {
@@ -3088,12 +3096,7 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 
 		err = hci_remove_link_key(hdev, &cp->addr.bdaddr);
 	} else {
-		u8 addr_type;
-
-		if (cp->addr.type == BDADDR_LE_PUBLIC)
-			addr_type = ADDR_LE_DEV_PUBLIC;
-		else
-			addr_type = ADDR_LE_DEV_RANDOM;
+		u8 addr_type = le_addr_type(cp->addr.type);
 
 		conn = hci_conn_hash_lookup_ba(hdev, LE_LINK,
 					       &cp->addr.bdaddr);
@@ -3546,16 +3549,9 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 		conn = hci_connect_acl(hdev, &cp->addr.bdaddr, sec_level,
 				       auth_type);
 	} else {
-		u8 addr_type;
+		u8 addr_type = le_addr_type(cp->addr.type);
 		struct hci_conn_params *p;
 
-		/* Convert from L2CAP channel address type to HCI address type
-		 */
-		if (cp->addr.type == BDADDR_LE_PUBLIC)
-			addr_type = ADDR_LE_DEV_PUBLIC;
-		else
-			addr_type = ADDR_LE_DEV_RANDOM;
-
 		/* When pairing a new device, it is expected to remember
 		 * this device for future connections. Adding the connection
 		 * parameter information ahead of time allows tracking
@@ -5602,14 +5598,9 @@ static int load_irks(struct sock *sk, struct hci_dev *hdev, void *cp_data,
 
 	for (i = 0; i < irk_count; i++) {
 		struct mgmt_irk_info *irk = &cp->irks[i];
-		u8 addr_type;
 
-		if (irk->addr.type == BDADDR_LE_PUBLIC)
-			addr_type = ADDR_LE_DEV_PUBLIC;
-		else
-			addr_type = ADDR_LE_DEV_RANDOM;
-
-		hci_add_irk(hdev, &irk->addr.bdaddr, addr_type, irk->val,
+		hci_add_irk(hdev, &irk->addr.bdaddr,
+			    le_addr_type(irk->addr.type), irk->val,
 			    BDADDR_ANY);
 	}
 
@@ -5689,12 +5680,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
 
 	for (i = 0; i < key_count; i++) {
 		struct mgmt_ltk_info *key = &cp->keys[i];
-		u8 type, addr_type, authenticated;
-
-		if (key->addr.type == BDADDR_LE_PUBLIC)
-			addr_type = ADDR_LE_DEV_PUBLIC;
-		else
-			addr_type = ADDR_LE_DEV_RANDOM;
+		u8 type, authenticated;
 
 		switch (key->type) {
 		case MGMT_LTK_UNAUTHENTICATED:
@@ -5720,9 +5706,9 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
 			continue;
 		}
 
-		hci_add_ltk(hdev, &key->addr.bdaddr, addr_type, type,
-			    authenticated, key->val, key->enc_size, key->ediv,
-			    key->rand);
+		hci_add_ltk(hdev, &key->addr.bdaddr,
+			    le_addr_type(key->addr.type), type, authenticated,
+			    key->val, key->enc_size, key->ediv, key->rand);
 	}
 
 	err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_LOAD_LONG_TERM_KEYS, 0,
@@ -6234,10 +6220,7 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
 		goto added;
 	}
 
-	if (cp->addr.type == BDADDR_LE_PUBLIC)
-		addr_type = ADDR_LE_DEV_PUBLIC;
-	else
-		addr_type = ADDR_LE_DEV_RANDOM;
+	addr_type = le_addr_type(cp->addr.type);
 
 	if (cp->action == 0x02)
 		auto_conn = HCI_AUTO_CONN_ALWAYS;
@@ -6366,10 +6349,7 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
 			goto complete;
 		}
 
-		if (cp->addr.type == BDADDR_LE_PUBLIC)
-			addr_type = ADDR_LE_DEV_PUBLIC;
-		else
-			addr_type = ADDR_LE_DEV_RANDOM;
+		addr_type = le_addr_type(cp->addr.type);
 
 		/* Kernel internally uses conn_params with resolvable private
 		 * address, but Remove Device allows only identity addresses.
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 02/13] Bluetooth: Add hci_conn_hash_lookup_le() helper function
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
  2015-10-21 15:02 ` [PATCH 01/13] Bluetooth: Add le_addr_type() helper function Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 03/13] Bluetooth: Use hci_conn_hash_lookup_le() when possible Johan Hedberg
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

Many of the existing LE connection lookups are forced to use
hci_conn_hash_lookup_ba() which doesn't take into account the address
type. What's worse, most of the users don't bother checking that the
returned address type matches what was wanted.

This patch adds a new helper API to look up LE connections based on
their address and address type, paving the way to have the
hci_conn_hash_lookup_ba() users converted to do more precise lookups.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0015d087d8b1..32bb281e6aa6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -794,6 +794,30 @@ static inline struct hci_conn *hci_conn_hash_lookup_ba(struct hci_dev *hdev,
 	return NULL;
 }
 
+static inline struct hci_conn *hci_conn_hash_lookup_le(struct hci_dev *hdev,
+						       bdaddr_t *ba,
+						       __u8 ba_type)
+{
+	struct hci_conn_hash *h = &hdev->conn_hash;
+	struct hci_conn  *c;
+
+	rcu_read_lock();
+
+	list_for_each_entry_rcu(c, &h->list, list) {
+		if (c->type != LE_LINK)
+		       continue;
+
+		if (ba_type == c->dst_type && !bacmp(&c->dst, ba)) {
+			rcu_read_unlock();
+			return c;
+		}
+	}
+
+	rcu_read_unlock();
+
+	return NULL;
+}
+
 static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
 							__u8 type, __u16 state)
 {
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 03/13] Bluetooth: Use hci_conn_hash_lookup_le() when possible
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
  2015-10-21 15:02 ` [PATCH 01/13] Bluetooth: Add le_addr_type() helper function Johan Hedberg
  2015-10-21 15:03 ` [PATCH 02/13] Bluetooth: Add hci_conn_hash_lookup_le() " Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 04/13] Bluetooth: 6lowpan: " Johan Hedberg
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

Use the new hci_conn_hash_lookup_le() API to look up LE connections.
This way we're guaranteed exact matches that also take into account
the address type.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_conn.c  |  9 +++------
 net/bluetooth/hci_event.c |  3 ++-
 net/bluetooth/mgmt.c      | 10 ++++++----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index ec4836f243bc..60fe6a570800 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -869,7 +869,7 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 	 * attempt, we simply update pending_sec_level and auth_type fields
 	 * and return the object found.
 	 */
-	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
+	conn = hci_conn_hash_lookup_le(hdev, dst, dst_type);
 	conn_unfinished = NULL;
 	if (conn) {
 		if (conn->state == BT_CONNECT &&
@@ -1019,13 +1019,10 @@ static bool is_connected(struct hci_dev *hdev, bdaddr_t *addr, u8 type)
 {
 	struct hci_conn *conn;
 
-	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, addr);
+	conn = hci_conn_hash_lookup_le(hdev, addr, type);
 	if (!conn)
 		return false;
 
-	if (conn->dst_type != type)
-		return false;
-
 	if (conn->state != BT_CONNECTED)
 		return false;
 
@@ -1098,7 +1095,7 @@ struct hci_conn *hci_connect_le_scan(struct hci_dev *hdev, bdaddr_t *dst,
 	 * attempt, we simply update pending_sec_level and auth_type fields
 	 * and return the object found.
 	 */
-	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
+	conn = hci_conn_hash_lookup_le(hdev, dst, dst_type);
 	if (conn) {
 		if (conn->pending_sec_level < sec_level)
 			conn->pending_sec_level = sec_level;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index b4571d84cafa..504892cfb25a 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1915,7 +1915,8 @@ static void hci_cs_le_create_conn(struct hci_dev *hdev, u8 status)
 
 	hci_dev_lock(hdev);
 
-	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->peer_addr);
+	conn = hci_conn_hash_lookup_le(hdev, &cp->peer_addr,
+				       cp->peer_addr_type);
 	if (!conn)
 		goto unlock;
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index de338d576907..19834f524a7b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3098,8 +3098,8 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	} else {
 		u8 addr_type = le_addr_type(cp->addr.type);
 
-		conn = hci_conn_hash_lookup_ba(hdev, LE_LINK,
-					       &cp->addr.bdaddr);
+		conn = hci_conn_hash_lookup_le(hdev, &cp->addr.bdaddr,
+					       addr_type);
 		if (conn) {
 			/* Defer clearing up the connection parameters
 			 * until closing to give a chance of keeping
@@ -3198,7 +3198,8 @@ static int disconnect(struct sock *sk, struct hci_dev *hdev, void *data,
 		conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
 					       &cp->addr.bdaddr);
 	else
-		conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
+		conn = hci_conn_hash_lookup_le(hdev, &cp->addr.bdaddr,
+					       le_addr_type(cp->addr.type));
 
 	if (!conn || conn->state == BT_OPEN || conn->state == BT_CLOSED) {
 		err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_DISCONNECT,
@@ -3695,7 +3696,8 @@ static int user_pairing_resp(struct sock *sk, struct hci_dev *hdev,
 	if (addr->type == BDADDR_BREDR)
 		conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &addr->bdaddr);
 	else
-		conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &addr->bdaddr);
+		conn = hci_conn_hash_lookup_le(hdev, &addr->bdaddr,
+					       le_addr_type(addr->type));
 
 	if (!conn) {
 		err = mgmt_cmd_complete(sk, hdev->id, mgmt_op,
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 04/13] Bluetooth: 6lowpan: Use hci_conn_hash_lookup_le() when possible
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (2 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 03/13] Bluetooth: Use hci_conn_hash_lookup_le() when possible Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 05/13] Bluetooth: Remove unnecessary indentation in unpair_device() Johan Hedberg
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

Use the new hci_conn_hash_lookup_le() API to look up LE connections.
This way we're guaranteed exact matches that also take into account
the address type.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/6lowpan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 4057d6e6d8d5..d85af2385486 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -1113,7 +1113,7 @@ static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
 		return -ENOENT;
 
 	hci_dev_lock(hdev);
-	hcon = hci_conn_hash_lookup_ba(hdev, LE_LINK, addr);
+	hcon = hci_conn_hash_lookup_le(hdev, addr, *addr_type);
 	hci_dev_unlock(hdev);
 
 	if (!hcon)
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 05/13] Bluetooth: Remove unnecessary indentation in unpair_device()
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (3 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 04/13] Bluetooth: 6lowpan: " Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 06/13] Bluetooth: Disable auto-connection parameters when unpairing Johan Hedberg
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

Instead of doing all of the LE-specific handling in an else-branch in
unpair_device() create a 'done' label for the BR/EDR branch to jump to
and then remove the else-branch completely.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 55 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 19834f524a7b..d56845a28af1 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3055,6 +3055,7 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	struct hci_cp_disconnect dc;
 	struct mgmt_pending_cmd *cmd;
 	struct hci_conn *conn;
+	u8 addr_type;
 	int err;
 
 	memset(&rp, 0, sizeof(rp));
@@ -3095,33 +3096,23 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 			conn = NULL;
 
 		err = hci_remove_link_key(hdev, &cp->addr.bdaddr);
-	} else {
-		u8 addr_type = le_addr_type(cp->addr.type);
-
-		conn = hci_conn_hash_lookup_le(hdev, &cp->addr.bdaddr,
-					       addr_type);
-		if (conn) {
-			/* Defer clearing up the connection parameters
-			 * until closing to give a chance of keeping
-			 * them if a repairing happens.
-			 */
-			set_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags);
-
-			/* If disconnection is not requested, then
-			 * clear the connection variable so that the
-			 * link is not terminated.
-			 */
-			if (!cp->disconnect)
-				conn = NULL;
-		} else {
-			hci_conn_params_del(hdev, &cp->addr.bdaddr, addr_type);
+		if (err < 0) {
+			err = mgmt_cmd_complete(sk, hdev->id,
+						MGMT_OP_UNPAIR_DEVICE,
+						MGMT_STATUS_NOT_PAIRED, &rp,
+						sizeof(rp));
+			goto unlock;
 		}
 
-		hci_remove_irk(hdev, &cp->addr.bdaddr, addr_type);
-
-		err = hci_remove_ltk(hdev, &cp->addr.bdaddr, addr_type);
+		goto done;
 	}
 
+	/* LE address type */
+	addr_type = le_addr_type(cp->addr.type);
+
+	hci_remove_irk(hdev, &cp->addr.bdaddr, addr_type);
+
+	err = hci_remove_ltk(hdev, &cp->addr.bdaddr, addr_type);
 	if (err < 0) {
 		err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_UNPAIR_DEVICE,
 					MGMT_STATUS_NOT_PAIRED, &rp,
@@ -3129,6 +3120,24 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto unlock;
 	}
 
+	conn = hci_conn_hash_lookup_le(hdev, &cp->addr.bdaddr, addr_type);
+	if (!conn) {
+		hci_conn_params_del(hdev, &cp->addr.bdaddr, addr_type);
+		goto done;
+	}
+
+	/* Defer clearing up the connection parameters until closing to
+	 * give a chance of keeping them if a repairing happens.
+	 */
+	set_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags);
+
+	/* If disconnection is not requested, then clear the connection
+	 * variable so that the link is not terminated.
+	 */
+	if (!cp->disconnect)
+		conn = NULL;
+
+done:
 	/* If the connection variable is set, then termination of the
 	 * link is requested.
 	 */
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 06/13] Bluetooth: Disable auto-connection parameters when unpairing
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (4 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 05/13] Bluetooth: Remove unnecessary indentation in unpair_device() Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 07/13] Bluetooth: Introduce hci_req helper to abort a connection Johan Hedberg
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

For connection parameters that are left around until a disconnection
we should at least clear any auto-connection properties. This way a
new Add Device call is required to re-set them after calling Unpair
Device.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d56845a28af1..3fa4cafc2c03 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3052,6 +3052,7 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 {
 	struct mgmt_cp_unpair_device *cp = data;
 	struct mgmt_rp_unpair_device rp;
+	struct hci_conn_params *params;
 	struct hci_cp_disconnect dc;
 	struct mgmt_pending_cmd *cmd;
 	struct hci_conn *conn;
@@ -3131,6 +3132,15 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	 */
 	set_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags);
 
+	/* Disable auto-connection parameters if present */
+	params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr, addr_type);
+	if (params) {
+		if (params->explicit_connect)
+			params->auto_connect = HCI_AUTO_CONN_EXPLICIT;
+		else
+			params->auto_connect = HCI_AUTO_CONN_DISABLED;
+	}
+
 	/* If disconnection is not requested, then clear the connection
 	 * variable so that the link is not terminated.
 	 */
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 07/13] Bluetooth: Introduce hci_req helper to abort a connection
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (5 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 06/13] Bluetooth: Disable auto-connection parameters when unpairing Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 08/13] Bluetooth: Take advantage of connection abort helpers Johan Hedberg
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

There are several different places needing to make sure that a
connection gets disconnected or canceled. The exact action needed
depends on the connection state, so centralizing this logic can save
quite a lot of code duplication.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_request.c | 86 +++++++++++++++++++++++++++++++++++++++++++++
 net/bluetooth/hci_request.h |  4 +++
 2 files changed, 90 insertions(+)

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index b7369220c9ef..b50e51900255 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -564,3 +564,89 @@ void hci_update_background_scan(struct hci_dev *hdev)
 	if (err && err != -ENODATA)
 		BT_ERR("Failed to run HCI request: err %d", err);
 }
+
+void __hci_abort_conn(struct hci_request *req, struct hci_conn *conn,
+		      u8 reason)
+{
+	switch (conn->state) {
+	case BT_CONNECTED:
+	case BT_CONFIG:
+		if (conn->type == AMP_LINK) {
+			struct hci_cp_disconn_phy_link cp;
+
+			cp.phy_handle = HCI_PHY_HANDLE(conn->handle);
+			cp.reason = hci_proto_disconn_ind(conn);
+			hci_req_add(req, HCI_OP_DISCONN_PHY_LINK, sizeof(cp),
+				    &cp);
+		} else {
+			struct hci_cp_disconnect dc;
+
+			dc.handle = cpu_to_le16(conn->handle);
+			dc.reason = reason;
+			hci_req_add(req, HCI_OP_DISCONNECT, sizeof(dc), &dc);
+		}
+
+		conn->state = BT_DISCONN;
+
+		break;
+	case BT_CONNECT:
+		if (test_bit(HCI_CONN_SCANNING, &conn->flags))
+			break;
+		if (conn->type == LE_LINK)
+			hci_req_add(req, HCI_OP_LE_CREATE_CONN_CANCEL,
+				    0, NULL);
+		else if (conn->type == ACL_LINK)
+			hci_req_add(req, HCI_OP_CREATE_CONN_CANCEL,
+				    6, &conn->dst);
+		break;
+	case BT_CONNECT2:
+		if (conn->type == ACL_LINK) {
+			struct hci_cp_reject_conn_req rej;
+
+			if (req->hdev->hci_ver < BLUETOOTH_VER_1_2)
+				return;
+
+			bacpy(&rej.bdaddr, &conn->dst);
+			rej.reason = reason;
+
+			hci_req_add(req, HCI_OP_REJECT_CONN_REQ,
+				    sizeof(rej), &rej);
+		} else if (conn->type == SCO_LINK || conn->type == ESCO_LINK) {
+			struct hci_cp_reject_sync_conn_req rej;
+
+			bacpy(&rej.bdaddr, &conn->dst);
+			rej.reason = reason;
+
+			hci_req_add(req, HCI_OP_REJECT_SYNC_CONN_REQ,
+				    sizeof(rej), &rej);
+		}
+		break;
+	default:
+		conn->state = BT_CLOSED;
+		break;
+	}
+}
+
+static void abort_conn_complete(struct hci_dev *hdev, u8 status, u16 opcode)
+{
+	if (status)
+		BT_DBG("Failed to abort connection: status 0x%2.2x", status);
+}
+
+int hci_abort_conn(struct hci_conn *conn, u8 reason)
+{
+	struct hci_request req;
+	int err;
+
+	hci_req_init(&req, conn->hdev);
+
+	__hci_abort_conn(&req, conn, reason);
+
+	err = hci_req_run(&req, abort_conn_complete);
+	if (err && err != -ENODATA) {
+		BT_ERR("Failed to run HCI request: err %d", err);
+		return err;
+	}
+
+	return 0;
+}
diff --git a/net/bluetooth/hci_request.h b/net/bluetooth/hci_request.h
index bf6df92f42db..25c7f1305dcb 100644
--- a/net/bluetooth/hci_request.h
+++ b/net/bluetooth/hci_request.h
@@ -55,3 +55,7 @@ int hci_update_random_address(struct hci_request *req, bool require_privacy,
 
 void hci_update_background_scan(struct hci_dev *hdev);
 void __hci_update_background_scan(struct hci_request *req);
+
+int hci_abort_conn(struct hci_conn *conn, u8 reason);
+void __hci_abort_conn(struct hci_request *req, struct hci_conn *conn,
+		      u8 reason);
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 08/13] Bluetooth: Take advantage of connection abort helpers
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (6 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 07/13] Bluetooth: Introduce hci_req helper to abort a connection Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 09/13] Bluetooth: Remove redundant (and possibly wrong) flag clearing Johan Hedberg
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

Convert the various places mapping connection state to
disconnect/cancel HCI command to use the new hci_abort_conn helper
API.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_conn.c | 79 +++++-------------------------------------------
 net/bluetooth/mgmt.c     | 36 ++--------------------
 2 files changed, 11 insertions(+), 104 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 60fe6a570800..c73ee1c1ec46 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -59,11 +59,6 @@ static const struct sco_param esco_param_msbc[] = {
 	{ EDR_ESCO_MASK | ESCO_EV3,   0x0008,	0x02 }, /* T1 */
 };
 
-static void hci_le_create_connection_cancel(struct hci_conn *conn)
-{
-	hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
-}
-
 /* This function requires the caller holds hdev->lock */
 static void hci_connect_le_scan_cleanup(struct hci_conn *conn)
 {
@@ -227,29 +222,6 @@ static void hci_acl_create_connection(struct hci_conn *conn)
 	hci_send_cmd(hdev, HCI_OP_CREATE_CONN, sizeof(cp), &cp);
 }
 
-static void hci_acl_create_connection_cancel(struct hci_conn *conn)
-{
-	struct hci_cp_create_conn_cancel cp;
-
-	BT_DBG("hcon %p", conn);
-
-	if (conn->hdev->hci_ver < BLUETOOTH_VER_1_2)
-		return;
-
-	bacpy(&cp.bdaddr, &conn->dst);
-	hci_send_cmd(conn->hdev, HCI_OP_CREATE_CONN_CANCEL, sizeof(cp), &cp);
-}
-
-static void hci_reject_sco(struct hci_conn *conn)
-{
-	struct hci_cp_reject_sync_conn_req cp;
-
-	cp.reason = HCI_ERROR_REJ_LIMITED_RESOURCES;
-	bacpy(&cp.bdaddr, &conn->dst);
-
-	hci_send_cmd(conn->hdev, HCI_OP_REJECT_SYNC_CONN_REQ, sizeof(cp), &cp);
-}
-
 int hci_disconnect(struct hci_conn *conn, __u8 reason)
 {
 	struct hci_cp_disconnect cp;
@@ -277,20 +249,6 @@ int hci_disconnect(struct hci_conn *conn, __u8 reason)
 	return hci_send_cmd(conn->hdev, HCI_OP_DISCONNECT, sizeof(cp), &cp);
 }
 
-static void hci_amp_disconn(struct hci_conn *conn)
-{
-	struct hci_cp_disconn_phy_link cp;
-
-	BT_DBG("hcon %p", conn);
-
-	conn->state = BT_DISCONN;
-
-	cp.phy_handle = HCI_PHY_HANDLE(conn->handle);
-	cp.reason = hci_proto_disconn_ind(conn);
-	hci_send_cmd(conn->hdev, HCI_OP_DISCONN_PHY_LINK,
-		     sizeof(cp), &cp);
-}
-
 static void hci_add_sco(struct hci_conn *conn, __u16 handle)
 {
 	struct hci_dev *hdev = conn->hdev;
@@ -454,35 +412,14 @@ static void hci_conn_timeout(struct work_struct *work)
 	if (refcnt > 0)
 		return;
 
-	switch (conn->state) {
-	case BT_CONNECT:
-	case BT_CONNECT2:
-		if (conn->out) {
-			if (conn->type == ACL_LINK)
-				hci_acl_create_connection_cancel(conn);
-			else if (conn->type == LE_LINK) {
-				if (test_bit(HCI_CONN_SCANNING, &conn->flags))
-					hci_connect_le_scan_remove(conn);
-				else
-					hci_le_create_connection_cancel(conn);
-			}
-		} else if (conn->type == SCO_LINK || conn->type == ESCO_LINK) {
-			hci_reject_sco(conn);
-		}
-		break;
-	case BT_CONFIG:
-	case BT_CONNECTED:
-		if (conn->type == AMP_LINK) {
-			hci_amp_disconn(conn);
-		} else {
-			__u8 reason = hci_proto_disconn_ind(conn);
-			hci_disconnect(conn, reason);
-		}
-		break;
-	default:
-		conn->state = BT_CLOSED;
-		break;
+	/* LE connections in scanning state need special handling */
+	if (conn->state == BT_CONNECT && conn->type == LE_LINK &&
+	    test_bit(HCI_CONN_SCANNING, &conn->flags)) {
+		hci_connect_le_scan_remove(conn);
+		return;
 	}
+
+	hci_abort_conn(conn, hci_proto_disconn_ind(conn));
 }
 
 /* Enter sniff mode */
@@ -550,7 +487,7 @@ static void le_conn_timeout(struct work_struct *work)
 		return;
 	}
 
-	hci_le_create_connection_cancel(conn);
+	hci_abort_conn(conn, HCI_ERROR_REMOTE_USER_TERM);
 }
 
 struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3fa4cafc2c03..e1083c928d5c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1639,35 +1639,8 @@ static int clean_up_hci_state(struct hci_dev *hdev)
 	discov_stopped = hci_stop_discovery(&req);
 
 	list_for_each_entry(conn, &hdev->conn_hash.list, list) {
-		struct hci_cp_disconnect dc;
-		struct hci_cp_reject_conn_req rej;
-
-		switch (conn->state) {
-		case BT_CONNECTED:
-		case BT_CONFIG:
-			dc.handle = cpu_to_le16(conn->handle);
-			dc.reason = 0x15; /* Terminated due to Power Off */
-			hci_req_add(&req, HCI_OP_DISCONNECT, sizeof(dc), &dc);
-			break;
-		case BT_CONNECT:
-			if (conn->type == LE_LINK)
-				hci_req_add(&req, HCI_OP_LE_CREATE_CONN_CANCEL,
-					    0, NULL);
-			else if (conn->type == ACL_LINK)
-				hci_req_add(&req, HCI_OP_CREATE_CONN_CANCEL,
-					    6, &conn->dst);
-			break;
-		case BT_CONNECT2:
-			bacpy(&rej.bdaddr, &conn->dst);
-			rej.reason = 0x15; /* Terminated due to Power Off */
-			if (conn->type == ACL_LINK)
-				hci_req_add(&req, HCI_OP_REJECT_CONN_REQ,
-					    sizeof(rej), &rej);
-			else if (conn->type == SCO_LINK)
-				hci_req_add(&req, HCI_OP_REJECT_SYNC_CONN_REQ,
-					    sizeof(rej), &rej);
-			break;
-		}
+		/* 0x15 == Terminated due to Power Off */
+		__hci_abort_conn(&req, conn, 0x15);
 	}
 
 	err = hci_req_run(&req, clean_up_hci_complete);
@@ -3053,7 +3026,6 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	struct mgmt_cp_unpair_device *cp = data;
 	struct mgmt_rp_unpair_device rp;
 	struct hci_conn_params *params;
-	struct hci_cp_disconnect dc;
 	struct mgmt_pending_cmd *cmd;
 	struct hci_conn *conn;
 	u8 addr_type;
@@ -3167,9 +3139,7 @@ done:
 
 	cmd->cmd_complete = addr_cmd_complete;
 
-	dc.handle = cpu_to_le16(conn->handle);
-	dc.reason = 0x13; /* Remote User Terminated Connection */
-	err = hci_send_cmd(hdev, HCI_OP_DISCONNECT, sizeof(dc), &dc);
+	err = hci_abort_conn(conn, HCI_ERROR_REMOTE_USER_TERM);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 09/13] Bluetooth: Remove redundant (and possibly wrong) flag clearing
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (7 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 08/13] Bluetooth: Take advantage of connection abort helpers Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 10/13] Bluetooth: Make hci_disconnect() behave correctly for all states Johan Hedberg
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

There's no need to clear the HCI_CONN_ENCRYPT_PEND flag in
smp_failure. In fact, this may cause the encryption tracking to get
out of sync as this has nothing to do with HCI activity.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/smp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 229d88eebf4e..94f9c4ca68f1 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -811,7 +811,6 @@ static void smp_failure(struct l2cap_conn *conn, u8 reason)
 		smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, sizeof(reason),
 			     &reason);
 
-	clear_bit(HCI_CONN_ENCRYPT_PEND, &hcon->flags);
 	mgmt_auth_failed(hcon, HCI_ERROR_AUTH_FAILURE);
 
 	if (chan->data)
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 10/13] Bluetooth: Make hci_disconnect() behave correctly for all states
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (8 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 09/13] Bluetooth: Remove redundant (and possibly wrong) flag clearing Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 11/13] Bluetooth: Add hdev helper variable to hci_le_create_connection_cancel Johan Hedberg
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

There are a few places that don't explicitly check the connection
state before calling hci_disconnect(). To make this API do the right
thing take advantage of the new hci_abort_conn() API and also make
sure to only read the clock offset if we're really connected.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_conn.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index c73ee1c1ec46..c562d5d5b14e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -224,8 +224,6 @@ static void hci_acl_create_connection(struct hci_conn *conn)
 
 int hci_disconnect(struct hci_conn *conn, __u8 reason)
 {
-	struct hci_cp_disconnect cp;
-
 	BT_DBG("hcon %p", conn);
 
 	/* When we are master of an established connection and it enters
@@ -233,7 +231,8 @@ int hci_disconnect(struct hci_conn *conn, __u8 reason)
 	 * current clock offset.  Processing of the result is done
 	 * within the event handling and hci_clock_offset_evt function.
 	 */
-	if (conn->type == ACL_LINK && conn->role == HCI_ROLE_MASTER) {
+	if (conn->type == ACL_LINK && conn->role == HCI_ROLE_MASTER &&
+	    (conn->state == BT_CONNECTED || conn->state == BT_CONFIG)) {
 		struct hci_dev *hdev = conn->hdev;
 		struct hci_cp_read_clock_offset clkoff_cp;
 
@@ -242,11 +241,7 @@ int hci_disconnect(struct hci_conn *conn, __u8 reason)
 			     &clkoff_cp);
 	}
 
-	conn->state = BT_DISCONN;
-
-	cp.handle = cpu_to_le16(conn->handle);
-	cp.reason = reason;
-	return hci_send_cmd(conn->hdev, HCI_OP_DISCONNECT, sizeof(cp), &cp);
+	return hci_abort_conn(conn, reason);
 }
 
 static void hci_add_sco(struct hci_conn *conn, __u16 handle)
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 11/13] Bluetooth: Add hdev helper variable to hci_le_create_connection_cancel
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (9 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 10/13] Bluetooth: Make hci_disconnect() behave correctly for all states Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 12/13] Bluetooth: Remove unnecessary hci_explicit_connect_lookup function Johan Hedberg
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

The hci_le_create_connection_cancel() function needs to use the hdev
pointer in many places so add a variable for it to avoid the need to
dereference the hci_conn every time.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_conn.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index c562d5d5b14e..77b2bf9e808e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -63,6 +63,7 @@ static const struct sco_param esco_param_msbc[] = {
 static void hci_connect_le_scan_cleanup(struct hci_conn *conn)
 {
 	struct hci_conn_params *params;
+	struct hci_dev *hdev = conn->hdev;
 	struct smp_irk *irk;
 	bdaddr_t *bdaddr;
 	u8 bdaddr_type;
@@ -71,13 +72,13 @@ static void hci_connect_le_scan_cleanup(struct hci_conn *conn)
 	bdaddr_type = conn->dst_type;
 
 	/* Check if we need to convert to identity address */
-	irk = hci_get_irk(conn->hdev, bdaddr, bdaddr_type);
+	irk = hci_get_irk(hdev, bdaddr, bdaddr_type);
 	if (irk) {
 		bdaddr = &irk->bdaddr;
 		bdaddr_type = irk->addr_type;
 	}
 
-	params = hci_explicit_connect_lookup(conn->hdev, bdaddr, bdaddr_type);
+	params = hci_explicit_connect_lookup(hdev, bdaddr, bdaddr_type);
 	if (!params)
 		return;
 
@@ -92,21 +93,21 @@ static void hci_connect_le_scan_cleanup(struct hci_conn *conn)
 
 	switch (params->auto_connect) {
 	case HCI_AUTO_CONN_EXPLICIT:
-		hci_conn_params_del(conn->hdev, bdaddr, bdaddr_type);
+		hci_conn_params_del(hdev, bdaddr, bdaddr_type);
 		/* return instead of break to avoid duplicate scan update */
 		return;
 	case HCI_AUTO_CONN_DIRECT:
 	case HCI_AUTO_CONN_ALWAYS:
-		list_add(&params->action, &conn->hdev->pend_le_conns);
+		list_add(&params->action, &hdev->pend_le_conns);
 		break;
 	case HCI_AUTO_CONN_REPORT:
-		list_add(&params->action, &conn->hdev->pend_le_reports);
+		list_add(&params->action, &hdev->pend_le_reports);
 		break;
 	default:
 		break;
 	}
 
-	hci_update_background_scan(conn->hdev);
+	hci_update_background_scan(hdev);
 }
 
 static void hci_conn_cleanup(struct hci_conn *conn)
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 12/13] Bluetooth: Remove unnecessary hci_explicit_connect_lookup function
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (10 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 11/13] Bluetooth: Add hdev helper variable to hci_le_create_connection_cancel Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 15:03 ` [PATCH 13/13] Bluetooth: Fix crash in SMP when unpairing Johan Hedberg
  2015-10-21 17:02 ` [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Marcel Holtmann
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

There's only one user of this helper which can be replaces with a call
to hci_pend_le_action_lookup() and a check for params->explicit_connect.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |  3 ---
 net/bluetooth/hci_conn.c         |  5 +++--
 net/bluetooth/hci_core.c         | 17 -----------------
 3 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 32bb281e6aa6..1878d0a96333 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1042,9 +1042,6 @@ void hci_conn_params_clear_disabled(struct hci_dev *hdev);
 struct hci_conn_params *hci_pend_le_action_lookup(struct list_head *list,
 						  bdaddr_t *addr,
 						  u8 addr_type);
-struct hci_conn_params *hci_explicit_connect_lookup(struct hci_dev *hdev,
-						    bdaddr_t *addr,
-						    u8 addr_type);
 
 void hci_uuids_clear(struct hci_dev *hdev);
 
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 77b2bf9e808e..85b82f7adbd2 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -78,8 +78,9 @@ static void hci_connect_le_scan_cleanup(struct hci_conn *conn)
 		bdaddr_type = irk->addr_type;
 	}
 
-	params = hci_explicit_connect_lookup(hdev, bdaddr, bdaddr_type);
-	if (!params)
+	params = hci_pend_le_action_lookup(&hdev->pend_le_conns, bdaddr,
+					   bdaddr_type);
+	if (!params || !params->explicit_connect)
 		return;
 
 	/* The connection attempt was doing scan for new RPA, and is
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 964fba4c96bf..086ed9389da1 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2941,23 +2941,6 @@ struct hci_conn_params *hci_pend_le_action_lookup(struct list_head *list,
 }
 
 /* This function requires the caller holds hdev->lock */
-struct hci_conn_params *hci_explicit_connect_lookup(struct hci_dev *hdev,
-						    bdaddr_t *addr,
-						    u8 addr_type)
-{
-	struct hci_conn_params *param;
-
-	list_for_each_entry(param, &hdev->pend_le_conns, action) {
-		if (bacmp(&param->addr, addr) == 0 &&
-		    param->addr_type == addr_type &&
-		    param->explicit_connect)
-			return param;
-	}
-
-	return NULL;
-}
-
-/* This function requires the caller holds hdev->lock */
 struct hci_conn_params *hci_conn_params_add(struct hci_dev *hdev,
 					    bdaddr_t *addr, u8 addr_type)
 {
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 13/13] Bluetooth: Fix crash in SMP when unpairing
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (11 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 12/13] Bluetooth: Remove unnecessary hci_explicit_connect_lookup function Johan Hedberg
@ 2015-10-21 15:03 ` Johan Hedberg
  2015-10-21 17:02 ` [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Marcel Holtmann
  13 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-10-21 15:03 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

When unpairing the keys stored in hci_dev are removed. If SMP is
ongoing the SMP context will also have references to these keys, so
removing them from the hci_dev lists will make the pointers invalid.
This can result in the following type of crashes:

 BUG: unable to handle kernel paging request at 6b6b6b6b
 IP: [<c11f26be>] __list_del_entry+0x44/0x71
 *pde = 00000000
 Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
 Modules linked in: hci_uart btqca btusb btintel btbcm btrtl hci_vhci rfcomm bluetooth_6lowpan bluetooth
 CPU: 0 PID: 723 Comm: kworker/u5:0 Not tainted 4.3.0-rc3+ #1379
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.1-20150318_183358- 04/01/2014
 Workqueue: hci0 hci_rx_work [bluetooth]
 task: f19da940 ti: f1a94000 task.ti: f1a94000
 EIP: 0060:[<c11f26be>] EFLAGS: 00010202 CPU: 0
 EIP is at __list_del_entry+0x44/0x71
 EAX: c0088d20 EBX: f30fcac0 ECX: 6b6b6b6b EDX: 6b6b6b6b
 ESI: f4b60000 EDI: c0088d20 EBP: f1a95d90 ESP: f1a95d8c
  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
 CR0: 8005003b CR2: 6b6b6b6b CR3: 319e5000 CR4: 00000690
 Stack:
  f30fcac0 f1a95db0 f82dc3e1 f1bfc000 00000000 c106524f f1bfc000 f30fd020
  f1a95dc0 f1a95dd0 f82dcbdb f1a95de0 f82dcbdb 00000067 f1bfc000 f30fd020
  f1a95de0 f1a95df0 f82d1126 00000067 f82d1126 00000006 f30fd020 f1bfc000
 Call Trace:
  [<f82dc3e1>] smp_chan_destroy+0x192/0x240 [bluetooth]
  [<c106524f>] ? trace_hardirqs_on_caller+0x14e/0x169
  [<f82dcbdb>] smp_teardown_cb+0x47/0x64 [bluetooth]
  [<f82dcbdb>] ? smp_teardown_cb+0x47/0x64 [bluetooth]
  [<f82d1126>] l2cap_chan_del+0x5d/0x14d [bluetooth]
  [<f82d1126>] ? l2cap_chan_del+0x5d/0x14d [bluetooth]
  [<f82d40ef>] l2cap_conn_del+0x109/0x17b [bluetooth]
  [<f82d40ef>] ? l2cap_conn_del+0x109/0x17b [bluetooth]
  [<f82c0205>] ? hci_event_packet+0x5b1/0x2092 [bluetooth]
  [<f82d41aa>] l2cap_disconn_cfm+0x49/0x50 [bluetooth]
  [<f82d41aa>] ? l2cap_disconn_cfm+0x49/0x50 [bluetooth]
  [<f82c0228>] hci_event_packet+0x5d4/0x2092 [bluetooth]
  [<c1332c16>] ? skb_release_data+0x6a/0x95
  [<f82ce5d4>] ? hci_send_to_monitor+0xe7/0xf4 [bluetooth]
  [<c1409708>] ? _raw_spin_unlock_irqrestore+0x44/0x57
  [<f82b3bb0>] hci_rx_work+0xf1/0x28b [bluetooth]
  [<f82b3bb0>] ? hci_rx_work+0xf1/0x28b [bluetooth]
  [<c10635a0>] ? __lock_is_held+0x2e/0x44
  [<c104772e>] process_one_work+0x232/0x432
  [<c1071ddc>] ? rcu_read_lock_sched_held+0x50/0x5a
  [<c104772e>] ? process_one_work+0x232/0x432
  [<c1047d48>] worker_thread+0x1b8/0x255
  [<c1047b90>] ? rescuer_thread+0x23c/0x23c
  [<c104bb71>] kthread+0x91/0x96
  [<c14096a7>] ? _raw_spin_unlock_irq+0x27/0x44
  [<c1409d61>] ret_from_kernel_thread+0x21/0x30
  [<c104bae0>] ? kthread_parkme+0x1e/0x1e

To solve the issue, introduce a new smp_cancel_pairing() API that can
be used to clean up the SMP state before touching the hci_dev lists.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |  3 +++
 net/bluetooth/smp.c  | 26 ++++++++++++++++++++++++++
 net/bluetooth/smp.h  |  1 +
 3 files changed, 30 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index e1083c928d5c..7f22119276f3 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3099,6 +3099,9 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto done;
 	}
 
+	/* Abort any ongoing SMP pairing */
+	smp_cancel_pairing(conn);
+
 	/* Defer clearing up the connection parameters until closing to
 	 * give a chance of keeping them if a repairing happens.
 	 */
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 94f9c4ca68f1..c91353841e40 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -2380,6 +2380,32 @@ unlock:
 	return ret;
 }
 
+void smp_cancel_pairing(struct hci_conn *hcon)
+{
+	struct l2cap_conn *conn = hcon->l2cap_data;
+	struct l2cap_chan *chan;
+	struct smp_chan *smp;
+
+	if (!conn)
+		return;
+
+	chan = conn->smp;
+	if (!chan)
+		return;
+
+	l2cap_chan_lock(chan);
+
+	smp = chan->data;
+	if (smp) {
+		if (test_bit(SMP_FLAG_COMPLETE, &smp->flags))
+			smp_failure(conn, 0);
+		else
+			smp_failure(conn, SMP_UNSPECIFIED);
+	}
+
+	l2cap_chan_unlock(chan);
+}
+
 static int smp_cmd_encrypt_info(struct l2cap_conn *conn, struct sk_buff *skb)
 {
 	struct smp_cmd_encrypt_info *rp = (void *) skb->data;
diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
index 6cf872563ea7..ffcc70b6b199 100644
--- a/net/bluetooth/smp.h
+++ b/net/bluetooth/smp.h
@@ -180,6 +180,7 @@ enum smp_key_pref {
 };
 
 /* SMP Commands */
+void smp_cancel_pairing(struct hci_conn *hcon);
 bool smp_sufficient_security(struct hci_conn *hcon, u8 sec_level,
 			     enum smp_key_pref key_pref);
 int smp_conn_security(struct hci_conn *hcon, __u8 sec_level);
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF
  2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
                   ` (12 preceding siblings ...)
  2015-10-21 15:03 ` [PATCH 13/13] Bluetooth: Fix crash in SMP when unpairing Johan Hedberg
@ 2015-10-21 17:02 ` Marcel Holtmann
  13 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2015-10-21 17:02 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth

Hi Johan,

> Here's the left-over set of patches from the UPF which are not as
> critical as the other ones that were already applied. I didn't (yet)
> spend time sorting or splitting these more logically, but I decided to
> send what I have now to get an early review round for them.
> 
> Johan
> 
> ----------------------------------------------------------------
> Johan Hedberg (13):
>      Bluetooth: Add le_addr_type() helper function
>      Bluetooth: Add hci_conn_hash_lookup_le() helper function
>      Bluetooth: Use hci_conn_hash_lookup_le() when possible
>      Bluetooth: 6lowpan: Use hci_conn_hash_lookup_le() when possible
>      Bluetooth: Remove unnecessary indentation in unpair_device()
>      Bluetooth: Disable auto-connection parameters when unpairing
>      Bluetooth: Introduce hci_req helper to abort a connection
>      Bluetooth: Take advantage of connection abort helpers
>      Bluetooth: Remove redundant (and possibly wrong) flag clearing
>      Bluetooth: Make hci_disconnect() behave correctly for all states
>      Bluetooth: Add hdev helper variable to hci_le_create_connection_cancel
>      Bluetooth: Remove unnecessary hci_explicit_connect_lookup function
>      Bluetooth: Fix crash in SMP when unpairing
> 
> include/net/bluetooth/hci_core.h |  27 ++++++-
> net/bluetooth/6lowpan.c          |   2 +-
> net/bluetooth/hci_conn.c         | 115 ++++++--------------------
> net/bluetooth/hci_core.c         |  17 ----
> net/bluetooth/hci_event.c        |   3 +-
> net/bluetooth/hci_request.c      |  86 ++++++++++++++++++++
> net/bluetooth/hci_request.h      |   4 +
> net/bluetooth/mgmt.c             | 164 ++++++++++++++++----------------------
> net/bluetooth/smp.c              |  27 ++++++-
> net/bluetooth/smp.h              |   1 +
> 10 files changed, 236 insertions(+), 210 deletions(-)

so far I applied patches 1-5, 9 and 11-12 to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-10-21 17:02 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-21 15:02 [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Johan Hedberg
2015-10-21 15:02 ` [PATCH 01/13] Bluetooth: Add le_addr_type() helper function Johan Hedberg
2015-10-21 15:03 ` [PATCH 02/13] Bluetooth: Add hci_conn_hash_lookup_le() " Johan Hedberg
2015-10-21 15:03 ` [PATCH 03/13] Bluetooth: Use hci_conn_hash_lookup_le() when possible Johan Hedberg
2015-10-21 15:03 ` [PATCH 04/13] Bluetooth: 6lowpan: " Johan Hedberg
2015-10-21 15:03 ` [PATCH 05/13] Bluetooth: Remove unnecessary indentation in unpair_device() Johan Hedberg
2015-10-21 15:03 ` [PATCH 06/13] Bluetooth: Disable auto-connection parameters when unpairing Johan Hedberg
2015-10-21 15:03 ` [PATCH 07/13] Bluetooth: Introduce hci_req helper to abort a connection Johan Hedberg
2015-10-21 15:03 ` [PATCH 08/13] Bluetooth: Take advantage of connection abort helpers Johan Hedberg
2015-10-21 15:03 ` [PATCH 09/13] Bluetooth: Remove redundant (and possibly wrong) flag clearing Johan Hedberg
2015-10-21 15:03 ` [PATCH 10/13] Bluetooth: Make hci_disconnect() behave correctly for all states Johan Hedberg
2015-10-21 15:03 ` [PATCH 11/13] Bluetooth: Add hdev helper variable to hci_le_create_connection_cancel Johan Hedberg
2015-10-21 15:03 ` [PATCH 12/13] Bluetooth: Remove unnecessary hci_explicit_connect_lookup function Johan Hedberg
2015-10-21 15:03 ` [PATCH 13/13] Bluetooth: Fix crash in SMP when unpairing Johan Hedberg
2015-10-21 17:02 ` [PATCH 00/13] Bluetooth: Cleanups & fixes from the UPF Marcel Holtmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox