2020-03-12 10:07:35

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 0/7] LE LL Priavcy support enabled

With this LE LL Priavcy feature available if BT controller supports
LL Privacy when privacy is enabled.

The patches are verified in below case.
Pairing - where the IRK is added to device Resolving List after
identity info & IRK is shared in SMP.
Unpair - Delete the resolving entry from the Device resolving list
Local IRK change - Whenever there is change in local IRK (privacy) the
resolving list entries in the device is updated.
Whitelist - filter policies for whitelist is updated based on LL_adress
resolution support.
Reload Resolving list - device reset/sysmte reboot the Resolving list
reloaded in to the device.

Sathish Narsimman (7):
Bluetooth: LL Privacy Delete Store Resolving list
Bluetooth: Add device to Resolving List
Bluetooth: Update the Resolving list when Local IRK changed
Bluetooth: LL_PRIVACY re-load resolving list
Bluetooth: LL Privacy Check and Update the Resolving List
Bluetooth: Enable LL Privacy
Bluetooth LL Privacy update White list

include/net/bluetooth/hci.h | 3 +
include/net/bluetooth/hci_core.h | 7 +
net/bluetooth/hci_conn.c | 32 ++++
net/bluetooth/hci_core.c | 20 ++-
net/bluetooth/hci_event.c | 32 +++-
net/bluetooth/hci_request.c | 251 ++++++++++++++++++++++++++++++-
net/bluetooth/hci_request.h | 2 +
net/bluetooth/mgmt.c | 10 +-
net/bluetooth/smp.c | 11 ++
9 files changed, 354 insertions(+), 14 deletions(-)

--
2.17.1


2020-03-12 10:07:41

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 2/7] Bluetooth: Add device to Resolving List

The Patch is used to add the device to resolving list. The patch
will look for the local IRK list and if the device is present in
the local resolving list it will be updated and the same updated
w.r.t to BT controller by first deleting the existing and adding
the new IRK w.r.t bd_addr.

Signed-off-by: Sathish Narsimman <[email protected]>
Signed-off-by: Joy Shermin <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 +
net/bluetooth/hci_request.c | 80 ++++++++++++++++++++++++++++++++
2 files changed, 82 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5f04ef88da35..e5e09d530ce7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1657,6 +1657,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __le64 rand,

void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
u8 *bdaddr_type);
+int hci_req_update_resolving_list(struct hci_dev *hdev, u8 addr_type,
+ bdaddr_t *bdaddr, u8 irk[16]);
void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
bdaddr_t *bdaddr);

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index f4bbd3b79210..88225a9ca1f8 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -885,6 +885,86 @@ static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
}
}

+int hci_req_update_resolving_list(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
+ u8 irk_val[16])
+{
+ struct hci_request req;
+ struct hci_cp_le_add_to_resolv_list cp;
+ struct bdaddr_list_with_irk *irk;
+ u8 entries;
+
+ BT_DBG("");
+
+ /* Nothing to be done if LL privacy is not supported */
+ if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY) )
+ return -EPROTONOSUPPORT;
+
+ /* Resolving List cannot be updated if address resolution
+ * in the controller is enabled and advertisement or scanning
+ * or create connection command is ongoing.
+ */
+ if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
+ ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
+ hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
+ hci_lookup_le_connect(hdev) ) )
+ return -EINVAL;
+
+ hci_req_init(&req, hdev);
+
+ irk = hci_bdaddr_list_lookup_with_irk(&hdev->le_resolv_list, bdaddr, type);
+ if (irk) {
+ /* Device is present in resolving list.*/
+ if (memcmp(irk->peer_irk, irk_val, 16) == 0) {
+ /* Device present in resolving list with same IRK.
+ * No HCI communication is required.Sort the
+ * Kernel list.
+ */
+ hci_bdaddr_list_add_with_irk(&irk->list, bdaddr, type, irk_val, NULL);
+ return 0;
+
+ }
+ /* IRK has changed for the device in resolving list
+ * Queue up commands to delete the existing entry and
+ * add new one. Sorting will be done when command complete
+ * for add command is receieved.
+ */
+ goto remote;
+ }
+ /* Device is not present in resolving list.If resolving list
+ * is not full add the device to resolving list.
+ */
+ entries = 0;
+ list_for_each_entry(irk, &hdev->le_resolv_list, list) {
+ entries++;
+ }
+
+ if (entries < hdev->le_resolv_list_size)
+ goto add;
+
+ /* If the resolving list is full, queue up HCI command to delete
+ * the entry that was used least recently to make space for the
+ * new device. Kernel list will be updated when command complete
+ * is received.
+ */
+ irk = list_first_entry_or_null(&hdev->le_resolv_list, struct bdaddr_list_with_irk, list);
+
+ if (!irk)
+ return -1;
+
+remote:
+ hci_req_del_from_resolving_list(hdev, irk->bdaddr_type, &irk->bdaddr);
+
+add:
+ cp.bdaddr_type = type;
+ bacpy(&cp.bdaddr, bdaddr);
+ memcpy(cp.peer_irk, irk_val, 16);
+ memcpy(cp.local_irk, hdev->irk, 16);
+
+ hci_req_add(&req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
+ hci_req_run(&req, NULL);
+ return 0;
+}
+
void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type, bdaddr_t *bdaddr)
{
struct hci_cp_le_del_from_resolv_list cp;
--
2.17.1

2020-03-12 10:07:45

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 4/7] Bluetooth: LL_PRIVACY re-load resolving list

The patch is used to load the entries in the local IRK to the
Bluetooth LE controller resolving list.

Signed-off-by: Sathish Narsimman <[email protected]>
Signed-off-by: Joy Shermin <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_request.c | 29 +++++++++++++++++++++++++++++
2 files changed, 30 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 43d31a9339a6..46d2d3e10c97 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1659,6 +1659,7 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
u8 *bdaddr_type);
int hci_req_update_resolving_list(struct hci_dev *hdev, u8 addr_type,
bdaddr_t *bdaddr, u8 irk[16]);
+void hci_load_resolving_list(struct hci_dev *hdev);
void hci_req_update_resolving_list_local_irk(struct hci_dev *hdev);

void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 9ffb62178d24..e17db3103a3d 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1015,6 +1015,35 @@ void hci_req_update_resolving_list_local_irk(struct hci_dev *hdev)
hci_req_run(&req, NULL);
}

+void hci_load_resolving_list (struct hci_dev *hdev)
+{
+ struct smp_irk *irk;
+ u8 num = 0;
+
+ /* Nothing to be done if LL privacy is not supported */
+ if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY) )
+ return;
+
+ if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
+ ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
+ hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
+ hci_lookup_le_connect(hdev) ) )
+ return;
+
+ /* Load the first le_resolving_list_size entries from IRK
+ * list in to resolving list.
+ */
+ rcu_read_lock();
+ list_for_each_entry_rcu(irk, &hdev->identity_resolving_keys, list) {
+ if (num++ >= hdev->le_resolv_list_size)
+ return;
+
+ hci_req_update_resolving_list(hdev, irk->addr_type, &irk->bdaddr, irk->val);
+ }
+
+ rcu_read_unlock();
+}
+
void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type, bdaddr_t *bdaddr)
{
struct hci_cp_le_del_from_resolv_list cp;
--
2.17.1

2020-03-12 10:07:51

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 6/7] Bluetooth: Enable LL Privacy

This enables Link Layer Privacy and sets RPA timeout to 900sec

< HCI Command: LE Set Address Resolution Enable (0x08|0x002d) plen 1
Address resolution: Enabled (0x01)
> HCI Event: Command Complete (0x0e) plen 4
LE Set Address Resolution Enable (0x08|0x002d) ncmd 1d
Status: Success (0x00)
< HCI Command: LE Set Resolvable Private Address Timeout (0x08|0x002e) plen 2
Timeout: 900 seconds
> HCI Event: Command Complete (0x0e) plen 4
LE Set Resolvable Private Address Timeout (0x08|0x002e) ncmd 1
Status: Success (0x00)

During Successfull pairing adding the information to device Resolving
list

< HCI Command: LE Add Device To Resolving List (0x08|0x0027) plen 39
Address type: Random (0x01)
Address: D1:79:92:B1:83:DF (Static)
Peer identity resolving key: ecc317056e5b385cda281b9c24a453e9
Local identity resolving key: 00000000000000000000000000000000

< ACL Data TX: Handle 3585 flags 0x00 dlen 21
SMP: Signing Information (0x0a) len 16
Signature key: 37b6c45656c7e8b3ec8db062c3e0c2b2
> ACL Data RX: Handle 3585 flags 0x02 dlen 9
ATT: Error Response (0x01) len 4
Read By Group Type Request (0x10)
Handle: 0x0001
Error: Attribute Not Found (0x0a)
> ACL Data RX: Handle 3585 flags 0x02 dlen 11
ATT: Read By Group Type Request (0x10) len 6
Handle range: 0x000a-0xffff
Attribute group type: Primary Service (0x2800)

> HCI Event: Command Complete (0x0e) plen 4
LE Add Device To Resolving List (0x08|0x0027) ncmd 1
Status: Success (0x00)

Signed-off-by: Sathish Narsimman <[email protected]>
---
include/net/bluetooth/hci.h | 2 ++
net/bluetooth/hci_conn.c | 32 ++++++++++++++++++++++++++++++++
net/bluetooth/hci_core.c | 20 +++++++++++++++++++-
net/bluetooth/hci_event.c | 32 +++++++++++++++++++++++++-------
net/bluetooth/hci_request.c | 16 +++++++++++++---
net/bluetooth/mgmt.c | 10 +++++++++-
net/bluetooth/smp.c | 11 +++++++++++
7 files changed, 111 insertions(+), 12 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 352bc43940ff..d8431ebb8baa 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1617,6 +1617,8 @@ struct hci_rp_le_read_resolv_list_size {

#define HCI_OP_LE_SET_ADDR_RESOLV_ENABLE 0x202d

+#define HCI_OP_LE_SET_RPA_TIMEOUT 0x202e
+
#define HCI_OP_LE_READ_MAX_DATA_LEN 0x202f
struct hci_rp_le_read_max_data_len {
__u8 status;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index e245bc155cc2..06067c0852a6 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -830,6 +830,14 @@ static void hci_req_add_le_create_conn(struct hci_request *req,

plen = sizeof(*cp);

+ /* If the own_addr_type is 0x03 or 0x02 and peer is present
+ * in Kernel IRK list, but not in resolving add the peer to
+ * the resolving list.
+ */
+ if (own_addr_type == 0x03 || own_addr_type == 0x02)
+ hci_req_check_and_update_resolving_list(hdev, conn->dst_type,
+ &conn->dst);
+
if (scan_1m(hdev)) {
cp->phys |= LE_SCAN_PHY_1M;
set_ext_conn_params(conn, p);
@@ -866,6 +874,14 @@ static void hci_req_add_le_create_conn(struct hci_request *req,
cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
cp.scan_window = cp.scan_interval;

+ /* If the own_addr_type is 0x03 or 0x02 and peer is present
+ * in Kernel IRK list, but not in resolving add the peer to
+ * the resolving list.
+ */
+ if (own_addr_type == 0x03 || own_addr_type == 0x02)
+ hci_req_check_and_update_resolving_list(hdev, conn->dst_type,
+ &conn->dst);
+
bacpy(&cp.peer_addr, &conn->dst);
cp.peer_addr_type = conn->dst_type;
cp.own_address_type = own_addr_type;
@@ -901,6 +917,14 @@ static void hci_req_directed_advertising(struct hci_request *req,
&own_addr_type, &random_addr) < 0)
return;

+ /* If the own_addr_type is 0x03 or 0x02 and peer is present
+ * in Kernel IRK list, but not in resolving add the peer to
+ * the resolving list.
+ */
+ if (own_addr_type == 0x03 || own_addr_type == 0x03)
+ hci_req_check_and_update_resolving_list(hdev, conn->dst_type,
+ &conn->dst);
+
memset(&cp, 0, sizeof(cp));

cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_DIRECT_IND);
@@ -959,6 +983,14 @@ static void hci_req_directed_advertising(struct hci_request *req,
&own_addr_type) < 0)
return;

+ /* If the own_addr_type is 0x03 or 0x02 and peer is present
+ * in Kernel IRK list, but not in resolving add the peer to
+ * the resolving list.
+ */
+ if (own_addr_type == 0x03 || own_addr_type == 0x02)
+ hci_req_check_and_update_resolving_list(hdev, conn->dst_type,
+ &conn->dst);
+
memset(&cp, 0, sizeof(cp));

/* Some controllers might reject command if intervals are not
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index dbd2ad3a26ed..7727be922084 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -713,7 +713,7 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
/* If the controller supports the LE Extended Create Connection
* command, enable the corresponding event.
*/
- if (use_ext_conn(hdev))
+ if (use_ext_conn(hdev) || hdev->le_features[0] & HCI_LE_LL_PRIVACY)
events[1] |= 0x02; /* LE Enhanced Connection
* Complete
*/
@@ -762,6 +762,24 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_CLEAR_RESOLV_LIST, 0, NULL);
}

+ if (hdev->commands[35] & 0x02) {
+ u8 addr_resoln_enable = 0x01;
+
+ /* Enable Address Resolution in controller */
+ hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE,
+ sizeof(addr_resoln_enable),
+ &addr_resoln_enable);
+ }
+
+ if (hdev->commands[35] & 0x40) {
+ __le16 rpa_timeout = cpu_to_le16(hdev->rpa_timeout);
+
+ /* Set RPA timeout */
+ hci_req_add(req, HCI_OP_LE_SET_RPA_TIMEOUT, 2,
+ &rpa_timeout);
+
+ }
+
if (hdev->le_features[0] & HCI_LE_DATA_LEN_EXT) {
/* Read LE Maximum Data Length */
hci_req_add(req, HCI_OP_LE_READ_MAX_DATA_LEN, 0, NULL);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 20408d386268..d5ce1921fadd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1684,7 +1684,11 @@ static void hci_cc_set_adv_param(struct hci_dev *hdev, struct sk_buff *skb)
return;

hci_dev_lock(hdev);
- hdev->adv_addr_type = cp->own_address_type;
+
+ if (cp->own_address_type == ADDR_LE_DEV_PUBLIC)
+ hdev->adv_addr_type = ADDR_LE_DEV_PUBLIC;
+ else
+ hdev->adv_addr_type = ADDR_LE_DEV_RANDOM;
hci_dev_unlock(hdev);
}

@@ -1704,7 +1708,12 @@ static void hci_cc_set_ext_adv_param(struct hci_dev *hdev, struct sk_buff *skb)
return;

hci_dev_lock(hdev);
- hdev->adv_addr_type = cp->own_addr_type;
+
+ if (cp->own_addr_type == ADDR_LE_DEV_PUBLIC)
+ hdev->adv_addr_type = ADDR_LE_DEV_PUBLIC;
+ else
+ hdev->adv_addr_type = ADDR_LE_DEV_RANDOM;
+
if (!hdev->cur_adv_instance) {
/* Store in hdev for instance 0 */
hdev->adv_tx_power = rp->tx_power;
@@ -2267,11 +2276,13 @@ static void cs_le_create_conn(struct hci_dev *hdev, bdaddr_t *peer_addr,
* is needed for SMP. These values will not change during the
* lifetime of the connection.
*/
- conn->init_addr_type = own_address_type;
- if (own_address_type == ADDR_LE_DEV_RANDOM)
+ if (own_address_type == 0x03 || own_address_type == 0x01) {
+ conn->init_addr_type = ADDR_LE_DEV_RANDOM;
bacpy(&conn->init_addr, &hdev->random_addr);
- else
+ } else {
+ conn->init_addr_type = ADDR_LE_DEV_PUBLIC;
bacpy(&conn->init_addr, &hdev->bdaddr);
+ }

conn->resp_addr_type = peer_addr_type;
bacpy(&conn->resp_addr, peer_addr);
@@ -5141,6 +5152,8 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
}
}

+ hci_req_check_and_update_resolving_list(hdev, (conn->dst_type), &conn->dst);
+
unlock:
hci_update_background_scan(hdev);
hci_dev_unlock(hdev);
@@ -5365,8 +5378,13 @@ static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
/* Only resolvable random addresses are valid for these
* kind of reports and others can be ignored.
*/
- if (!hci_bdaddr_is_rpa(direct_addr, direct_addr_type))
- return;
+ if (!hci_bdaddr_is_rpa(direct_addr, direct_addr_type)) {
+ /* Controller with LL Privacy Supported tend to resolve
+ * the RPA. In that case we should ignore this condition
+ */
+ if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY))
+ return;
+ }

/* If the controller is not using resolvable random
* addresses, then this report can be ignored.
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 6b685cdc7de5..12868caf50ea 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1916,7 +1916,12 @@ int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,
if (use_rpa) {
int to;

- *own_addr_type = ADDR_LE_DEV_RANDOM;
+ if (hdev->le_features[0] & HCI_LE_LL_PRIVACY) {
+ /*if there is no resolving list used in the controller
+ * use the bd_addr from latest Le_set_random_address*/
+ *own_addr_type = 0x03;
+ } else
+ *own_addr_type = ADDR_LE_DEV_RANDOM;

if (adv_instance) {
if (!adv_instance->rpa_expired &&
@@ -2074,7 +2079,7 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)

hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp), &cp);

- if (own_addr_type == ADDR_LE_DEV_RANDOM &&
+ if (((own_addr_type == ADDR_LE_DEV_RANDOM) || (own_addr_type == 0x03)) &&
bacmp(&random_addr, BDADDR_ANY)) {
struct hci_cp_le_set_adv_set_rand_addr cp;

@@ -2341,7 +2346,12 @@ int hci_update_random_address(struct hci_request *req, bool require_privacy,
if (use_rpa) {
int to;

- *own_addr_type = ADDR_LE_DEV_RANDOM;
+ if (hdev->le_features[0] & HCI_LE_LL_PRIVACY) {
+ /*if there is no resolving list used in the controller
+ * use the bd_addr from latest Le_set_random_address*/
+ *own_addr_type = 0x03;
+ } else
+ *own_addr_type = ADDR_LE_DEV_RANDOM;

if (!hci_dev_test_and_clear_flag(hdev, HCI_RPA_EXPIRED) &&
!bacmp(&hdev->random_addr, &hdev->rpa))
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 6552003a170e..2b2bc011a130 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5084,8 +5084,10 @@ static int set_debug_keys(struct sock *sk, struct hci_dev *hdev,
if (err < 0)
goto unlock;

- if (changed)
+ if (changed) {
err = new_settings(hdev, sk);
+ hci_req_update_resolving_list_local_irk(hdev);
+ }

unlock:
hci_dev_unlock(hdev);
@@ -5227,6 +5229,8 @@ static int load_irks(struct sock *sk, struct hci_dev *hdev, void *cp_data,
BDADDR_ANY);
}

+ /* Load the resolving list with entries from IRK list.*/
+ hci_load_resolving_list(hdev);
hci_dev_set_flag(hdev, HCI_RPA_RESOLVING);

err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_LOAD_IRKS, 0, NULL, 0);
@@ -7213,6 +7217,10 @@ void mgmt_power_on(struct hci_dev *hdev, int err)
if (match.sk)
sock_put(match.sk);

+ /* If load _irk was called when controller was powered down,
+ * then the resolving list has to be updated now.
+ */
+ hci_load_resolving_list(hdev);
hci_dev_unlock(hdev);
}

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 1476a91ce935..85be3f5d869a 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -2425,6 +2425,8 @@ int smp_cancel_and_remove_pairing(struct hci_dev *hdev, bdaddr_t *bdaddr,
struct smp_chan *smp;
int err;

+ hci_req_del_from_resolving_list(hdev, addr_type, bdaddr);
+
err = hci_remove_ltk(hdev, bdaddr, addr_type);
hci_remove_irk(hdev, bdaddr, addr_type);

@@ -2619,6 +2621,15 @@ static int smp_cmd_ident_addr_info(struct l2cap_conn *conn,
smp->remote_irk = hci_add_irk(conn->hcon->hdev, &smp->id_addr,
smp->id_addr_type, smp->irk, &rpa);

+ /* Add the device to the resolving list.*/
+ hci_req_update_resolving_list(conn->hcon->hdev,
+ smp->remote_irk->addr_type,
+ &smp->remote_irk->bdaddr,
+ smp->remote_irk->val);
+
+
+ /*TODO: Check remote device Supports LL Privacy. If not set the device
+ * with IRK to Device-privacy Mode - Should be discussed - Based on */
distribute:
if (!(smp->remote_key_dist & KEY_DIST_MASK))
smp_distribute_keys(smp);
--
2.17.1

2020-03-12 10:09:47

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 7/7] Bluetooth LL Privacy update White list

Update white list filter policy of if LL Privacy is enabled
When Address Resolution is enabled and device is already in the white
list then follow the filter policy

Signed-off-by: Sathish Narsimman <[email protected]>
---
net/bluetooth/hci_request.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 12868caf50ea..47cb2602713a 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -711,7 +711,16 @@ static int add_to_white_list(struct hci_request *req,
/* White list can not be used with RPAs */
if (!allow_rpa &&
hci_find_irk_by_addr(hdev, &params->addr, params->addr_type)) {
- return -1;
+ /* White list can not be used with RPAs if address
+ * resolution is disabled in the controller
+ */
+ if (!hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION))
+ return -1;
+
+ /* if the peer device is not present in the resolving list */
+ if (!hci_bdaddr_list_lookup_with_irk(&hdev->le_resolv_list, &b->bdaddr,
+ b->bdaddr_type))
+ return -1;
}

/* During suspend, only wakeable devices can be in whitelist */
@@ -768,7 +777,16 @@ static u8 update_white_list(struct hci_request *req)
/* White list can not be used with RPAs */
if (!allow_rpa &&
hci_find_irk_by_addr(hdev, &b->bdaddr, b->bdaddr_type)) {
- return 0x00;
+ /* White list can not be used with RPAs if address
+ * resolution is disabled in the controller
+ */
+ if (!hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION))
+ return 0x00;
+
+ /* if the peer device is not present in the resolving list */
+ if (!hci_bdaddr_list_lookup_with_irk(&hdev->le_resolv_list, &b->bdaddr,
+ b->bdaddr_type))
+ return 0x00;
}

num_entries++;
--
2.17.1

2020-03-12 10:09:47

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 5/7] Bluetooth: LL Privacy Check and Update the Resolving List

This patch is an helper function which will be usefull to check
the Local Resolving list and update the controller Resolving list
during le_create_conn , le_directed_adv.

Signed-off-by: Sathish Narsimman <[email protected]>
Signed-off-by: Joy Shermin <[email protected]>
---
net/bluetooth/hci_request.c | 21 +++++++++++++++++++++
net/bluetooth/hci_request.h | 2 ++
2 files changed, 23 insertions(+)

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index e17db3103a3d..6b685cdc7de5 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1015,6 +1015,27 @@ void hci_req_update_resolving_list_local_irk(struct hci_dev *hdev)
hci_req_run(&req, NULL);
}

+void hci_req_check_and_update_resolving_list(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr)
+{
+ struct smp_irk *irk;
+
+ BT_DBG("");
+
+ /* Nothing to be done if LL privacy is not supported */
+ if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY) )
+ return;
+
+ if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
+ ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
+ hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
+ hci_lookup_le_connect(hdev) ) )
+ return;
+
+ irk = hci_find_irk_by_addr(hdev, bdaddr, type);
+ if (irk)
+ hci_req_update_resolving_list(hdev, irk->addr_type, &irk->bdaddr, irk->val);
+}
+
void hci_load_resolving_list (struct hci_dev *hdev)
{
struct smp_irk *irk;
diff --git a/net/bluetooth/hci_request.h b/net/bluetooth/hci_request.h
index 0e81614d235e..92a71f35543b 100644
--- a/net/bluetooth/hci_request.h
+++ b/net/bluetooth/hci_request.h
@@ -90,6 +90,8 @@ void __hci_req_clear_ext_adv_sets(struct hci_request *req);
int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,
bool use_rpa, struct adv_info *adv_instance,
u8 *own_addr_type, bdaddr_t *rand_addr);
+void hci_req_check_and_update_resolving_list(struct hci_dev *hdev, u8 addr_type,
+ bdaddr_t *bdaddr);

void __hci_req_update_class(struct hci_request *req);

--
2.17.1

2020-03-12 10:09:47

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 3/7] Bluetooth: Update the Resolving list when Local IRK changed

Whenever the Local IRK changes. i.e privacy mode is toggled
this patch helps to change the IRK list in the bluetooth
controller

Signed-off-by: Sathish Narsimman <[email protected]>
Signed-off-by: Joy Shermin <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_request.c | 50 ++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e5e09d530ce7..43d31a9339a6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1659,6 +1659,8 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
u8 *bdaddr_type);
int hci_req_update_resolving_list(struct hci_dev *hdev, u8 addr_type,
bdaddr_t *bdaddr, u8 irk[16]);
+void hci_req_update_resolving_list_local_irk(struct hci_dev *hdev);
+
void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
bdaddr_t *bdaddr);

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 88225a9ca1f8..9ffb62178d24 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -965,6 +965,56 @@ int hci_req_update_resolving_list(struct hci_dev *hdev, u8 type, bdaddr_t *bdadd
return 0;
}

+void hci_req_update_resolving_list_local_irk(struct hci_dev *hdev)
+{
+ struct bdaddr_list_with_irk *irk;
+ struct hci_request req;
+
+ BT_DBG("");
+
+ /* Nothing to be done if LL privacy is not supported. */
+ if (!(hdev->le_features[0] & HCI_LE_LL_PRIVACY))
+ return;
+
+ /* If resolving list is empty, nothing is to be done.*/
+ if (list_empty(&hdev->le_resolv_list))
+ return;
+
+ /* Resolving List cannot be updated if address resolution
+ * in the controller is enabled and advertisement or scanning
+ * or create connection command is ongoing.
+ */
+ if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
+ ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
+ hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
+ hci_lookup_le_connect(hdev) ) )
+ return;
+
+ /* If resolving list is not empty, then clear the resolving list
+ * and add back each entry with updated local IRK.
+ */
+ hci_req_init(&req, hdev);
+
+ list_for_each_entry(irk, &hdev->le_resolv_list, list) {
+ struct hci_cp_le_add_to_resolv_list cp;
+ struct hci_cp_le_del_from_resolv_list cp1;
+
+ cp1.bdaddr_type = irk->bdaddr_type;
+ bacpy(&cp1.bdaddr, &irk->bdaddr);
+ hci_req_add(&req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
+ sizeof(cp1), &cp1);
+
+ cp.bdaddr_type = irk->bdaddr_type;
+ bacpy(&cp.bdaddr, &irk->bdaddr);
+ memcpy(cp.peer_irk, irk->peer_irk, 16);
+ memcpy(cp.local_irk, hdev->irk, 16);
+ hci_req_add(&req, HCI_OP_LE_ADD_TO_RESOLV_LIST,
+ sizeof(cp), &cp);
+ }
+
+ hci_req_run(&req, NULL);
+}
+
void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type, bdaddr_t *bdaddr)
{
struct hci_cp_le_del_from_resolv_list cp;
--
2.17.1

2020-03-12 10:09:47

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 1/7] Bluetooth: LL Privacy Delete Store Resolving list

This patch help to delte the resolving list stored in the BT
Controller w.r.t BD_ADDR.

Signed-off-by: Sathish Narsimman <[email protected]>
---
include/net/bluetooth/hci.h | 1 +
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_request.c | 33 ++++++++++++++++++++++++++++++++
3 files changed, 36 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5f60e135aeb6..352bc43940ff 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -457,6 +457,7 @@ enum {
#define HCI_LE_DATA_LEN_EXT 0x20
#define HCI_LE_PHY_2M 0x01
#define HCI_LE_PHY_CODED 0x08
+#define HCI_LE_LL_PRIVACY 0x40
#define HCI_LE_EXT_ADV 0x10
#define HCI_LE_EXT_SCAN_POLICY 0x80
#define HCI_LE_PHY_2M 0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index d4e28773d378..5f04ef88da35 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1657,6 +1657,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __le64 rand,

void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
u8 *bdaddr_type);
+void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
+ bdaddr_t *bdaddr);

#define SCO_AIRMODE_MASK 0x0003
#define SCO_AIRMODE_CVSD 0x0000
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index bf83179ab9d1..f4bbd3b79210 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -885,6 +885,39 @@ static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
}
}

+void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type, bdaddr_t *bdaddr)
+{
+ struct hci_cp_le_del_from_resolv_list cp;
+ struct hci_request req;
+ struct bdaddr_list_with_irk *irk;
+
+ BT_DBG("");
+
+ /* Nothing to be done if LL privacy is not supported */
+ if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY) )
+ return;
+
+ if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
+ ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
+ hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
+ hci_lookup_le_connect(hdev) ) )
+ return;
+
+ /* If the device is not present in resolving list
+ * no need to send HCI command.
+ */
+ irk = hci_bdaddr_list_lookup_with_irk(&hdev->le_resolv_list, bdaddr, addr_type);
+ if (!irk)
+ return;
+
+ hci_req_init(&req, hdev);
+ cp.bdaddr_type = addr_type;
+ bacpy(&cp.bdaddr, bdaddr);
+
+ hci_req_add(&req, HCI_OP_LE_DEL_FROM_RESOLV_LIST, sizeof(cp), &cp);
+ hci_req_run(&req, NULL);
+}
+
void hci_req_add_le_passive_scan(struct hci_request *req)
{
struct hci_dev *hdev = req->hdev;
--
2.17.1

2020-03-18 08:57:15

by Sathish Narasimman

[permalink] [raw]
Subject: Re: [PATCH 0/7] LE LL Priavcy support enabled

Hi

A gentle reminder to review the patches

On Thu, Mar 12, 2020 at 3:36 PM Sathish Narsimman <[email protected]> wrote:
>
> With this LE LL Priavcy feature available if BT controller supports
> LL Privacy when privacy is enabled.
>
> The patches are verified in below case.
> Pairing - where the IRK is added to device Resolving List after
> identity info & IRK is shared in SMP.
> Unpair - Delete the resolving entry from the Device resolving list
> Local IRK change - Whenever there is change in local IRK (privacy) the
> resolving list entries in the device is updated.
> Whitelist - filter policies for whitelist is updated based on LL_adress
> resolution support.
> Reload Resolving list - device reset/sysmte reboot the Resolving list
> reloaded in to the device.
>
> Sathish Narsimman (7):
> Bluetooth: LL Privacy Delete Store Resolving list
> Bluetooth: Add device to Resolving List
> Bluetooth: Update the Resolving list when Local IRK changed
> Bluetooth: LL_PRIVACY re-load resolving list
> Bluetooth: LL Privacy Check and Update the Resolving List
> Bluetooth: Enable LL Privacy
> Bluetooth LL Privacy update White list
>
> include/net/bluetooth/hci.h | 3 +
> include/net/bluetooth/hci_core.h | 7 +
> net/bluetooth/hci_conn.c | 32 ++++
> net/bluetooth/hci_core.c | 20 ++-
> net/bluetooth/hci_event.c | 32 +++-
> net/bluetooth/hci_request.c | 251 ++++++++++++++++++++++++++++++-
> net/bluetooth/hci_request.h | 2 +
> net/bluetooth/mgmt.c | 10 +-
> net/bluetooth/smp.c | 11 ++
> 9 files changed, 354 insertions(+), 14 deletions(-)
>
> --
> 2.17.1
>

2020-03-18 09:30:40

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 2/7] Bluetooth: Add device to Resolving List

Hi,

On Thursday, 12 March 2020 11:07:49 CET Sathish Narsimman wrote:
> The Patch is used to add the device to resolving list. The patch
> will look for the local IRK list and if the device is present in
> the local resolving list it will be updated and the same updated
> w.r.t to BT controller by first deleting the existing and adding
> the new IRK w.r.t bd_addr.
>
> Signed-off-by: Sathish Narsimman <[email protected]>
> Signed-off-by: Joy Shermin <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 +
> net/bluetooth/hci_request.c | 80 ++++++++++++++++++++++++++++++++
> 2 files changed, 82 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h
> b/include/net/bluetooth/hci_core.h index 5f04ef88da35..e5e09d530ce7 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1657,6 +1657,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16
> ediv, __le64 rand,
>
> void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> u8 *bdaddr_type);
> +int hci_req_update_resolving_list(struct hci_dev *hdev, u8 addr_type,
> + bdaddr_t *bdaddr, u8 irk[16]);
> void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
> bdaddr_t *bdaddr);
>
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index f4bbd3b79210..88225a9ca1f8 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -885,6 +885,86 @@ static void hci_req_start_scan(struct hci_request *req,
> u8 type, u16 interval, }
> }
>
> +int hci_req_update_resolving_list(struct hci_dev *hdev, u8 type, bdaddr_t
> *bdaddr, + u8 irk_val[16])
> +{
> + struct hci_request req;
> + struct hci_cp_le_add_to_resolv_list cp;
> + struct bdaddr_list_with_irk *irk;
> + u8 entries;
> +
> + BT_DBG("");
> +
> + /* Nothing to be done if LL privacy is not supported */
> + if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY) )
> + return -EPROTONOSUPPORT;
> +
> + /* Resolving List cannot be updated if address resolution
> + * in the controller is enabled and advertisement or scanning
> + * or create connection command is ongoing.
> + */
> + if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
> + ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
> + hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
> + hci_lookup_le_connect(hdev) ) )
> + return -EINVAL;
> +
> + hci_req_init(&req, hdev);
> +
> + irk = hci_bdaddr_list_lookup_with_irk(&hdev->le_resolv_list, bdaddr,
> type); + if (irk) {
> + /* Device is present in resolving list.*/
> + if (memcmp(irk->peer_irk, irk_val, 16) == 0) {
> + /* Device present in resolving list with same
IRK.
> + * No HCI communication is required.Sort the
> + * Kernel list.
> + */
> + hci_bdaddr_list_add_with_irk(&irk->list, bdaddr,
type, irk_val, NULL);
> + return 0;
> +
> + }
> + /* IRK has changed for the device in resolving list
> + * Queue up commands to delete the existing entry and
> + * add new one. Sorting will be done when command complete
> + * for add command is receieved.
> + */
> + goto remote;
> + }
> + /* Device is not present in resolving list.If resolving list
> + * is not full add the device to resolving list.
> + */
> + entries = 0;
> + list_for_each_entry(irk, &hdev->le_resolv_list, list) {
> + entries++;
> + }
> +
> + if (entries < hdev->le_resolv_list_size)
> + goto add;
> +
> + /* If the resolving list is full, queue up HCI command to delete
> + * the entry that was used least recently to make space for the
> + * new device. Kernel list will be updated when command complete
> + * is received.
> + */
> + irk = list_first_entry_or_null(&hdev->le_resolv_list, struct
> bdaddr_list_with_irk, list); +
> + if (!irk)
> + return -1;

I'm not sure about this... we should have some fallback mechanism when HW RL
is full. Maybe some rotation of devies on list or simply disable address
resolution in controller? Otherwise we won't be able to connect to some of
paired devices.

> +
> +remote:
> + hci_req_del_from_resolving_list(hdev, irk->bdaddr_type, &irk-
>bdaddr);
> +
> +add:
> + cp.bdaddr_type = type;
> + bacpy(&cp.bdaddr, bdaddr);
> + memcpy(cp.peer_irk, irk_val, 16);
> + memcpy(cp.local_irk, hdev->irk, 16);
> +
> + hci_req_add(&req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
> + hci_req_run(&req, NULL);
> + return 0;
> +}
> +
> void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
> bdaddr_t *bdaddr) {
> struct hci_cp_le_del_from_resolv_list cp;


--
pozdrawiam
Szymon Janc


2020-03-18 10:30:12

by Sathish Narasimman

[permalink] [raw]
Subject: Re: [PATCH 2/7] Bluetooth: Add device to Resolving List

Hi Szymon,

On Wed, Mar 18, 2020 at 3:00 PM Szymon Janc <[email protected]> wrote:
>
> Hi,
>
> On Thursday, 12 March 2020 11:07:49 CET Sathish Narsimman wrote:
> > The Patch is used to add the device to resolving list. The patch
> > will look for the local IRK list and if the device is present in
> > the local resolving list it will be updated and the same updated
> > w.r.t to BT controller by first deleting the existing and adding
> > the new IRK w.r.t bd_addr.
> >
> > Signed-off-by: Sathish Narsimman <[email protected]>
> > Signed-off-by: Joy Shermin <[email protected]>
> > ---
> > include/net/bluetooth/hci_core.h | 2 +
> > net/bluetooth/hci_request.c | 80 ++++++++++++++++++++++++++++++++
> > 2 files changed, 82 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci_core.h
> > b/include/net/bluetooth/hci_core.h index 5f04ef88da35..e5e09d530ce7 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -1657,6 +1657,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16
> > ediv, __le64 rand,
> >
> > void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> > u8 *bdaddr_type);
> > +int hci_req_update_resolving_list(struct hci_dev *hdev, u8 addr_type,
> > + bdaddr_t *bdaddr, u8 irk[16]);
> > void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
> > bdaddr_t *bdaddr);
> >
> > diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> > index f4bbd3b79210..88225a9ca1f8 100644
> > --- a/net/bluetooth/hci_request.c
> > +++ b/net/bluetooth/hci_request.c
> > @@ -885,6 +885,86 @@ static void hci_req_start_scan(struct hci_request *req,
> > u8 type, u16 interval, }
> > }
> >
> > +int hci_req_update_resolving_list(struct hci_dev *hdev, u8 type, bdaddr_t
> > *bdaddr, + u8 irk_val[16])
> > +{
> > + struct hci_request req;
> > + struct hci_cp_le_add_to_resolv_list cp;
> > + struct bdaddr_list_with_irk *irk;
> > + u8 entries;
> > +
> > + BT_DBG("");
> > +
> > + /* Nothing to be done if LL privacy is not supported */
> > + if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY) )
> > + return -EPROTONOSUPPORT;
> > +
> > + /* Resolving List cannot be updated if address resolution
> > + * in the controller is enabled and advertisement or scanning
> > + * or create connection command is ongoing.
> > + */
> > + if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
> > + ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
> > + hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
> > + hci_lookup_le_connect(hdev) ) )
> > + return -EINVAL;
> > +
> > + hci_req_init(&req, hdev);
> > +
> > + irk = hci_bdaddr_list_lookup_with_irk(&hdev->le_resolv_list, bdaddr,
> > type); + if (irk) {
> > + /* Device is present in resolving list.*/
> > + if (memcmp(irk->peer_irk, irk_val, 16) == 0) {
> > + /* Device present in resolving list with same
> IRK.
> > + * No HCI communication is required.Sort the
> > + * Kernel list.
> > + */
> > + hci_bdaddr_list_add_with_irk(&irk->list, bdaddr,
> type, irk_val, NULL);
> > + return 0;
> > +
> > + }
> > + /* IRK has changed for the device in resolving list
> > + * Queue up commands to delete the existing entry and
> > + * add new one. Sorting will be done when command complete
> > + * for add command is receieved.
> > + */
> > + goto remote;
> > + }
> > + /* Device is not present in resolving list.If resolving list
> > + * is not full add the device to resolving list.
> > + */
> > + entries = 0;
> > + list_for_each_entry(irk, &hdev->le_resolv_list, list) {
> > + entries++;
> > + }
> > +
> > + if (entries < hdev->le_resolv_list_size)
> > + goto add;
> > +
> > + /* If the resolving list is full, queue up HCI command to delete
> > + * the entry that was used least recently to make space for the
> > + * new device. Kernel list will be updated when command complete
> > + * is received.
> > + */
> > + irk = list_first_entry_or_null(&hdev->le_resolv_list, struct
> > bdaddr_list_with_irk, list); +
> > + if (!irk)
> > + return -1;
>
> I'm not sure about this... we should have some fallback mechanism when HW RL
> is full. Maybe some rotation of devies on list or simply disable address
> resolution in controller? Otherwise we won't be able to connect to some of
> paired devices.
What I understood was
if (HW RL is full) {
don't delete the device from the kernel resolving list.
delete the device from HW RL list(least used)
but we have to use a method to denote that it is deleted from HW RL list.
}
from the above method at what scenario we need to add the deleted
device back to the HW RL list?

And yes disabling HW RL list much simpler.
>
> > +
> > +remote:
> > + hci_req_del_from_resolving_list(hdev, irk->bdaddr_type, &irk-
> >bdaddr);
> > +
> > +add:
> > + cp.bdaddr_type = type;
> > + bacpy(&cp.bdaddr, bdaddr);
> > + memcpy(cp.peer_irk, irk_val, 16);
> > + memcpy(cp.local_irk, hdev->irk, 16);
> > +
> > + hci_req_add(&req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
> > + hci_req_run(&req, NULL);
> > + return 0;
> > +}
> > +
> > void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
> > bdaddr_t *bdaddr) {
> > struct hci_cp_le_del_from_resolv_list cp;
>
>
> --
> pozdrawiam
> Szymon Janc
>
>
Regards
Sathish N

2020-03-18 10:43:10

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/7] Bluetooth: LL Privacy Delete Store Resolving list

Hi Sathish,

> This patch help to delte the resolving list stored in the BT
> Controller w.r.t BD_ADDR.
>
> Signed-off-by: Sathish Narsimman <[email protected]>
> ---
> include/net/bluetooth/hci.h | 1 +
> include/net/bluetooth/hci_core.h | 2 ++
> net/bluetooth/hci_request.c | 33 ++++++++++++++++++++++++++++++++
> 3 files changed, 36 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 5f60e135aeb6..352bc43940ff 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -457,6 +457,7 @@ enum {
> #define HCI_LE_DATA_LEN_EXT 0x20
> #define HCI_LE_PHY_2M 0x01
> #define HCI_LE_PHY_CODED 0x08
> +#define HCI_LE_LL_PRIVACY 0x40
> #define HCI_LE_EXT_ADV 0x10
> #define HCI_LE_EXT_SCAN_POLICY 0x80
> #define HCI_LE_PHY_2M 0x01
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index d4e28773d378..5f04ef88da35 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1657,6 +1657,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __le64 rand,
>
> void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> u8 *bdaddr_type);
> +void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
> + bdaddr_t *bdaddr);
>
> #define SCO_AIRMODE_MASK 0x0003
> #define SCO_AIRMODE_CVSD 0x0000
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index bf83179ab9d1..f4bbd3b79210 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -885,6 +885,39 @@ static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
> }
> }
>
> +void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type, bdaddr_t *bdaddr)
> +{
> + struct hci_cp_le_del_from_resolv_list cp;
> + struct hci_request req;
> + struct bdaddr_list_with_irk *irk;
> +
> + BT_DBG("");
> +
> + /* Nothing to be done if LL privacy is not supported */
> + if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY) )
> + return;
> +
> + if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
> + ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
> + hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
> + hci_lookup_le_connect(hdev) ) )
> + return;

please don’t invent your own coding style. Follow the netdev convention.

Regards

Marcel

2020-03-18 10:45:45

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 3/7] Bluetooth: Update the Resolving list when Local IRK changed

Hi Sathish,

> Whenever the Local IRK changes. i.e privacy mode is toggled
> this patch helps to change the IRK list in the bluetooth
> controller
>
> Signed-off-by: Sathish Narsimman <[email protected]>
> Signed-off-by: Joy Shermin <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 ++
> net/bluetooth/hci_request.c | 50 ++++++++++++++++++++++++++++++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e5e09d530ce7..43d31a9339a6 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1659,6 +1659,8 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> u8 *bdaddr_type);
> int hci_req_update_resolving_list(struct hci_dev *hdev, u8 addr_type,
> bdaddr_t *bdaddr, u8 irk[16]);
> +void hci_req_update_resolving_list_local_irk(struct hci_dev *hdev);
> +
> void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
> bdaddr_t *bdaddr);
>
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 88225a9ca1f8..9ffb62178d24 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -965,6 +965,56 @@ int hci_req_update_resolving_list(struct hci_dev *hdev, u8 type, bdaddr_t *bdadd
> return 0;
> }
>
> +void hci_req_update_resolving_list_local_irk(struct hci_dev *hdev)
> +{
> + struct bdaddr_list_with_irk *irk;
> + struct hci_request req;
> +
> + BT_DBG("");
> +
> + /* Nothing to be done if LL privacy is not supported. */
> + if (!(hdev->le_features[0] & HCI_LE_LL_PRIVACY))
> + return;
> +
> + /* If resolving list is empty, nothing is to be done.*/
> + if (list_empty(&hdev->le_resolv_list))
> + return;

actually the le_resolv_list represents the current list programmed into the controller.

> +
> + /* Resolving List cannot be updated if address resolution
> + * in the controller is enabled and advertisement or scanning
> + * or create connection command is ongoing.
> + */
> + if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
> + ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
> + hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
> + hci_lookup_le_connect(hdev) ) )
> + return;

Coding style needs to be correct.

> +
> + /* If resolving list is not empty, then clear the resolving list
> + * and add back each entry with updated local IRK.
> + */
> + hci_req_init(&req, hdev);
> +
> + list_for_each_entry(irk, &hdev->le_resolv_list, list) {
> + struct hci_cp_le_add_to_resolv_list cp;
> + struct hci_cp_le_del_from_resolv_list cp1;
> +
> + cp1.bdaddr_type = irk->bdaddr_type;
> + bacpy(&cp1.bdaddr, &irk->bdaddr);
> + hci_req_add(&req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
> + sizeof(cp1), &cp1);
> +
> + cp.bdaddr_type = irk->bdaddr_type;
> + bacpy(&cp.bdaddr, &irk->bdaddr);
> + memcpy(cp.peer_irk, irk->peer_irk, 16);
> + memcpy(cp.local_irk, hdev->irk, 16);
> + hci_req_add(&req, HCI_OP_LE_ADD_TO_RESOLV_LIST,
> + sizeof(cp), &cp);
> + }

Can we please do this the smart way, no need to remove and entry to just add it back later.

Regards

Marcel

2020-03-18 10:47:46

by Sathish Narasimman

[permalink] [raw]
Subject: Re: [PATCH 1/7] Bluetooth: LL Privacy Delete Store Resolving list

Hi Marcel

On Wed, Mar 18, 2020 at 4:12 PM Marcel Holtmann <[email protected]> wrote:
>
> Hi Sathish,
>
> > This patch help to delte the resolving list stored in the BT
> > Controller w.r.t BD_ADDR.
> >
> > Signed-off-by: Sathish Narsimman <[email protected]>
> > ---
> > include/net/bluetooth/hci.h | 1 +
> > include/net/bluetooth/hci_core.h | 2 ++
> > net/bluetooth/hci_request.c | 33 ++++++++++++++++++++++++++++++++
> > 3 files changed, 36 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 5f60e135aeb6..352bc43940ff 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -457,6 +457,7 @@ enum {
> > #define HCI_LE_DATA_LEN_EXT 0x20
> > #define HCI_LE_PHY_2M 0x01
> > #define HCI_LE_PHY_CODED 0x08
> > +#define HCI_LE_LL_PRIVACY 0x40
> > #define HCI_LE_EXT_ADV 0x10
> > #define HCI_LE_EXT_SCAN_POLICY 0x80
> > #define HCI_LE_PHY_2M 0x01
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index d4e28773d378..5f04ef88da35 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -1657,6 +1657,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __le64 rand,
> >
> > void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> > u8 *bdaddr_type);
> > +void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
> > + bdaddr_t *bdaddr);
> >
> > #define SCO_AIRMODE_MASK 0x0003
> > #define SCO_AIRMODE_CVSD 0x0000
> > diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> > index bf83179ab9d1..f4bbd3b79210 100644
> > --- a/net/bluetooth/hci_request.c
> > +++ b/net/bluetooth/hci_request.c
> > @@ -885,6 +885,39 @@ static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
> > }
> > }
> >
> > +void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type, bdaddr_t *bdaddr)
> > +{
> > + struct hci_cp_le_del_from_resolv_list cp;
> > + struct hci_request req;
> > + struct bdaddr_list_with_irk *irk;
> > +
> > + BT_DBG("");
> > +
> > + /* Nothing to be done if LL privacy is not supported */
> > + if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY) )
> > + return;
> > +
> > + if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
> > + ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
> > + hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
> > + hci_lookup_le_connect(hdev) ) )
> > + return;
>
> please don’t invent your own coding style. Follow the netdev convention.

Sure will correct the coding style in next version of the patch
>
> Regards
>
> Marcel
>

2020-03-18 11:04:29

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 6/7] Bluetooth: Enable LL Privacy

Hi Sathish,

> This enables Link Layer Privacy and sets RPA timeout to 900sec
>
> < HCI Command: LE Set Address Resolution Enable (0x08|0x002d) plen 1
> Address resolution: Enabled (0x01)
>> HCI Event: Command Complete (0x0e) plen 4
> LE Set Address Resolution Enable (0x08|0x002d) ncmd 1d
> Status: Success (0x00)
> < HCI Command: LE Set Resolvable Private Address Timeout (0x08|0x002e) plen 2
> Timeout: 900 seconds
>> HCI Event: Command Complete (0x0e) plen 4
> LE Set Resolvable Private Address Timeout (0x08|0x002e) ncmd 1
> Status: Success (0x00)
>
> During Successfull pairing adding the information to device Resolving
> list
>
> < HCI Command: LE Add Device To Resolving List (0x08|0x0027) plen 39
> Address type: Random (0x01)
> Address: D1:79:92:B1:83:DF (Static)
> Peer identity resolving key: ecc317056e5b385cda281b9c24a453e9
> Local identity resolving key: 00000000000000000000000000000000
>
> < ACL Data TX: Handle 3585 flags 0x00 dlen 21
> SMP: Signing Information (0x0a) len 16
> Signature key: 37b6c45656c7e8b3ec8db062c3e0c2b2
>> ACL Data RX: Handle 3585 flags 0x02 dlen 9
> ATT: Error Response (0x01) len 4
> Read By Group Type Request (0x10)
> Handle: 0x0001
> Error: Attribute Not Found (0x0a)
>> ACL Data RX: Handle 3585 flags 0x02 dlen 11
> ATT: Read By Group Type Request (0x10) len 6
> Handle range: 0x000a-0xffff
> Attribute group type: Primary Service (0x2800)
>
>> HCI Event: Command Complete (0x0e) plen 4
> LE Add Device To Resolving List (0x08|0x0027) ncmd 1
> Status: Success (0x00)
>
> Signed-off-by: Sathish Narsimman <[email protected]>
> ---
> include/net/bluetooth/hci.h | 2 ++
> net/bluetooth/hci_conn.c | 32 ++++++++++++++++++++++++++++++++
> net/bluetooth/hci_core.c | 20 +++++++++++++++++++-
> net/bluetooth/hci_event.c | 32 +++++++++++++++++++++++++-------
> net/bluetooth/hci_request.c | 16 +++++++++++++---
> net/bluetooth/mgmt.c | 10 +++++++++-
> net/bluetooth/smp.c | 11 +++++++++++
> 7 files changed, 111 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 352bc43940ff..d8431ebb8baa 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1617,6 +1617,8 @@ struct hci_rp_le_read_resolv_list_size {
>
> #define HCI_OP_LE_SET_ADDR_RESOLV_ENABLE 0x202d
>
> +#define HCI_OP_LE_SET_RPA_TIMEOUT 0x202e
> +
> #define HCI_OP_LE_READ_MAX_DATA_LEN 0x202f
> struct hci_rp_le_read_max_data_len {
> __u8 status;
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index e245bc155cc2..06067c0852a6 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -830,6 +830,14 @@ static void hci_req_add_le_create_conn(struct hci_request *req,
>
> plen = sizeof(*cp);
>
> + /* If the own_addr_type is 0x03 or 0x02 and peer is present
> + * in Kernel IRK list, but not in resolving add the peer to
> + * the resolving list.
> + */
> + if (own_addr_type == 0x03 || own_addr_type == 0x02)
> + hci_req_check_and_update_resolving_list(hdev, conn->dst_type,
> + &conn->dst);
> +

I am failing to understand this part. The address should have been in the resolving list already. Either via Load Identify Resolving Keys mgmt command or from the SMP pairing. Doing that at time when calling Create Connection seems wrong. It should be independent from that.

> if (scan_1m(hdev)) {
> cp->phys |= LE_SCAN_PHY_1M;
> set_ext_conn_params(conn, p);
> @@ -866,6 +874,14 @@ static void hci_req_add_le_create_conn(struct hci_request *req,
> cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
> cp.scan_window = cp.scan_interval;
>
> + /* If the own_addr_type is 0x03 or 0x02 and peer is present
> + * in Kernel IRK list, but not in resolving add the peer to
> + * the resolving list.
> + */
> + if (own_addr_type == 0x03 || own_addr_type == 0x02)
> + hci_req_check_and_update_resolving_list(hdev, conn->dst_type,
> + &conn->dst);
> +
> bacpy(&cp.peer_addr, &conn->dst);
> cp.peer_addr_type = conn->dst_type;
> cp.own_address_type = own_addr_type;
> @@ -901,6 +917,14 @@ static void hci_req_directed_advertising(struct hci_request *req,
> &own_addr_type, &random_addr) < 0)
> return;
>
> + /* If the own_addr_type is 0x03 or 0x02 and peer is present
> + * in Kernel IRK list, but not in resolving add the peer to
> + * the resolving list.
> + */
> + if (own_addr_type == 0x03 || own_addr_type == 0x03)
> + hci_req_check_and_update_resolving_list(hdev, conn->dst_type,
> + &conn->dst);
> +
> memset(&cp, 0, sizeof(cp));
>
> cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_DIRECT_IND);
> @@ -959,6 +983,14 @@ static void hci_req_directed_advertising(struct hci_request *req,
> &own_addr_type) < 0)
> return;
>
> + /* If the own_addr_type is 0x03 or 0x02 and peer is present
> + * in Kernel IRK list, but not in resolving add the peer to
> + * the resolving list.
> + */
> + if (own_addr_type == 0x03 || own_addr_type == 0x02)
> + hci_req_check_and_update_resolving_list(hdev, conn->dst_type,
> + &conn->dst);
> +

We just keep repeating this code. It doesn’t look right.

> memset(&cp, 0, sizeof(cp));
>
> /* Some controllers might reject command if intervals are not
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index dbd2ad3a26ed..7727be922084 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -713,7 +713,7 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> /* If the controller supports the LE Extended Create Connection
> * command, enable the corresponding event.
> */
> - if (use_ext_conn(hdev))
> + if (use_ext_conn(hdev) || hdev->le_features[0] & HCI_LE_LL_PRIVACY)
> events[1] |= 0x02; /* LE Enhanced Connection
> * Complete
> */

I do not get this change.

> @@ -762,6 +762,24 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> hci_req_add(req, HCI_OP_LE_CLEAR_RESOLV_LIST, 0, NULL);
> }
>
> + if (hdev->commands[35] & 0x02) {
> + u8 addr_resoln_enable = 0x01;

I don’t know what resoln is suppose to stand for.

> +
> + /* Enable Address Resolution in controller */
> + hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE,
> + sizeof(addr_resoln_enable),
> + &addr_resoln_enable);
> + }
> +
> + if (hdev->commands[35] & 0x40) {
> + __le16 rpa_timeout = cpu_to_le16(hdev->rpa_timeout);
> +
> + /* Set RPA timeout */
> + hci_req_add(req, HCI_OP_LE_SET_RPA_TIMEOUT, 2,
> + &rpa_timeout);
> +
> + }
> +

I think you should be adding RPA timeout handling in a separate patch. It is also important to note that there is a debugfs setting that should be capable of changing this value at runtime.

> if (hdev->le_features[0] & HCI_LE_DATA_LEN_EXT) {
> /* Read LE Maximum Data Length */
> hci_req_add(req, HCI_OP_LE_READ_MAX_DATA_LEN, 0, NULL);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 20408d386268..d5ce1921fadd 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1684,7 +1684,11 @@ static void hci_cc_set_adv_param(struct hci_dev *hdev, struct sk_buff *skb)
> return;
>
> hci_dev_lock(hdev);
> - hdev->adv_addr_type = cp->own_address_type;
> +
> + if (cp->own_address_type == ADDR_LE_DEV_PUBLIC)
> + hdev->adv_addr_type = ADDR_LE_DEV_PUBLIC;
> + else
> + hdev->adv_addr_type = ADDR_LE_DEV_RANDOM;

I don’t get this change.

> hci_dev_unlock(hdev);
> }
>
> @@ -1704,7 +1708,12 @@ static void hci_cc_set_ext_adv_param(struct hci_dev *hdev, struct sk_buff *skb)
> return;
>
> hci_dev_lock(hdev);
> - hdev->adv_addr_type = cp->own_addr_type;
> +
> + if (cp->own_addr_type == ADDR_LE_DEV_PUBLIC)
> + hdev->adv_addr_type = ADDR_LE_DEV_PUBLIC;
> + else
> + hdev->adv_addr_type = ADDR_LE_DEV_RANDOM;
> +

Same as above. I fail to see why this is important.

> if (!hdev->cur_adv_instance) {
> /* Store in hdev for instance 0 */
> hdev->adv_tx_power = rp->tx_power;
> @@ -2267,11 +2276,13 @@ static void cs_le_create_conn(struct hci_dev *hdev, bdaddr_t *peer_addr,
> * is needed for SMP. These values will not change during the
> * lifetime of the connection.
> */
> - conn->init_addr_type = own_address_type;
> - if (own_address_type == ADDR_LE_DEV_RANDOM)
> + if (own_address_type == 0x03 || own_address_type == 0x01) {
> + conn->init_addr_type = ADDR_LE_DEV_RANDOM;
> bacpy(&conn->init_addr, &hdev->random_addr);
> - else
> + } else {
> + conn->init_addr_type = ADDR_LE_DEV_PUBLIC;
> bacpy(&conn->init_addr, &hdev->bdaddr);
> + }

This requires an appropriate comment change as well.

>
> conn->resp_addr_type = peer_addr_type;
> bacpy(&conn->resp_addr, peer_addr);
> @@ -5141,6 +5152,8 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
> }
> }
>
> + hci_req_check_and_update_resolving_list(hdev, (conn->dst_type), &conn->dst);
> +
> unlock:
> hci_update_background_scan(hdev);
> hci_dev_unlock(hdev);
> @@ -5365,8 +5378,13 @@ static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
> /* Only resolvable random addresses are valid for these
> * kind of reports and others can be ignored.
> */
> - if (!hci_bdaddr_is_rpa(direct_addr, direct_addr_type))
> - return;
> + if (!hci_bdaddr_is_rpa(direct_addr, direct_addr_type)) {
> + /* Controller with LL Privacy Supported tend to resolve
> + * the RPA. In that case we should ignore this condition
> + */
> + if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY))
> + return;

Please obey the coding style.

And just checking the existence of feature bit is not enough. We also need to check that the feature is active and actually used in this case.

> + }
>
> /* If the controller is not using resolvable random
> * addresses, then this report can be ignored.
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 6b685cdc7de5..12868caf50ea 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -1916,7 +1916,12 @@ int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,
> if (use_rpa) {
> int to;
>
> - *own_addr_type = ADDR_LE_DEV_RANDOM;
> + if (hdev->le_features[0] & HCI_LE_LL_PRIVACY) {
> + /*if there is no resolving list used in the controller
> + * use the bd_addr from latest Le_set_random_address*/
> + *own_addr_type = 0x03;
> + } else
> + *own_addr_type = ADDR_LE_DEV_RANDOM;

I really don’t see how this is going to work. Local privacy can off and maybe remote device don’t use privacy. As noted above the le_features bit is not an indicate if LL Privacy is in use.

In addition, please get the coding style right.
>
> if (adv_instance) {
> if (!adv_instance->rpa_expired &&
> @@ -2074,7 +2079,7 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)
>
> hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp), &cp);
>
> - if (own_addr_type == ADDR_LE_DEV_RANDOM &&
> + if (((own_addr_type == ADDR_LE_DEV_RANDOM) || (own_addr_type == 0x03)) &&

The extra (x == y) are not needed and it might be a good idea to introduce constants for 0x02 and 0x03 early on.

> bacmp(&random_addr, BDADDR_ANY)) {
> struct hci_cp_le_set_adv_set_rand_addr cp;
>
> @@ -2341,7 +2346,12 @@ int hci_update_random_address(struct hci_request *req, bool require_privacy,
> if (use_rpa) {
> int to;
>
> - *own_addr_type = ADDR_LE_DEV_RANDOM;
> + if (hdev->le_features[0] & HCI_LE_LL_PRIVACY) {
> + /*if there is no resolving list used in the controller
> + * use the bd_addr from latest Le_set_random_address*/
> + *own_addr_type = 0x03;
> + } else
> + *own_addr_type = ADDR_LE_DEV_RANDOM;

I really need to stop reviewing patches where the coding style mistakes are this many.

>
> if (!hci_dev_test_and_clear_flag(hdev, HCI_RPA_EXPIRED) &&
> !bacmp(&hdev->random_addr, &hdev->rpa))
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 6552003a170e..2b2bc011a130 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -5084,8 +5084,10 @@ static int set_debug_keys(struct sock *sk, struct hci_dev *hdev,
> if (err < 0)
> goto unlock;
>
> - if (changed)
> + if (changed) {
> err = new_settings(hdev, sk);
> + hci_req_update_resolving_list_local_irk(hdev);
> + }
>
> unlock:
> hci_dev_unlock(hdev);
> @@ -5227,6 +5229,8 @@ static int load_irks(struct sock *sk, struct hci_dev *hdev, void *cp_data,
> BDADDR_ANY);
> }
>
> + /* Load the resolving list with entries from IRK list.*/
> + hci_load_resolving_list(hdev);
> hci_dev_set_flag(hdev, HCI_RPA_RESOLVING);
>
> err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_LOAD_IRKS, 0, NULL, 0);
> @@ -7213,6 +7217,10 @@ void mgmt_power_on(struct hci_dev *hdev, int err)
> if (match.sk)
> sock_put(match.sk);
>
> + /* If load _irk was called when controller was powered down,
> + * then the resolving list has to be updated now.
> + */
> + hci_load_resolving_list(hdev);
> hci_dev_unlock(hdev);
> }
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 1476a91ce935..85be3f5d869a 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -2425,6 +2425,8 @@ int smp_cancel_and_remove_pairing(struct hci_dev *hdev, bdaddr_t *bdaddr,
> struct smp_chan *smp;
> int err;
>
> + hci_req_del_from_resolving_list(hdev, addr_type, bdaddr);
> +
> err = hci_remove_ltk(hdev, bdaddr, addr_type);
> hci_remove_irk(hdev, bdaddr, addr_type);
>
> @@ -2619,6 +2621,15 @@ static int smp_cmd_ident_addr_info(struct l2cap_conn *conn,
> smp->remote_irk = hci_add_irk(conn->hcon->hdev, &smp->id_addr,
> smp->id_addr_type, smp->irk, &rpa);
>
> + /* Add the device to the resolving list.*/
> + hci_req_update_resolving_list(conn->hcon->hdev,
> + smp->remote_irk->addr_type,
> + &smp->remote_irk->bdaddr,
> + smp->remote_irk->val);
> +
> +
> + /*TODO: Check remote device Supports LL Privacy. If not set the device
> + * with IRK to Device-privacy Mode - Should be discussed - Based on */

I rather have this fixed instead of keeping a TODO in here.

> distribute:
> if (!(smp->remote_key_dist & KEY_DIST_MASK))
> smp_distribute_keys(smp);

Regards

Marcel

2020-03-18 11:07:07

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 4/7] Bluetooth: LL_PRIVACY re-load resolving list

Hi Sathish,

> The patch is used to load the entries in the local IRK to the
> Bluetooth LE controller resolving list.
>
> Signed-off-by: Sathish Narsimman <[email protected]>
> Signed-off-by: Joy Shermin <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_request.c | 29 +++++++++++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 43d31a9339a6..46d2d3e10c97 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1659,6 +1659,7 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> u8 *bdaddr_type);
> int hci_req_update_resolving_list(struct hci_dev *hdev, u8 addr_type,
> bdaddr_t *bdaddr, u8 irk[16]);
> +void hci_load_resolving_list(struct hci_dev *hdev);
> void hci_req_update_resolving_list_local_irk(struct hci_dev *hdev);
>
> void hci_req_del_from_resolving_list(struct hci_dev *hdev, u8 addr_type,
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 9ffb62178d24..e17db3103a3d 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -1015,6 +1015,35 @@ void hci_req_update_resolving_list_local_irk(struct hci_dev *hdev)
> hci_req_run(&req, NULL);
> }
>
> +void hci_load_resolving_list (struct hci_dev *hdev)
> +{
> + struct smp_irk *irk;
> + u8 num = 0;
> +
> + /* Nothing to be done if LL privacy is not supported */
> + if ( !(hdev->le_features[0] & HCI_LE_LL_PRIVACY) )
> + return;
> +
> + if ( !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) &&
> + ( hci_dev_test_flag(hdev, HCI_LE_ADV) ||
> + hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
> + hci_lookup_le_connect(hdev) ) )
> + return;
> +
> + /* Load the first le_resolving_list_size entries from IRK
> + * list in to resolving list.
> + */
> + rcu_read_lock();
> + list_for_each_entry_rcu(irk, &hdev->identity_resolving_keys, list) {
> + if (num++ >= hdev->le_resolv_list_size)
> + return;
> +
> + hci_req_update_resolving_list(hdev, irk->addr_type, &irk->bdaddr, irk->val);
> + }

I comparison to the le_whitelist, the le_resolv_list_size does not have to match the actual available memory of the controller. It is just some guesstimate. I think we have to design this in a way that we add the IRK anyway and handle the error case gracefully.

I am also confused why we are not actually checking here if an entry is already present and just loading the whole list. I have seen code being duplicated in at least 3 places now.

Regards

Marcel

2020-04-01 20:05:32

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: Re: [PATCH 0/7] LE LL Priavcy support enabled

Hi Satish,

Looking through the patches in this series, it looks like you are
adding the IRK for all connected devices and I'm not sure that's the
best method.
The resolv list seems to be useful in the same way as the le
whitelist: to filter incoming advertisements for devices we care
about.

Thus, to simplify your design, could we not do the same thing as the
le whitelist:
* Update the resolv list anytime passive/background scan is being enabled
* Only keep entries in the resolv list that are part of the
pend_le_conn or pend_le_report lists

Then, you would only need to update the resolv list in
hci_req_add_le_passive_scan and any IRK changes would just disable
passive scan, remove IRKs if existing and re-enable passive scan
(which would add it back with the new one).

Thanks,
Abhishek

On Wed, Mar 18, 2020 at 1:56 AM Sathish Narasimman <[email protected]> wrote:
>
> Hi
>
> A gentle reminder to review the patches
>
> On Thu, Mar 12, 2020 at 3:36 PM Sathish Narsimman <[email protected]> wrote:
> >
> > With this LE LL Priavcy feature available if BT controller supports
> > LL Privacy when privacy is enabled.
> >
> > The patches are verified in below case.
> > Pairing - where the IRK is added to device Resolving List after
> > identity info & IRK is shared in SMP.
> > Unpair - Delete the resolving entry from the Device resolving list
> > Local IRK change - Whenever there is change in local IRK (privacy) the
> > resolving list entries in the device is updated.
> > Whitelist - filter policies for whitelist is updated based on LL_adress
> > resolution support.
> > Reload Resolving list - device reset/sysmte reboot the Resolving list
> > reloaded in to the device.
> >
> > Sathish Narsimman (7):
> > Bluetooth: LL Privacy Delete Store Resolving list
> > Bluetooth: Add device to Resolving List
> > Bluetooth: Update the Resolving list when Local IRK changed
> > Bluetooth: LL_PRIVACY re-load resolving list
> > Bluetooth: LL Privacy Check and Update the Resolving List
> > Bluetooth: Enable LL Privacy
> > Bluetooth LL Privacy update White list
> >
> > include/net/bluetooth/hci.h | 3 +
> > include/net/bluetooth/hci_core.h | 7 +
> > net/bluetooth/hci_conn.c | 32 ++++
> > net/bluetooth/hci_core.c | 20 ++-
> > net/bluetooth/hci_event.c | 32 +++-
> > net/bluetooth/hci_request.c | 251 ++++++++++++++++++++++++++++++-
> > net/bluetooth/hci_request.h | 2 +
> > net/bluetooth/mgmt.c | 10 +-
> > net/bluetooth/smp.c | 11 ++
> > 9 files changed, 354 insertions(+), 14 deletions(-)
> >
> > --
> > 2.17.1
> >

2020-04-06 16:35:02

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 0/7] LE LL Priavcy support enabled

Hi Abhishek,

> Looking through the patches in this series, it looks like you are
> adding the IRK for all connected devices and I'm not sure that's the
> best method.
> The resolv list seems to be useful in the same way as the le
> whitelist: to filter incoming advertisements for devices we care
> about.
>
> Thus, to simplify your design, could we not do the same thing as the
> le whitelist:
> * Update the resolv list anytime passive/background scan is being enabled
> * Only keep entries in the resolv list that are part of the
> pend_le_conn or pend_le_report lists
>
> Then, you would only need to update the resolv list in
> hci_req_add_le_passive_scan and any IRK changes would just disable
> passive scan, remove IRKs if existing and re-enable passive scan
> (which would add it back with the new one).

so I have been looking at this again and yes, we should just put IRKs in the resolving list for devices that we also put in the whitelist. And we only use the whitelist for background scanning.

This means I would only focus on enabling background scanning. For everything else, we can just let the host do the resolving.

Enable passive scanning
-> Enable resolving list if privacy device in whitelist
-> Set Scan Parameters
-> Set Scan Enable

Disable passive scanning
-> Set Scan Disable
-> Disable resolving list if enabled

And when updating the whitelist, also add update the resolving list with needed entries for the whitelist. This means if the privacy enabled device goes into the whitelist, add the IRK to the resolving list. Remove all no longer needed IRKs.

Regards

Marcel