[PATCH 00/10] Bluetooth: More patches for privacy

[PATCH 00/10] Bluetooth: More patches for privacy

[johan.hedberg]

Hi,

Here are more patches towards implementing support for privacy. Included
are also some cleanup and fix patches for issues I came accross when
dealing with the code.

Johan

----------------------------------------------------------------
Johan Hedberg (10):
      Bluetooth: Remove SMP data specific crypto context
      Bluetooth: Fix missing address type check for removing LTKs
      Bluetooth: Remove return values from functions that don't need them
      Bluetooth: Fix hci_remove_ltk failure when no match is found
      Bluetooth: Fix completing SMP as peripheral when no keys are expected
      Bluetooth: Fix removing any IRKs when unpairing devices
      Bluetooth: Add convenience function for fetching IRKs
      Bluetooth: Track the LE Identity Address in struct hci_conn
      Bluetooth: Fix updating Identity Address in L2CAP channels
      Bluetooth: Wait for SMP key distribution completion when pairing

 include/net/bluetooth/hci_core.h | 23 +++++++++++++-----
 include/net/bluetooth/l2cap.h    |  1 +
 net/bluetooth/hci_core.c         | 49 ++++++++++++++++++++++----------------
 net/bluetooth/hci_event.c        |  7 ++++++
 net/bluetooth/l2cap_core.c       | 17 +++++++++++++
 net/bluetooth/mgmt.c             | 45 ++++++++++++++++++++++++++--------
 net/bluetooth/smp.c              | 25 ++++++++++---------
 net/bluetooth/smp.h              |  2 +-
 8 files changed, 119 insertions(+), 50 deletions(-)

[PATCH 01/10] Bluetooth: Remove SMP data specific crypto context

[johan.hedberg]

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

Now that each HCI device has its own AES crypto context we don't need
the one stored in the SMP data any more. This patch removes the variable
from struct smp_chan and updates the SMP code to use the per-hdev crypto
context.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/smp.c | 15 ++-------------
 net/bluetooth/smp.h |  1 -
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 5867c1c3f436..6dcb35640b6f 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -413,21 +413,13 @@ static void confirm_work(struct work_struct *work)
 {
 	struct smp_chan *smp = container_of(work, struct smp_chan, confirm);
 	struct l2cap_conn *conn = smp->conn;
-	struct crypto_blkcipher *tfm;
+	struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm_aes;
 	struct smp_cmd_pairing_confirm cp;
 	int ret;
 	u8 res[16], reason;
 
 	BT_DBG("conn %p", conn);
 
-	tfm = crypto_alloc_blkcipher("ecb(aes)", 0, CRYPTO_ALG_ASYNC);
-	if (IS_ERR(tfm)) {
-		reason = SMP_UNSPECIFIED;
-		goto error;
-	}
-
-	smp->tfm = tfm;
-
 	if (conn->hcon->out)
 		ret = smp_c1(tfm, smp->tk, smp->prnd, smp->preq, smp->prsp,
 			     conn->hcon->src_type, &conn->hcon->src,
@@ -457,7 +449,7 @@ static void random_work(struct work_struct *work)
 	struct smp_chan *smp = container_of(work, struct smp_chan, random);
 	struct l2cap_conn *conn = smp->conn;
 	struct hci_conn *hcon = conn->hcon;
-	struct crypto_blkcipher *tfm = smp->tfm;
+	struct crypto_blkcipher *tfm = hcon->hdev->tfm_aes;
 	u8 reason, confirm[16], res[16], key[16];
 	int ret;
 
@@ -562,9 +554,6 @@ void smp_chan_destroy(struct l2cap_conn *conn)
 
 	BUG_ON(!smp);
 
-	if (smp->tfm)
-		crypto_free_blkcipher(smp->tfm);
-
 	kfree(smp);
 	conn->smp_chan = NULL;
 	conn->hcon->smp_conn = NULL;
diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
index 4f373bc56ad7..8f54c9b152de 100644
--- a/net/bluetooth/smp.h
+++ b/net/bluetooth/smp.h
@@ -133,7 +133,6 @@ struct smp_chan {
 	u8		id_addr_type;
 	u8		irk[16];
 	unsigned long	smp_flags;
-	struct crypto_blkcipher	*tfm;
 	struct work_struct confirm;
 	struct work_struct random;
 };
-- 
1.8.5.3

Re: [PATCH 01/10] Bluetooth: Remove SMP data specific crypto context

[Marcel Holtmann]

Hi Johan,

> Now that each HCI device has its own AES crypto context we don't need
> the one stored in the SMP data any more. This patch removes the variable
> from struct smp_chan and updates the SMP code to use the per-hdev crypto
> context.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/smp.c | 15 ++-------------
> net/bluetooth/smp.h |  1 -
> 2 files changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 5867c1c3f436..6dcb35640b6f 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -413,21 +413,13 @@ static void confirm_work(struct work_struct *work)
> {
> 	struct smp_chan *smp = container_of(work, struct smp_chan, confirm);
> 	struct l2cap_conn *conn = smp->conn;
> -	struct crypto_blkcipher *tfm;
> +	struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm_aes;
> 	struct smp_cmd_pairing_confirm cp;
> 	int ret;
> 	u8 res[16], reason;
> 
> 	BT_DBG("conn %p", conn);
> 
> -	tfm = crypto_alloc_blkcipher("ecb(aes)", 0, CRYPTO_ALG_ASYNC);
> -	if (IS_ERR(tfm)) {
> -		reason = SMP_UNSPECIFIED;
> -		goto error;
> -	}
> -
> -	smp->tfm = tfm;
> -

so I am curious now to what happens if we are running SMP and at the same time are doing RPA resolving. Can the same crypto context be used that way?

Regards

Marcel

Re: [PATCH 01/10] Bluetooth: Remove SMP data specific crypto context

[Johan Hedberg]

Hi Marcel,

On Tue, Feb 18, 2014, Marcel Holtmann wrote:
> > Now that each HCI device has its own AES crypto context we don't need
> > the one stored in the SMP data any more. This patch removes the variable
> > from struct smp_chan and updates the SMP code to use the per-hdev crypto
> > context.
> > 
> > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> > ---
> > net/bluetooth/smp.c | 15 ++-------------
> > net/bluetooth/smp.h |  1 -
> > 2 files changed, 2 insertions(+), 14 deletions(-)
> > 
> > diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> > index 5867c1c3f436..6dcb35640b6f 100644
> > --- a/net/bluetooth/smp.c
> > +++ b/net/bluetooth/smp.c
> > @@ -413,21 +413,13 @@ static void confirm_work(struct work_struct *work)
> > {
> > 	struct smp_chan *smp = container_of(work, struct smp_chan, confirm);
> > 	struct l2cap_conn *conn = smp->conn;
> > -	struct crypto_blkcipher *tfm;
> > +	struct crypto_blkcipher *tfm = conn->hcon->hdev->tfm_aes;
> > 	struct smp_cmd_pairing_confirm cp;
> > 	int ret;
> > 	u8 res[16], reason;
> > 
> > 	BT_DBG("conn %p", conn);
> > 
> > -	tfm = crypto_alloc_blkcipher("ecb(aes)", 0, CRYPTO_ALG_ASYNC);
> > -	if (IS_ERR(tfm)) {
> > -		reason = SMP_UNSPECIFIED;
> > -		goto error;
> > -	}
> > -
> > -	smp->tfm = tfm;
> > -
> 
> so I am curious now to what happens if we are running SMP and at the
> same time are doing RPA resolving. Can the same crypto context be used
> that way?

That's a fair point, and after a quick look through the crypto API it
seems like this might not be safe. A simple fix would be to ensure that
any access is between hci_dev_lock & hci_dev_unlock.

Johan

Re: [PATCH 01/10] Bluetooth: Remove SMP data specific crypto context

[Johan Hedberg]

Hi Marcel,

On Tue, Feb 18, 2014, Johan Hedberg wrote:
> > so I am curious now to what happens if we are running SMP and at the
> > same time are doing RPA resolving. Can the same crypto context be used
> > that way?
> 
> That's a fair point, and after a quick look through the crypto API it
> seems like this might not be safe. A simple fix would be to ensure that
> any access is between hci_dev_lock & hci_dev_unlock.

A quick check shows that all access external to smp.c is already done
under the hdev lock but the two places internal to smp.c are not (both
of which are from within workqueue callbacks, so taking the lock there
should be safe). Do you agree that this is the simplest/best solution?

Johan

Re: [PATCH 01/10] Bluetooth: Remove SMP data specific crypto context

[Marcel Holtmann]

Hi Johan,

>>> so I am curious now to what happens if we are running SMP and at the
>>> same time are doing RPA resolving. Can the same crypto context be used
>>> that way?
>> 
>> That's a fair point, and after a quick look through the crypto API it
>> seems like this might not be safe. A simple fix would be to ensure that
>> any access is between hci_dev_lock & hci_dev_unlock.
> 
> A quick check shows that all access external to smp.c is already done
> under the hdev lock but the two places internal to smp.c are not (both
> of which are from within workqueue callbacks, so taking the lock there
> should be safe). Do you agree that this is the simplest/best solution?

since we always do only one AES round anyway, that seems to be a good solution to this.

Regards

Marcel

[PATCH 02/10] Bluetooth: Fix missing address type check for removing LTKs

[johan.hedberg]

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

When removing Long Term Keys we should also be checking that the given
address type (public vs random) matches. This patch updates the
hci_remove_ltk function to take an extra parameter and uses it for
address type matching.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |  2 +-
 net/bluetooth/hci_core.c         |  4 ++--
 net/bluetooth/mgmt.c             | 14 +++++++++++---
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 86ea4bab9e77..ab94abdeb3c1 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -792,7 +792,7 @@ int hci_add_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 addr_type, u8 type,
 		__le16 ediv, u8 rand[8]);
 struct smp_ltk *hci_find_ltk_by_addr(struct hci_dev *hdev, bdaddr_t *bdaddr,
 				     u8 addr_type, bool master);
-int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr);
+int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 bdaddr_type);
 int hci_smp_ltks_clear(struct hci_dev *hdev);
 int hci_remove_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr);
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 59a76b2566eb..957c8f4cc4c7 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2810,12 +2810,12 @@ int hci_remove_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr)
 	return 0;
 }
 
-int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr)
+int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 bdaddr_type)
 {
 	struct smp_ltk *k, *tmp;
 
 	list_for_each_entry_safe(k, tmp, &hdev->long_term_keys, list) {
-		if (bacmp(bdaddr, &k->bdaddr))
+		if (bacmp(bdaddr, &k->bdaddr) || k->bdaddr_type != bdaddr_type)
 			continue;
 
 		BT_DBG("%s removing %pMR", hdev->name, bdaddr);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 782e2bb10881..473f8687b28b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2318,10 +2318,18 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto unlock;
 	}
 
-	if (cp->addr.type == BDADDR_BREDR)
+	if (cp->addr.type == BDADDR_BREDR) {
 		err = hci_remove_link_key(hdev, &cp->addr.bdaddr);
-	else
-		err = hci_remove_ltk(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;
+
+		err = hci_remove_ltk(hdev, &cp->addr.bdaddr, addr_type);
+	}
 
 	if (err < 0) {
 		err = cmd_complete(sk, hdev->id, MGMT_OP_UNPAIR_DEVICE,
-- 
1.8.5.3

[PATCH 03/10] Bluetooth: Remove return values from functions that don't need them

[johan.hedberg]

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

There are many functions that never fail but still declare an integer
return value for no reason. This patch converts these functions to use a
void return value to avoid any confusion of whether they can fail or not.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h | 10 +++++-----
 net/bluetooth/hci_core.c         | 26 +++++++++-----------------
 net/bluetooth/mgmt.c             |  2 +-
 3 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ab94abdeb3c1..964a7888ad0c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -768,7 +768,7 @@ int hci_inquiry(void __user *arg);
 
 struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev,
 					 bdaddr_t *bdaddr, u8 type);
-int hci_blacklist_clear(struct hci_dev *hdev);
+void hci_blacklist_clear(struct hci_dev *hdev);
 int hci_blacklist_add(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
 int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
 
@@ -779,9 +779,9 @@ void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
 void hci_conn_params_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
 void hci_conn_params_clear(struct hci_dev *hdev);
 
-int hci_uuids_clear(struct hci_dev *hdev);
+void hci_uuids_clear(struct hci_dev *hdev);
 
-int hci_link_keys_clear(struct hci_dev *hdev);
+void hci_link_keys_clear(struct hci_dev *hdev);
 struct link_key *hci_find_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr);
 int hci_add_link_key(struct hci_dev *hdev, struct hci_conn *conn, int new_key,
 		     bdaddr_t *bdaddr, u8 *val, u8 type, u8 pin_len);
@@ -793,7 +793,7 @@ int hci_add_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 addr_type, u8 type,
 struct smp_ltk *hci_find_ltk_by_addr(struct hci_dev *hdev, bdaddr_t *bdaddr,
 				     u8 addr_type, bool master);
 int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 bdaddr_type);
-int hci_smp_ltks_clear(struct hci_dev *hdev);
+void hci_smp_ltks_clear(struct hci_dev *hdev);
 int hci_remove_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr);
 
 struct smp_irk *hci_find_irk_by_rpa(struct hci_dev *hdev, bdaddr_t *rpa);
@@ -803,7 +803,7 @@ int hci_add_irk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 addr_type,
 		u8 val[16], bdaddr_t *rpa);
 void hci_smp_irks_clear(struct hci_dev *hdev);
 
-int hci_remote_oob_data_clear(struct hci_dev *hdev);
+void hci_remote_oob_data_clear(struct hci_dev *hdev);
 struct oob_data *hci_find_remote_oob_data(struct hci_dev *hdev,
 					  bdaddr_t *bdaddr);
 int hci_add_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr,
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 957c8f4cc4c7..fd5bb4086613 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2506,7 +2506,7 @@ static void hci_discov_off(struct work_struct *work)
 	mgmt_discoverable_timeout(hdev);
 }
 
-int hci_uuids_clear(struct hci_dev *hdev)
+void hci_uuids_clear(struct hci_dev *hdev)
 {
 	struct bt_uuid *uuid, *tmp;
 
@@ -2514,11 +2514,9 @@ int hci_uuids_clear(struct hci_dev *hdev)
 		list_del(&uuid->list);
 		kfree(uuid);
 	}
-
-	return 0;
 }
 
-int hci_link_keys_clear(struct hci_dev *hdev)
+void hci_link_keys_clear(struct hci_dev *hdev)
 {
 	struct list_head *p, *n;
 
@@ -2530,11 +2528,9 @@ int hci_link_keys_clear(struct hci_dev *hdev)
 		list_del(p);
 		kfree(key);
 	}
-
-	return 0;
 }
 
-int hci_smp_ltks_clear(struct hci_dev *hdev)
+void hci_smp_ltks_clear(struct hci_dev *hdev)
 {
 	struct smp_ltk *k, *tmp;
 
@@ -2542,8 +2538,6 @@ int hci_smp_ltks_clear(struct hci_dev *hdev)
 		list_del(&k->list);
 		kfree(k);
 	}
-
-	return 0;
 }
 
 void hci_smp_irks_clear(struct hci_dev *hdev)
@@ -2873,7 +2867,7 @@ int hci_remove_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr)
 	return 0;
 }
 
-int hci_remote_oob_data_clear(struct hci_dev *hdev)
+void hci_remote_oob_data_clear(struct hci_dev *hdev)
 {
 	struct oob_data *data, *n;
 
@@ -2881,8 +2875,6 @@ int hci_remote_oob_data_clear(struct hci_dev *hdev)
 		list_del(&data->list);
 		kfree(data);
 	}
-
-	return 0;
 }
 
 int hci_add_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr,
@@ -2951,7 +2943,7 @@ struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev,
 	return NULL;
 }
 
-int hci_blacklist_clear(struct hci_dev *hdev)
+void hci_blacklist_clear(struct hci_dev *hdev)
 {
 	struct list_head *p, *n;
 
@@ -2961,8 +2953,6 @@ int hci_blacklist_clear(struct hci_dev *hdev)
 		list_del(p);
 		kfree(b);
 	}
-
-	return 0;
 }
 
 int hci_blacklist_add(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
@@ -2991,8 +2981,10 @@ int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
 {
 	struct bdaddr_list *entry;
 
-	if (!bacmp(bdaddr, BDADDR_ANY))
-		return hci_blacklist_clear(hdev);
+	if (!bacmp(bdaddr, BDADDR_ANY)) {
+		hci_blacklist_clear(hdev);
+		return 0;
+	}
 
 	entry = hci_blacklist_lookup(hdev, bdaddr, type);
 	if (!entry)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 473f8687b28b..fbb76a0de580 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2073,7 +2073,7 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
 	}
 
 	if (memcmp(cp->uuid, bt_uuid_any, 16) == 0) {
-		err = hci_uuids_clear(hdev);
+		hci_uuids_clear(hdev);
 
 		if (enable_service_cache(hdev)) {
 			err = cmd_complete(sk, hdev->id, MGMT_OP_REMOVE_UUID,
-- 
1.8.5.3

[PATCH 04/10] Bluetooth: Fix hci_remove_ltk failure when no match is found

[johan.hedberg]

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

There is code (in mgmt.c) that depends on the hci_remove_ltk function to
fail if no match is found. This patch adds tracking of removed LTKs
(there can be up to two: one for master and another for slave) in the
hci_remove_ltk function and returns -ENOENT of no matches were found.

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

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fd5bb4086613..69b7145bfce2 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2807,6 +2807,7 @@ int hci_remove_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr)
 int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 bdaddr_type)
 {
 	struct smp_ltk *k, *tmp;
+	int removed = 0;
 
 	list_for_each_entry_safe(k, tmp, &hdev->long_term_keys, list) {
 		if (bacmp(bdaddr, &k->bdaddr) || k->bdaddr_type != bdaddr_type)
@@ -2816,9 +2817,10 @@ int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 bdaddr_type)
 
 		list_del(&k->list);
 		kfree(k);
+		removed++;
 	}
 
-	return 0;
+	return removed ? 0 : -ENOENT;
 }
 
 /* HCI command timer function */
-- 
1.8.5.3

[PATCH 05/10] Bluetooth: Fix completing SMP as peripheral when no keys are expected

[johan.hedberg]

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

When we're the acceptors (peripheral/slave) of an SMP procedure and
we've completed distributing our keys we should only stick around
waiting for keys from the remote side if any of the initiator
distribution bits were actually set. This patch fixes the
smp_distribute_keys function to clear the SMP context when this
situation occurs.

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

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 6dcb35640b6f..54672c9ab6a5 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -1164,7 +1164,7 @@ int smp_distribute_keys(struct l2cap_conn *conn, __u8 force)
 		*keydist &= ~SMP_DIST_SIGN;
 	}
 
-	if (conn->hcon->out || force) {
+	if (conn->hcon->out || force || !(rsp->init_key_dist & 0x07)) {
 		clear_bit(HCI_CONN_LE_SMP_PEND, &conn->hcon->flags);
 		cancel_delayed_work_sync(&conn->security_timer);
 		smp_chan_destroy(conn);
-- 
1.8.5.3

[PATCH 06/10] Bluetooth: Fix removing any IRKs when unpairing devices

[johan.hedberg]

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

When mgmt_unpair_device is called we should also remove any associated
IRKs. This patch adds a hci_remove_irk convenience function and ensures
that it's called when mgmt_unpair_device is called.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_core.c         | 15 +++++++++++++++
 net/bluetooth/mgmt.c             |  2 ++
 3 files changed, 18 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 964a7888ad0c..ac468de11cb7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -801,6 +801,7 @@ struct smp_irk *hci_find_irk_by_addr(struct hci_dev *hdev, bdaddr_t *bdaddr,
 				     u8 addr_type);
 int hci_add_irk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 addr_type,
 		u8 val[16], bdaddr_t *rpa);
+void hci_remove_irk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 addr_type);
 void hci_smp_irks_clear(struct hci_dev *hdev);
 
 void hci_remote_oob_data_clear(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 69b7145bfce2..cdba4709f012 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2823,6 +2823,21 @@ int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 bdaddr_type)
 	return removed ? 0 : -ENOENT;
 }
 
+void hci_remove_irk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 addr_type)
+{
+	struct smp_irk *k, *tmp;
+
+	list_for_each_entry_safe(k, tmp, &hdev->long_term_keys, list) {
+		if (bacmp(bdaddr, &k->bdaddr) || k->addr_type != addr_type)
+			continue;
+
+		BT_DBG("%s removing %pMR", hdev->name, bdaddr);
+
+		list_del(&k->list);
+		kfree(k);
+	}
+}
+
 /* HCI command timer function */
 static void hci_cmd_timeout(unsigned long arg)
 {
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fbb76a0de580..90aac905a98b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2328,6 +2328,8 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 		else
 			addr_type = ADDR_LE_DEV_RANDOM;
 
+		hci_remove_irk(hdev, &cp->addr.bdaddr, addr_type);
+
 		err = hci_remove_ltk(hdev, &cp->addr.bdaddr, addr_type);
 	}
 
-- 
1.8.5.3

[PATCH 07/10] Bluetooth: Add convenience function for fetching IRKs

[johan.hedberg]

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

There are many situations where we need to check if an LE address is an
RPA and if so try to look up the IRK for it. To simplify such cases this
patch adds a convenience function for the job.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ac468de11cb7..4461c0051228 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1083,6 +1083,15 @@ static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
 	return false;
 }
 
+static inline struct smp_irk *hci_get_irk(struct hci_dev *hdev,
+					  bdaddr_t *bdaddr, u8 addr_type)
+{
+	if (!hci_bdaddr_is_rpa(bdaddr, addr_type))
+		return NULL;
+
+	return hci_find_irk_by_rpa(hdev, bdaddr);
+}
+
 int hci_register_cb(struct hci_cb *hcb);
 int hci_unregister_cb(struct hci_cb *hcb);
 
-- 
1.8.5.3

[PATCH 08/10] Bluetooth: Track the LE Identity Address in struct hci_conn

[johan.hedberg]

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

Since we want user space to see and use the LE Identity Address whenever
interfacing with the kernel it makes sense to track that instead of the
real address (the two will only be different in the case of an RPA).
This patch adds the necessary updates to when an LE connection gets
established and when receiving the Identity Address from a remote
device.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_event.c | 7 +++++++
 net/bluetooth/smp.c       | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d2c6878a9d6a..f31410a071b5 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3568,6 +3568,7 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_le_conn_complete *ev = (void *) skb->data;
 	struct hci_conn *conn;
+	struct smp_irk *irk;
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
 
@@ -3600,6 +3601,12 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		}
 	}
 
+	irk = hci_get_irk(hdev, &ev->bdaddr, ev->bdaddr_type);
+	if (irk) {
+		bacpy(&conn->dst, &irk->bdaddr);
+		conn->dst_type = irk->addr_type;
+	}
+
 	if (ev->status) {
 		mgmt_connect_failed(hdev, &conn->dst, conn->type,
 				    conn->dst_type, ev->status);
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 54672c9ab6a5..4d14ccc7b330 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -973,6 +973,9 @@ static int smp_cmd_ident_addr_info(struct l2cap_conn *conn,
 	hci_add_irk(conn->hcon->hdev, &smp->id_addr, smp->id_addr_type,
 		    smp->irk, &rpa);
 
+	bacpy(&hcon->dst, &smp->id_addr);
+	hcon->dst_type = smp->id_addr_type;
+
 	smp_distribute_keys(conn, 1);
 
 	return 0;
-- 
1.8.5.3

Re: [PATCH 08/10] Bluetooth: Track the LE Identity Address in struct hci_conn

[Marcel Holtmann]

Hi Johan,

> Since we want user space to see and use the LE Identity Address whenever
> interfacing with the kernel it makes sense to track that instead of the
> real address (the two will only be different in the case of an RPA).
> This patch adds the necessary updates to when an LE connection gets
> established and when receiving the Identity Address from a remote
> device.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_event.c | 7 +++++++
> net/bluetooth/smp.c       | 3 +++
> 2 files changed, 10 insertions(+)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index d2c6878a9d6a..f31410a071b5 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3568,6 +3568,7 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> {
> 	struct hci_ev_le_conn_complete *ev = (void *) skb->data;
> 	struct hci_conn *conn;
> +	struct smp_irk *irk;
> 
> 	BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
> 
> @@ -3600,6 +3601,12 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> 		}
> 	}
> 
> +	irk = hci_get_irk(hdev, &ev->bdaddr, ev->bdaddr_type);
> +	if (irk) {
> +		bacpy(&conn->dst, &irk->bdaddr);
> +		conn->dst_type = irk->addr_type;
> +	}
> +
> 	if (ev->status) {
> 		mgmt_connect_failed(hdev, &conn->dst, conn->type,
> 				    conn->dst_type, ev->status);
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 54672c9ab6a5..4d14ccc7b330 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -973,6 +973,9 @@ static int smp_cmd_ident_addr_info(struct l2cap_conn *conn,
> 	hci_add_irk(conn->hcon->hdev, &smp->id_addr, smp->id_addr_type,
> 		    smp->irk, &rpa);
> 
> +	bacpy(&hcon->dst, &smp->id_addr);
> +	hcon->dst_type = smp->id_addr_type;
> +

is this really safe to just overwrite the dst and dst_type. If we do that, we never really know the address that used for the actual connection.

For example what happens if now a stupid L2CAP connection gets intimated for the RPA or we have an incoming request for the same RPA. Can we really at this point say, we never need the RPA again?

Regards

Marcel

[PATCH 09/10] Bluetooth: Fix updating Identity Address in L2CAP channels

[johan.hedberg]

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

When we receive a remote identity address during SMP key distribution we
should ensure that any associated L2CAP channel instances get their
address information correspondingly updated (so that e.g. doing
getpeername on associated sockets returns the correct address).

This patch adds a new L2CAP core function l2cap_conn_update_id_addr()
which is used to iterate through all L2CAP channels associated with a
connection and update their address information.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/l2cap.h |  1 +
 net/bluetooth/l2cap_core.c    | 17 +++++++++++++++++
 net/bluetooth/smp.c           |  2 ++
 3 files changed, 20 insertions(+)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 13bec91785f4..4abdcb220e3a 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -881,6 +881,7 @@ int l2cap_ertm_init(struct l2cap_chan *chan);
 void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
 void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
 void l2cap_chan_del(struct l2cap_chan *chan, int err);
+void l2cap_conn_update_id_addr(struct hci_conn *hcon);
 void l2cap_send_conn_req(struct l2cap_chan *chan);
 void l2cap_move_start(struct l2cap_chan *chan);
 void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 6e6b3a9c8e6d..d25fefcc0bb5 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -609,6 +609,23 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
 	return;
 }
 
+void l2cap_conn_update_id_addr(struct hci_conn *hcon)
+{
+	struct l2cap_conn *conn = hcon->l2cap_data;
+	struct l2cap_chan *chan;
+
+	mutex_lock(&conn->chan_lock);
+
+	list_for_each_entry(chan, &conn->chan_l, list) {
+		l2cap_chan_lock(chan);
+		bacpy(&chan->dst, &hcon->dst);
+		chan->dst_type = hcon->dst_type;
+		l2cap_chan_unlock(chan);
+	}
+
+	mutex_unlock(&conn->chan_lock);
+}
+
 static void l2cap_chan_le_connect_reject(struct l2cap_chan *chan)
 {
 	struct l2cap_conn *conn = chan->conn;
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 4d14ccc7b330..faf04bd4263a 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -976,6 +976,8 @@ static int smp_cmd_ident_addr_info(struct l2cap_conn *conn,
 	bacpy(&hcon->dst, &smp->id_addr);
 	hcon->dst_type = smp->id_addr_type;
 
+	l2cap_conn_update_id_addr(hcon);
+
 	smp_distribute_keys(conn, 1);
 
 	return 0;
-- 
1.8.5.3

Re: [PATCH 09/10] Bluetooth: Fix updating Identity Address in L2CAP channels

[Marcel Holtmann]

Hi Johan,

> When we receive a remote identity address during SMP key distribution we
> should ensure that any associated L2CAP channel instances get their
> address information correspondingly updated (so that e.g. doing
> getpeername on associated sockets returns the correct address).
> 
> This patch adds a new L2CAP core function l2cap_conn_update_id_addr()
> which is used to iterate through all L2CAP channels associated with a
> connection and update their address information.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/l2cap.h |  1 +
> net/bluetooth/l2cap_core.c    | 17 +++++++++++++++++
> net/bluetooth/smp.c           |  2 ++
> 3 files changed, 20 insertions(+)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 13bec91785f4..4abdcb220e3a 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -881,6 +881,7 @@ int l2cap_ertm_init(struct l2cap_chan *chan);
> void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
> void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
> void l2cap_chan_del(struct l2cap_chan *chan, int err);
> +void l2cap_conn_update_id_addr(struct hci_conn *hcon);
> void l2cap_send_conn_req(struct l2cap_chan *chan);
> void l2cap_move_start(struct l2cap_chan *chan);
> void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 6e6b3a9c8e6d..d25fefcc0bb5 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -609,6 +609,23 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
> 	return;
> }
> 
> +void l2cap_conn_update_id_addr(struct hci_conn *hcon)
> +{
> +	struct l2cap_conn *conn = hcon->l2cap_data;
> +	struct l2cap_chan *chan;
> +
> +	mutex_lock(&conn->chan_lock);
> +
> +	list_for_each_entry(chan, &conn->chan_l, list) {
> +		l2cap_chan_lock(chan);
> +		bacpy(&chan->dst, &hcon->dst);
> +		chan->dst_type = hcon->dst_type;

this should use bdaddr_type(conn->hcon, conn->hcon->dst_type);

Remember that the L2CAP address type matches the mgmt one. While on hci_dev and hci_conn level we have the one from HCI specification.

> +		l2cap_chan_unlock(chan);
> +	}
> +
> +	mutex_unlock(&conn->chan_lock);
> +}
> +
> static void l2cap_chan_le_connect_reject(struct l2cap_chan *chan)
> {
> 	struct l2cap_conn *conn = chan->conn;
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 4d14ccc7b330..faf04bd4263a 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -976,6 +976,8 @@ static int smp_cmd_ident_addr_info(struct l2cap_conn *conn,
> 	bacpy(&hcon->dst, &smp->id_addr);
> 	hcon->dst_type = smp->id_addr_type;
> 
> +	l2cap_conn_update_id_addr(hcon);
> +
> 	smp_distribute_keys(conn, 1);
> 
> 	return 0;

Regards

Marcel

[PATCH 10/10] Bluetooth: Wait for SMP key distribution completion when pairing

[johan.hedberg]

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

When we initiate pairing through mgmt_pair_device the code has so far
been waiting for a successful HCI Encrypt Change event in order to
respond to the mgmt command. However, putting privacy into the play we
actually want the key distribution to be complete before replying so
that we can include the Identity Address in the mgmt response.

This patch updates the various hci_conn callbacks for LE in mgmt.c to
only respond in the case of failure, and adds a new mgmt_smp_complete
function that the SMP code will call once key distribution has been
completed.

Since the smp_chan_destroy function that's used to indicate completion
and clean up the SMP context can be called from various places,
including outside of smp.c, the easiest way to track failure vs success
is a new flag that we set once key distribution has been successfully
completed.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/mgmt.c             | 27 +++++++++++++++++++++------
 net/bluetooth/smp.c              |  5 +++++
 net/bluetooth/smp.h              |  1 +
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4461c0051228..64c4e3f0a515 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1212,6 +1212,7 @@ int mgmt_device_blocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
 int mgmt_device_unblocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
 void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, u8 persistent);
 void mgmt_reenable_advertising(struct hci_dev *hdev);
+void mgmt_smp_complete(struct hci_conn *conn, bool complete);
 
 /* HCI info for socket */
 #define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 90aac905a98b..58d7cc39f035 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2655,6 +2655,18 @@ static void pairing_complete(struct pending_cmd *cmd, u8 status)
 	mgmt_pending_remove(cmd);
 }
 
+void mgmt_smp_complete(struct hci_conn *conn, bool complete)
+{
+	u8 status = complete ? MGMT_STATUS_SUCCESS : MGMT_STATUS_FAILED;
+	struct pending_cmd *cmd;
+
+	cmd = find_pairing(conn);
+	if (!cmd)
+		BT_DBG("Unable to find a pending command");
+	else
+		pairing_complete(cmd, mgmt_status(status));
+}
+
 static void pairing_complete_cb(struct hci_conn *conn, u8 status)
 {
 	struct pending_cmd *cmd;
@@ -2668,7 +2680,7 @@ static void pairing_complete_cb(struct hci_conn *conn, u8 status)
 		pairing_complete(cmd, mgmt_status(status));
 }
 
-static void le_connect_complete_cb(struct hci_conn *conn, u8 status)
+static void le_pairing_complete_cb(struct hci_conn *conn, u8 status)
 {
 	struct pending_cmd *cmd;
 
@@ -2755,13 +2767,16 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	}
 
 	/* For LE, just connecting isn't a proof that the pairing finished */
-	if (cp->addr.type == BDADDR_BREDR)
+	if (cp->addr.type == BDADDR_BREDR) {
 		conn->connect_cfm_cb = pairing_complete_cb;
-	else
-		conn->connect_cfm_cb = le_connect_complete_cb;
+		conn->security_cfm_cb = pairing_complete_cb;
+		conn->disconn_cfm_cb = pairing_complete_cb;
+	} else {
+		conn->connect_cfm_cb = le_pairing_complete_cb;
+		conn->security_cfm_cb = le_pairing_complete_cb;
+		conn->disconn_cfm_cb = le_pairing_complete_cb;
+	}
 
-	conn->security_cfm_cb = pairing_complete_cb;
-	conn->disconn_cfm_cb = pairing_complete_cb;
 	conn->io_capability = cp->io_cap;
 	cmd->user_data = conn;
 
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index faf04bd4263a..9741623de6e7 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -551,9 +551,13 @@ static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)
 void smp_chan_destroy(struct l2cap_conn *conn)
 {
 	struct smp_chan *smp = conn->smp_chan;
+	bool complete;
 
 	BUG_ON(!smp);
 
+	complete = test_bit(SMP_FLAG_COMPLETE, &smp->smp_flags);
+	mgmt_smp_complete(conn->hcon, complete);
+
 	kfree(smp);
 	conn->smp_chan = NULL;
 	conn->hcon->smp_conn = NULL;
@@ -1172,6 +1176,7 @@ int smp_distribute_keys(struct l2cap_conn *conn, __u8 force)
 	if (conn->hcon->out || force || !(rsp->init_key_dist & 0x07)) {
 		clear_bit(HCI_CONN_LE_SMP_PEND, &conn->hcon->flags);
 		cancel_delayed_work_sync(&conn->security_timer);
+		set_bit(SMP_FLAG_COMPLETE, &smp->smp_flags);
 		smp_chan_destroy(conn);
 	}
 
diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
index 8f54c9b152de..675fd3b21d2c 100644
--- a/net/bluetooth/smp.h
+++ b/net/bluetooth/smp.h
@@ -118,6 +118,7 @@ struct smp_cmd_security_req {
 #define SMP_FLAG_TK_VALID	1
 #define SMP_FLAG_CFM_PENDING	2
 #define SMP_FLAG_MITM_AUTH	3
+#define SMP_FLAG_COMPLETE	4
 
 struct smp_chan {
 	struct l2cap_conn *conn;
-- 
1.8.5.3

[PATCH v2 10/10] Bluetooth: Wait for SMP key distribution completion when pairing

[johan.hedberg]

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

When we initiate pairing through mgmt_pair_device the code has so far
been waiting for a successful HCI Encrypt Change event in order to
respond to the mgmt command. However, putting privacy into the play we
actually want the key distribution to be complete before replying so
that we can include the Identity Address in the mgmt response.

This patch updates the various hci_conn callbacks for LE in mgmt.c to
only respond in the case of failure, and adds a new mgmt_smp_complete
function that the SMP code will call once key distribution has been
completed.

Since the smp_chan_destroy function that's used to indicate completion
and clean up the SMP context can be called from various places,
including outside of smp.c, the easiest way to track failure vs success
is a new flag that we set once key distribution has been successfully
completed.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
v2: Fix passing correct status value to pairing_complete()

 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/mgmt.c             | 27 +++++++++++++++++++++------
 net/bluetooth/smp.c              |  5 +++++
 net/bluetooth/smp.h              |  1 +
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4461c0051228..64c4e3f0a515 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1212,6 +1212,7 @@ int mgmt_device_blocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
 int mgmt_device_unblocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
 void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, u8 persistent);
 void mgmt_reenable_advertising(struct hci_dev *hdev);
+void mgmt_smp_complete(struct hci_conn *conn, bool complete);
 
 /* HCI info for socket */
 #define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 90aac905a98b..f9ae72ca556d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2655,6 +2655,18 @@ static void pairing_complete(struct pending_cmd *cmd, u8 status)
 	mgmt_pending_remove(cmd);
 }
 
+void mgmt_smp_complete(struct hci_conn *conn, bool complete)
+{
+	u8 status = complete ? MGMT_STATUS_SUCCESS : MGMT_STATUS_FAILED;
+	struct pending_cmd *cmd;
+
+	cmd = find_pairing(conn);
+	if (!cmd)
+		BT_DBG("Unable to find a pending command");
+	else
+		pairing_complete(cmd, status);
+}
+
 static void pairing_complete_cb(struct hci_conn *conn, u8 status)
 {
 	struct pending_cmd *cmd;
@@ -2668,7 +2680,7 @@ static void pairing_complete_cb(struct hci_conn *conn, u8 status)
 		pairing_complete(cmd, mgmt_status(status));
 }
 
-static void le_connect_complete_cb(struct hci_conn *conn, u8 status)
+static void le_pairing_complete_cb(struct hci_conn *conn, u8 status)
 {
 	struct pending_cmd *cmd;
 
@@ -2755,13 +2767,16 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	}
 
 	/* For LE, just connecting isn't a proof that the pairing finished */
-	if (cp->addr.type == BDADDR_BREDR)
+	if (cp->addr.type == BDADDR_BREDR) {
 		conn->connect_cfm_cb = pairing_complete_cb;
-	else
-		conn->connect_cfm_cb = le_connect_complete_cb;
+		conn->security_cfm_cb = pairing_complete_cb;
+		conn->disconn_cfm_cb = pairing_complete_cb;
+	} else {
+		conn->connect_cfm_cb = le_pairing_complete_cb;
+		conn->security_cfm_cb = le_pairing_complete_cb;
+		conn->disconn_cfm_cb = le_pairing_complete_cb;
+	}
 
-	conn->security_cfm_cb = pairing_complete_cb;
-	conn->disconn_cfm_cb = pairing_complete_cb;
 	conn->io_capability = cp->io_cap;
 	cmd->user_data = conn;
 
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index faf04bd4263a..9741623de6e7 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -551,9 +551,13 @@ static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)
 void smp_chan_destroy(struct l2cap_conn *conn)
 {
 	struct smp_chan *smp = conn->smp_chan;
+	bool complete;
 
 	BUG_ON(!smp);
 
+	complete = test_bit(SMP_FLAG_COMPLETE, &smp->smp_flags);
+	mgmt_smp_complete(conn->hcon, complete);
+
 	kfree(smp);
 	conn->smp_chan = NULL;
 	conn->hcon->smp_conn = NULL;
@@ -1172,6 +1176,7 @@ int smp_distribute_keys(struct l2cap_conn *conn, __u8 force)
 	if (conn->hcon->out || force || !(rsp->init_key_dist & 0x07)) {
 		clear_bit(HCI_CONN_LE_SMP_PEND, &conn->hcon->flags);
 		cancel_delayed_work_sync(&conn->security_timer);
+		set_bit(SMP_FLAG_COMPLETE, &smp->smp_flags);
 		smp_chan_destroy(conn);
 	}
 
diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
index 8f54c9b152de..675fd3b21d2c 100644
--- a/net/bluetooth/smp.h
+++ b/net/bluetooth/smp.h
@@ -118,6 +118,7 @@ struct smp_cmd_security_req {
 #define SMP_FLAG_TK_VALID	1
 #define SMP_FLAG_CFM_PENDING	2
 #define SMP_FLAG_MITM_AUTH	3
+#define SMP_FLAG_COMPLETE	4
 
 struct smp_chan {
 	struct l2cap_conn *conn;
-- 
1.8.5.3

Re: [PATCH v2 10/10] Bluetooth: Wait for SMP key distribution completion when pairing

[Marcel Holtmann]

Hi Johan,

> When we initiate pairing through mgmt_pair_device the code has so far
> been waiting for a successful HCI Encrypt Change event in order to
> respond to the mgmt command. However, putting privacy into the play we
> actually want the key distribution to be complete before replying so
> that we can include the Identity Address in the mgmt response.
> 
> This patch updates the various hci_conn callbacks for LE in mgmt.c to
> only respond in the case of failure, and adds a new mgmt_smp_complete
> function that the SMP code will call once key distribution has been
> completed.
> 
> Since the smp_chan_destroy function that's used to indicate completion
> and clean up the SMP context can be called from various places,
> including outside of smp.c, the easiest way to track failure vs success
> is a new flag that we set once key distribution has been successfully
> completed.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> v2: Fix passing correct status value to pairing_complete()
> 
> include/net/bluetooth/hci_core.h |  1 +
> net/bluetooth/mgmt.c             | 27 +++++++++++++++++++++------
> net/bluetooth/smp.c              |  5 +++++
> net/bluetooth/smp.h              |  1 +
> 4 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 4461c0051228..64c4e3f0a515 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1212,6 +1212,7 @@ int mgmt_device_blocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
> int mgmt_device_unblocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
> void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, u8 persistent);
> void mgmt_reenable_advertising(struct hci_dev *hdev);
> +void mgmt_smp_complete(struct hci_conn *conn, bool complete);
> 
> /* HCI info for socket */
> #define hci_pi(sk) ((struct hci_pinfo *) sk)
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 90aac905a98b..f9ae72ca556d 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -2655,6 +2655,18 @@ static void pairing_complete(struct pending_cmd *cmd, u8 status)
> 	mgmt_pending_remove(cmd);
> }
> 
> +void mgmt_smp_complete(struct hci_conn *conn, bool complete)
> +{
> +	u8 status = complete ? MGMT_STATUS_SUCCESS : MGMT_STATUS_FAILED;
> +	struct pending_cmd *cmd;
> +
> +	cmd = find_pairing(conn);
> +	if (!cmd)
> +		BT_DBG("Unable to find a pending command”);

why do we bother with the debug print here?

> +	else
> +		pairing_complete(cmd, status);

I would just do this:

	if (cmd)
		pairing_complete(cmd, status);

Unless you have future patches that do more work here.

> +}
> +
> static void pairing_complete_cb(struct hci_conn *conn, u8 status)
> {
> 	struct pending_cmd *cmd;
> @@ -2668,7 +2680,7 @@ static void pairing_complete_cb(struct hci_conn *conn, u8 status)
> 		pairing_complete(cmd, mgmt_status(status));
> }
> 
> -static void le_connect_complete_cb(struct hci_conn *conn, u8 status)
> +static void le_pairing_complete_cb(struct hci_conn *conn, u8 status)
> {
> 	struct pending_cmd *cmd;
> 
> @@ -2755,13 +2767,16 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
> 	}
> 
> 	/* For LE, just connecting isn't a proof that the pairing finished */
> -	if (cp->addr.type == BDADDR_BREDR)
> +	if (cp->addr.type == BDADDR_BREDR) {
> 		conn->connect_cfm_cb = pairing_complete_cb;
> -	else
> -		conn->connect_cfm_cb = le_connect_complete_cb;
> +		conn->security_cfm_cb = pairing_complete_cb;
> +		conn->disconn_cfm_cb = pairing_complete_cb;
> +	} else {
> +		conn->connect_cfm_cb = le_pairing_complete_cb;
> +		conn->security_cfm_cb = le_pairing_complete_cb;
> +		conn->disconn_cfm_cb = le_pairing_complete_cb;
> +	}
> 
> -	conn->security_cfm_cb = pairing_complete_cb;
> -	conn->disconn_cfm_cb = pairing_complete_cb;
> 	conn->io_capability = cp->io_cap;
> 	cmd->user_data = conn;
> 
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index faf04bd4263a..9741623de6e7 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -551,9 +551,13 @@ static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)
> void smp_chan_destroy(struct l2cap_conn *conn)
> {
> 	struct smp_chan *smp = conn->smp_chan;
> +	bool complete;
> 
> 	BUG_ON(!smp);
> 
> +	complete = test_bit(SMP_FLAG_COMPLETE, &smp->smp_flags);
> +	mgmt_smp_complete(conn->hcon, complete);
> +
> 	kfree(smp);
> 	conn->smp_chan = NULL;
> 	conn->hcon->smp_conn = NULL;
> @@ -1172,6 +1176,7 @@ int smp_distribute_keys(struct l2cap_conn *conn, __u8 force)
> 	if (conn->hcon->out || force || !(rsp->init_key_dist & 0x07)) {
> 		clear_bit(HCI_CONN_LE_SMP_PEND, &conn->hcon->flags);
> 		cancel_delayed_work_sync(&conn->security_timer);
> +		set_bit(SMP_FLAG_COMPLETE, &smp->smp_flags);
> 		smp_chan_destroy(conn);
> 	}
> 
> diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
> index 8f54c9b152de..675fd3b21d2c 100644
> --- a/net/bluetooth/smp.h
> +++ b/net/bluetooth/smp.h
> @@ -118,6 +118,7 @@ struct smp_cmd_security_req {
> #define SMP_FLAG_TK_VALID	1
> #define SMP_FLAG_CFM_PENDING	2
> #define SMP_FLAG_MITM_AUTH	3
> +#define SMP_FLAG_COMPLETE	4
> 
> struct smp_chan {
> 	struct l2cap_conn *conn;

Regards

Marcel

Re: [PATCH v2 10/10] Bluetooth: Wait for SMP key distribution completion when pairing

[Johan Hedberg]

Hi Marcel,

On Tue, Feb 18, 2014, Marcel Holtmann wrote:
> > +	cmd = find_pairing(conn);
> > +	if (!cmd)
> > +		BT_DBG("Unable to find a pending command”);
> 
> why do we bother with the debug print here?
> 
> > +	else
> > +		pairing_complete(cmd, status);
> 
> I would just do this:
> 
> 	if (cmd)
> 		pairing_complete(cmd, status);
> 
> Unless you have future patches that do more work here.

The main reason was to get some idea how frequently this function gets
called unnecessarily, but you're right in that it's kind of useless.
I've removed it in my latest set.

Johan

Re: [PATCH 00/10] Bluetooth: More patches for privacy

[Marcel Holtmann]

Hi Johan,

> Here are more patches towards implementing support for privacy. Included
> are also some cleanup and fix patches for issues I came accross when
> dealing with the code.
> 
> Johan
> 
> ----------------------------------------------------------------
> Johan Hedberg (10):
>      Bluetooth: Remove SMP data specific crypto context
>      Bluetooth: Fix missing address type check for removing LTKs
>      Bluetooth: Remove return values from functions that don't need them
>      Bluetooth: Fix hci_remove_ltk failure when no match is found
>      Bluetooth: Fix completing SMP as peripheral when no keys are expected
>      Bluetooth: Fix removing any IRKs when unpairing devices
>      Bluetooth: Add convenience function for fetching IRKs
>      Bluetooth: Track the LE Identity Address in struct hci_conn
>      Bluetooth: Fix updating Identity Address in L2CAP channels
>      Bluetooth: Wait for SMP key distribution completion when pairing
> 
> include/net/bluetooth/hci_core.h | 23 +++++++++++++-----
> include/net/bluetooth/l2cap.h    |  1 +
> net/bluetooth/hci_core.c         | 49 ++++++++++++++++++++++----------------
> net/bluetooth/hci_event.c        |  7 ++++++
> net/bluetooth/l2cap_core.c       | 17 +++++++++++++
> net/bluetooth/mgmt.c             | 45 ++++++++++++++++++++++++++--------
> net/bluetooth/smp.c              | 25 ++++++++++---------
> net/bluetooth/smp.h              |  2 +-
> 8 files changed, 119 insertions(+), 50 deletions(-)

I applied patches 2-7 to bluetooth-next now. For the rest I have questions/comments.

Regards

Marcel