[PATCH] Add Support to use Resolving list

[PATCH] Add Support to use Resolving list

[spoorthix.k]

From: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>

As per Core specification 5.0, Vol 2, Part E, Section 7.8.38,
following code changes implements LE add device to Resolving List.

Signed-off-by: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_request.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c36dc1e..99a38cf36 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -420,6 +420,7 @@ enum {
 #define HCI_LE_SLAVE_FEATURES		0x08
 #define HCI_LE_PING			0x10
 #define HCI_LE_DATA_LEN_EXT		0x20
+#define HCI_LE_LL_PRIVACY		0x40
 #define HCI_LE_PHY_2M			0x01
 #define HCI_LE_PHY_CODED		0x08
 #define HCI_LE_EXT_ADV			0x10
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index ca73d36..37ee023 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -665,6 +665,72 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
 	}
 }
 
+static void add_to_resolve_list(struct hci_request *req,
+				struct hci_conn_params *params)
+{
+	struct hci_cp_le_add_to_resolv_list cp;
+	struct bdaddr_list_with_irk *entry;
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return;
+
+	memset(&cp, 0, sizeof(cp));
+
+	cp.bdaddr_type = params->addr_type;
+	bacpy(&cp.bdaddr, &params->addr);
+	memcpy(entry->peer_irk, cp.peer_irk, 16);
+	memcpy(entry->local_irk, cp.local_irk, 16);
+
+	hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
+}
+
+static void update_resolve_list(struct hci_request *req)
+{
+	struct hci_dev *hdev = req->hdev;
+	struct hci_conn_params *params;
+	int err;
+	u8 resolve_list_entries = 0;
+
+	list_for_each_entry(params, &hdev->le_resolv_list, action) {
+		/* Cannot Remove or add the device to the Resolving list
+		 * whenever there is an outstanding connection.
+		 */
+		if (!hci_pend_le_action_lookup(&hdev->pend_le_conns,
+					       &params->addr,
+					       params->addr_type) &&
+		    !hci_pend_le_action_lookup(&hdev->pend_le_reports,
+					       &params->addr,
+					       params->addr_type)) {
+			struct hci_cp_le_del_from_resolv_list cp;
+
+			cp.bdaddr_type = params->addr_type;
+			bacpy(&cp.bdaddr, &params->addr);
+
+			hci_req_add(req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
+				    sizeof(cp), &cp);
+			continue;
+		}
+		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
+					   &params->addr, params->addr_type))
+			continue;
+
+		if (hci_find_irk_by_addr(hdev, &params->addr,
+					 params->addr_type)) {
+			/* Add device to resolving list */
+			resolve_list_entries++;
+			add_to_resolve_list(req, params);
+		}
+		if (resolve_list_entries >= hdev->le_resolv_list_size) {
+			err = smp_generate_rpa(hdev, hdev->irk, &hdev->rpa);
+			if (err < 0)
+				BT_ERR("%s failed to generate new RPA",
+				       hdev->name);
+		}
+		resolve_list_entries++;
+	}
+}
+
 static void add_to_white_list(struct hci_request *req,
 			      struct hci_conn_params *params)
 {
@@ -891,6 +957,14 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
 	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY))
 		filter_policy |= 0x02;
 
+	/* When filter policy is not 0x00 (no whitelist) and 0x01
+	 * (whitelist enabled) and if LE Privacy is supported in controller
+	 * add the device to resolving list.
+	 */
+	if ((filter_policy == 0x02 || filter_policy == 0x03) &&
+	    (hci_dev_test_flag(hdev, HCI_LE_LL_PRIVACY)))
+		update_resolve_list(req);
+
 	hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
 			   hdev->le_scan_window, own_addr_type, filter_policy);
 }
-- 
1.9.1

Re: [PATCH] Add Support to use Resolving list

[Marcel Holtmann]

Hi Spoorthi,

> As per Core specification 5.0, Vol 2, Part E, Section 7.8.38,
> following code changes implements LE add device to Resolving List.
> 
> Signed-off-by: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>
> ---
> include/net/bluetooth/hci.h |  1 +
> net/bluetooth/hci_request.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 75 insertions(+)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index c36dc1e..99a38cf36 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -420,6 +420,7 @@ enum {
> #define HCI_LE_SLAVE_FEATURES		0x08
> #define HCI_LE_PING			0x10
> #define HCI_LE_DATA_LEN_EXT		0x20
> +#define HCI_LE_LL_PRIVACY		0x40
> #define HCI_LE_PHY_2M			0x01
> #define HCI_LE_PHY_CODED		0x08
> #define HCI_LE_EXT_ADV			0x10
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index ca73d36..37ee023 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -665,6 +665,72 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
> 	}
> }
> 
> +static void add_to_resolve_list(struct hci_request *req,
> +				struct hci_conn_params *params)
> +{
> +	struct hci_cp_le_add_to_resolv_list cp;
> +	struct bdaddr_list_with_irk *entry;
> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		return;
> +
> +	memset(&cp, 0, sizeof(cp));
> +
> +	cp.bdaddr_type = params->addr_type;
> +	bacpy(&cp.bdaddr, &params->addr);
> +	memcpy(entry->peer_irk, cp.peer_irk, 16);
> +	memcpy(entry->local_irk, cp.local_irk, 16);
> +
> +	hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
> +}
> +
> +static void update_resolve_list(struct hci_request *req)
> +{
> +	struct hci_dev *hdev = req->hdev;
> +	struct hci_conn_params *params;
> +	int err;
> +	u8 resolve_list_entries = 0;
> +
> +	list_for_each_entry(params, &hdev->le_resolv_list, action) {
> +		/* Cannot Remove or add the device to the Resolving list
> +		 * whenever there is an outstanding connection.
> +		 */
> +		if (!hci_pend_le_action_lookup(&hdev->pend_le_conns,
> +					       &params->addr,
> +					       params->addr_type) &&
> +		    !hci_pend_le_action_lookup(&hdev->pend_le_reports,
> +					       &params->addr,
> +					       params->addr_type)) {
> +			struct hci_cp_le_del_from_resolv_list cp;
> +
> +			cp.bdaddr_type = params->addr_type;
> +			bacpy(&cp.bdaddr, &params->addr);
> +
> +			hci_req_add(req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
> +				    sizeof(cp), &cp);
> +			continue;
> +		}

I do not understand this logic. The device gets removed and then you move on to the next one?

> +		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
> +					   &params->addr, params->addr_type))
> +			continue;

What is this good for?

> +
> +		if (hci_find_irk_by_addr(hdev, &params->addr,
> +					 params->addr_type)) {
> +			/* Add device to resolving list */
> +			resolve_list_entries++;
> +			add_to_resolve_list(req, params);
> +		}
> +		if (resolve_list_entries >= hdev->le_resolv_list_size) {
> +			err = smp_generate_rpa(hdev, hdev->irk, &hdev->rpa);
> +			if (err < 0)
> +				BT_ERR("%s failed to generate new RPA",
> +				       hdev->name);
> +		}

Wh are you doing this?

> +		resolve_list_entries++;
> +	}
> +}
> +
> static void add_to_white_list(struct hci_request *req,
> 			      struct hci_conn_params *params)
> {
> @@ -891,6 +957,14 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
> 	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY))
> 		filter_policy |= 0x02;
> 
> +	/* When filter policy is not 0x00 (no whitelist) and 0x01
> +	 * (whitelist enabled) and if LE Privacy is supported in controller
> +	 * add the device to resolving list.
> +	 */
> +	if ((filter_policy == 0x02 || filter_policy == 0x03) &&
> +	    (hci_dev_test_flag(hdev, HCI_LE_LL_PRIVACY)))
> +		update_resolve_list(req);
> +

I do not understand this logic. The resolving list has really nothing to do with the whitelist. Even for active scan we want to use the resolving list for known devices. I mean yes, in theory we could resolve all RPAs in the host during active scan / discovery, but is that really something we want.

> 	hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
> 			   hdev->le_scan_window, own_addr_type, filter_policy);
> }

Regards

Marcel

Re: [PATCH] Add support to use Resolving list

[Johan Hedberg]

Hi Spoorthi,

On 18 Sep 2019, at 13.36, spoorthix.k@intel.com wrote:
> +static void add_to_resolve_list(struct hci_request *req,
> +				struct hci_conn_params *params)
> +{
> +	struct hci_cp_le_add_to_resolv_list cp;
> +	struct bdaddr_list_with_irk *entry;
> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		return;
> +
> +	memset(&cp, 0, sizeof(cp));
> +
> +	cp.bdaddr_type = params->addr_type;
> +	bacpy(&cp.bdaddr, &params->addr);
> +	memcpy(entry->peer_irk, cp.peer_irk, 16);
> +	memcpy(entry->local_irk, cp.local_irk, 16);
> +
> +	hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
> +}

Are you missing a call to add the entry to some list here? As it stands now this just results in the allocated memory being leaked. Where & when should it be freed?

Johan

Re: [PATCH] Add support to use Resolving list

[Marcel Holtmann]

Hi Spoorthi,

please add a proper commit message to the patch. I let this slide for now, but really we have not accepted any patch that doesn’t have a detailed commit message.

> 
> Signed-off-by: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>
> ---
> include/net/bluetooth/hci.h |  1 +
> net/bluetooth/hci_request.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 89 insertions(+)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 5bc1e30..1574dc1 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -433,6 +433,7 @@ enum {
> #define HCI_LE_SLAVE_FEATURES		0x08
> #define HCI_LE_PING			0x10
> #define HCI_LE_DATA_LEN_EXT		0x20
> +#define HCI_LE_LL_PRIVACY		0x40
> #define HCI_LE_PHY_2M			0x01
> #define HCI_LE_PHY_CODED		0x08
> #define HCI_LE_EXT_ADV			0x10
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 621f1a9..2c0d7e8 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -670,6 +670,82 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
> 	}
> }
> 
> +static void add_to_resolve_list(struct hci_request *req,
> +				struct hci_conn_params *params)
> +{
> +	struct hci_cp_le_add_to_resolv_list cp;
> +	struct bdaddr_list_with_irk *entry;
> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		return;
> +
> +	memset(&cp, 0, sizeof(cp));
> +
> +	cp.bdaddr_type = params->addr_type;
> +	bacpy(&cp.bdaddr, &params->addr);
> +	memcpy(entry->peer_irk, cp.peer_irk, 16);
> +	memcpy(entry->local_irk, cp.local_irk, 16);
> +
> +	hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
> +}
> +
> +static void update_resolve_list(struct hci_request *req)
> +{
> +	struct hci_dev *hdev = req->hdev;
> +	struct bdaddr_list *b;
> +	struct hci_conn_params *params;
> +	int err;
> +	u8 resolve_list_entries = 0;
> +
> +	list_for_each_entry(b, &hdev->le_resolv_list, list) {
> +	/* Cannot Remove or add the device to the Resolving list
> +	 * whenever there is an outstanding connection.
> +	 */
> +		if (!hci_pend_le_action_lookup(&hdev->pend_le_conns,
> +					       &b->bdaddr,
> +					       b->bdaddr_type) &&
> +		    !hci_pend_le_action_lookup(&hdev->pend_le_reports,
> +					       &b->bdaddr,
> +					       b->bdaddr_type)) {
> +			struct hci_cp_le_del_from_resolv_list cp;
> +
> +			cp.bdaddr_type = b->bdaddr_type;
> +			bacpy(&cp.bdaddr, &b->bdaddr);
> +
> +			hci_req_add(req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
> +				    sizeof(cp), &cp);
> +		}
> +	}
> +	/* During background scanning/active scanning the
> +	 * device BD address is populated in LE pending
> +	 * connections list. So, track the list and add to Resolving
> +	 * list if found by IRK.
> +	 */
> +	list_for_each_entry(params, &hdev->pend_le_conns, action) {
> +		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
> +					   &params->addr, params->addr_type))
> +			resolve_list_entries++;
> +
> +		if (hci_find_irk_by_addr(hdev, &params->addr,
> +					 params->addr_type)) {
> +			/* Add device to resolving list */
> +			resolve_list_entries++;
> +			add_to_resolve_list(req, params);
> +		}
> +	}
> +
> +	/* Device can be resolved in the Host if size of resolving
> +	 * list is greater than defined in the controller.
> +	 */
> +	if (resolve_list_entries >= hdev->le_resolv_list_size) {
> +		err = smp_generate_rpa(hdev, hdev->irk, &hdev->rpa);
> +		if (err < 0)
> +			BT_ERR("%s failed to generate new RPA",
> +			       hdev->name);
> +		}
> +}
> +
> static void add_to_white_list(struct hci_request *req,
> 			      struct hci_conn_params *params)
> {
> @@ -896,6 +972,12 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
> 	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY))
> 		filter_policy |= 0x02;
> 
> +	/* If LE Privacy is supported in controller
> +	 * add the device to resolving list.
> +	 */
> +	if (hci_dev_test_flag(hdev, HCI_LE_LL_PRIVACY))
> +		update_resolve_list(req);
> +
> 	hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
> 			   hdev->le_scan_window, own_addr_type, filter_policy);
> }
> @@ -2513,6 +2595,12 @@ static int active_scan(struct hci_request *req, unsigned long opt)
> 	if (err < 0)
> 		own_addr_type = ADDR_LE_DEV_PUBLIC;
> 
> +	/* Update resolving list when privacy feature is
> +	 * is enabled in the controller */
> +	if (hci_dev_test_flag(hdev, HCI_LE_LL_PRIVACY)) {
> +		update_resolve_list(req);
> +	}
> +
> 	hci_req_start_scan(req, LE_SCAN_ACTIVE, interval, DISCOV_LE_SCAN_WIN,
> 			   own_addr_type, 0);

I am actually not convinced you have fully tested this patch at all. Maybe it is better you actually write a bunch of test cases for one of our -tester tools and get this automated.

So there are two cases when we want to ensure that the resolving list is up-to-date and correct. And that is mainly just before we either start background scanning (passive scanning) or before we do discovery (active scanning). We also need to ensure that the resolving list is correct when we do advertising (scannable and connectable), but I just ignore that for now.

When we do passive scanning, our whitelist is important. That is why hci_req_le_add_le_passive_scanning has this line:

        /* Adding or removing entries from the white list must                   
         * happen before enabling scanning. The controller does                  
         * not allow white list modification while scanning.                     
         */                                                                      
        filter_policy = update_white_list(req);

Updating the whitelist returns the filter policy on when to use the whitelist. It will not be used when we have an RPA in our device list. However that really only holds true if the controller can not resolve it. Otherwise we can keep using the whitelist.

Now update_white_list contains these pieces:

                if (hci_find_irk_by_addr(hdev, &params->addr,                    
                                         params->addr_type)) {                   
                        /* White list can not be used with RPAs */               
                        return 0x00;                                             
                }                                                

This is no longer a valid statement if we successfully programmed the address into the controller. And if we have switched on resolving list.

So we need to correctly store the current resolving list information and map and in case resolving list is active, we need to add checks here to decide on the filer policy.

For active scanning this is not needed since we are not using the whitelist. Actually for active scanning I don’t even care much if we are using the resolving list. If it is there, great if not, we can just as easily resolve the RPAs in the host. So for active scanning, I would not bother touching the resolving list at all.

Regards

Marcel

[PATCH] Add support to use Resolving list

[spoorthix.k]

From: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>

Signed-off-by: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_request.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5bc1e30..1574dc1 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -433,6 +433,7 @@ enum {
 #define HCI_LE_SLAVE_FEATURES		0x08
 #define HCI_LE_PING			0x10
 #define HCI_LE_DATA_LEN_EXT		0x20
+#define HCI_LE_LL_PRIVACY		0x40
 #define HCI_LE_PHY_2M			0x01
 #define HCI_LE_PHY_CODED		0x08
 #define HCI_LE_EXT_ADV			0x10
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 621f1a9..2c0d7e8 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -670,6 +670,82 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
 	}
 }
 
+static void add_to_resolve_list(struct hci_request *req,
+				struct hci_conn_params *params)
+{
+	struct hci_cp_le_add_to_resolv_list cp;
+	struct bdaddr_list_with_irk *entry;
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return;
+
+	memset(&cp, 0, sizeof(cp));
+
+	cp.bdaddr_type = params->addr_type;
+	bacpy(&cp.bdaddr, &params->addr);
+	memcpy(entry->peer_irk, cp.peer_irk, 16);
+	memcpy(entry->local_irk, cp.local_irk, 16);
+
+	hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
+}
+
+static void update_resolve_list(struct hci_request *req)
+{
+	struct hci_dev *hdev = req->hdev;
+	struct bdaddr_list *b;
+	struct hci_conn_params *params;
+	int err;
+	u8 resolve_list_entries = 0;
+
+	list_for_each_entry(b, &hdev->le_resolv_list, list) {
+	/* Cannot Remove or add the device to the Resolving list
+	 * whenever there is an outstanding connection.
+	 */
+		if (!hci_pend_le_action_lookup(&hdev->pend_le_conns,
+					       &b->bdaddr,
+					       b->bdaddr_type) &&
+		    !hci_pend_le_action_lookup(&hdev->pend_le_reports,
+					       &b->bdaddr,
+					       b->bdaddr_type)) {
+			struct hci_cp_le_del_from_resolv_list cp;
+
+			cp.bdaddr_type = b->bdaddr_type;
+			bacpy(&cp.bdaddr, &b->bdaddr);
+
+			hci_req_add(req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
+				    sizeof(cp), &cp);
+		}
+	}
+	/* During background scanning/active scanning the
+	 * device BD address is populated in LE pending
+	 * connections list. So, track the list and add to Resolving
+	 * list if found by IRK.
+	 */
+	list_for_each_entry(params, &hdev->pend_le_conns, action) {
+		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
+					   &params->addr, params->addr_type))
+			resolve_list_entries++;
+
+		if (hci_find_irk_by_addr(hdev, &params->addr,
+					 params->addr_type)) {
+			/* Add device to resolving list */
+			resolve_list_entries++;
+			add_to_resolve_list(req, params);
+		}
+	}
+
+	/* Device can be resolved in the Host if size of resolving
+	 * list is greater than defined in the controller.
+	 */
+	if (resolve_list_entries >= hdev->le_resolv_list_size) {
+		err = smp_generate_rpa(hdev, hdev->irk, &hdev->rpa);
+		if (err < 0)
+			BT_ERR("%s failed to generate new RPA",
+			       hdev->name);
+		}
+}
+
 static void add_to_white_list(struct hci_request *req,
 			      struct hci_conn_params *params)
 {
@@ -896,6 +972,12 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
 	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY))
 		filter_policy |= 0x02;
 
+	/* If LE Privacy is supported in controller
+	 * add the device to resolving list.
+	 */
+	if (hci_dev_test_flag(hdev, HCI_LE_LL_PRIVACY))
+		update_resolve_list(req);
+
 	hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
 			   hdev->le_scan_window, own_addr_type, filter_policy);
 }
@@ -2513,6 +2595,12 @@ static int active_scan(struct hci_request *req, unsigned long opt)
 	if (err < 0)
 		own_addr_type = ADDR_LE_DEV_PUBLIC;
 
+	/* Update resolving list when privacy feature is
+	 * is enabled in the controller */
+	if (hci_dev_test_flag(hdev, HCI_LE_LL_PRIVACY)) {
+		update_resolve_list(req);
+	}
+
 	hci_req_start_scan(req, LE_SCAN_ACTIVE, interval, DISCOV_LE_SCAN_WIN,
 			   own_addr_type, 0);
 	return 0;
-- 
1.9.1

Re: [PATCH] Add Support to use Resolving List

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3904 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc6 next-20190829]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/spoorthix-k-intel-com/Add-Support-to-use-Resolving-List/20190830-003531
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   net/bluetooth/hci_request.c: In function 'update_resolve_list':
>> net/bluetooth/hci_request.c:735:1: warning: no return statement in function returning non-void [-Wreturn-type]
    }
    ^
   net/bluetooth/hci_request.c: In function 'hci_req_add_le_passive_scan':
>> net/bluetooth/hci_request.c:684:25: warning: 'params' may be used uninitialized in this function [-Wmaybe-uninitialized]
     cp.bdaddr_type = params->addr_type;
                      ~~~~~~^~~~~~~~~~~
   net/bluetooth/hci_request.c:694:26: note: 'params' was declared here
     struct hci_conn_params *params;
                             ^~~~~~

vim +735 net/bluetooth/hci_request.c

   672	
   673	static void add_to_resolve_list(struct hci_request *req,
   674					struct hci_conn_params *params)
   675	{
   676		struct hci_cp_le_add_to_resolv_list cp;
   677		struct bdaddr_list_with_irk *entry;
   678	
   679		entry = kzalloc(sizeof(*entry), GFP_KERNEL);
   680		if (!entry)
   681			return;
   682		memset(&cp, 0, sizeof(cp));
   683	
 > 684		cp.bdaddr_type = params->addr_type;
   685		bacpy(&cp.bdaddr, &params->addr);
   686		memcpy(entry->peer_irk, cp.peer_irk, 16);
   687		memcpy(entry->local_irk, cp.local_irk, 16);
   688		hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
   689	}
   690	
   691	static u8 update_resolve_list(struct hci_request *req)
   692	{
   693		struct hci_dev *hdev = req->hdev;
   694		struct hci_conn_params *params;
   695		struct bdaddr_list *b;
   696		int err;
   697		u8 resolve_list_entries = 0;
   698	
   699		list_for_each_entry(b, &hdev->le_resolv_list, list) {
   700			/* Cannot Remove or add the device to the Resolving list
   701			 * whenever there is an outstanding connection.
   702			 */
   703			if (!hci_pend_le_action_lookup(&hdev->pend_le_conns,
   704						       &b->bdaddr, b->bdaddr_type) &&
   705			    !hci_pend_le_action_lookup(&hdev->pend_le_reports,
   706						       &b->bdaddr, b->bdaddr_type)) {
   707				struct hci_cp_le_del_from_resolv_list cp;
   708	
   709				cp.bdaddr_type = b->bdaddr_type;
   710				bacpy(&cp.bdaddr, &b->bdaddr);
   711	
   712				hci_req_add(req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
   713					    sizeof(cp), &cp);
   714				continue;
   715			}
   716	
   717			if (hci_find_irk_by_addr(hdev, &b->bdaddr, b->bdaddr_type)) {
   718				/* Add device to resolving list */
   719				resolve_list_entries++;
   720				add_to_resolve_list(req, params);
   721			}
   722	
   723			if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
   724						   &params->addr, params->addr_type))
   725				continue;
   726	
   727			if (resolve_list_entries >= hdev->le_resolv_list_size) {
   728				err = smp_generate_rpa(hdev, hdev->irk, &hdev->rpa);
   729				if (err < 0)
   730					BT_ERR("%s failed to generate new RPA",
   731					       hdev->name);
   732			}
   733			resolve_list_entries++;
   734		}
 > 735	}
   736	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50935 bytes --]

[PATCH] Add Support to use Resolving List

[spoorthix.k]

From: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>

As per Core specification 5.0, Vol 2, Part E, Section 7.8.38,
following code changes implements LE add device to Resolving List.

Signed-off-by: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_request.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c36dc1e..99a38cf36 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -420,6 +420,7 @@ enum {
 #define HCI_LE_SLAVE_FEATURES		0x08
 #define HCI_LE_PING			0x10
 #define HCI_LE_DATA_LEN_EXT		0x20
+#define HCI_LE_LL_PRIVACY		0x40
 #define HCI_LE_PHY_2M			0x01
 #define HCI_LE_PHY_CODED		0x08
 #define HCI_LE_EXT_ADV			0x10
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index ca73d36..48a1777 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -665,6 +665,70 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
 	}
 }
 
+static void add_to_resolve_list(struct hci_request *req,
+				struct hci_conn_params *params)
+{
+	struct hci_cp_le_add_to_resolv_list cp;
+	struct bdaddr_list_with_irk *entry;
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return;
+	memset(&cp, 0, sizeof(cp));
+
+	cp.bdaddr_type = params->addr_type;
+	bacpy(&cp.bdaddr, &params->addr);
+	memcpy(entry->peer_irk, cp.peer_irk, 16);
+	memcpy(entry->local_irk, cp.local_irk, 16);
+	hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
+}
+
+static u8 update_resolve_list(struct hci_request *req)
+{
+	struct hci_dev *hdev = req->hdev;
+	struct hci_conn_params *params;
+	struct bdaddr_list *b;
+	int err;
+	u8 resolve_list_entries = 0;
+
+	list_for_each_entry(b, &hdev->le_resolv_list, list) {
+		/* Cannot Remove or add the device to the Resolving list
+		 * whenever there is an outstanding connection.
+		 */
+		if (!hci_pend_le_action_lookup(&hdev->pend_le_conns,
+					       &b->bdaddr, b->bdaddr_type) &&
+		    !hci_pend_le_action_lookup(&hdev->pend_le_reports,
+					       &b->bdaddr, b->bdaddr_type)) {
+			struct hci_cp_le_del_from_resolv_list cp;
+
+			cp.bdaddr_type = b->bdaddr_type;
+			bacpy(&cp.bdaddr, &b->bdaddr);
+
+			hci_req_add(req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
+				    sizeof(cp), &cp);
+			continue;
+		}
+
+		if (hci_find_irk_by_addr(hdev, &b->bdaddr, b->bdaddr_type)) {
+			/* Add device to resolving list */
+			resolve_list_entries++;
+			add_to_resolve_list(req, params);
+		}
+
+		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
+					   &params->addr, params->addr_type))
+			continue;
+
+		if (resolve_list_entries >= hdev->le_resolv_list_size) {
+			err = smp_generate_rpa(hdev, hdev->irk, &hdev->rpa);
+			if (err < 0)
+				BT_ERR("%s failed to generate new RPA",
+				       hdev->name);
+		}
+		resolve_list_entries++;
+	}
+}
+
 static void add_to_white_list(struct hci_request *req,
 			      struct hci_conn_params *params)
 {
@@ -891,6 +955,14 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
 	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY))
 		filter_policy |= 0x02;
 
+	/* When filter policy is not 0x00 (no whitelist) and 0x01
+	 * (whitelist enabled) and if LE Privacy is supported in controller
+	 * add the device to resolving list.
+	 */
+	if ((filter_policy == 0x02 || filter_policy == 0x03) &&
+	    (hci_dev_test_flag(hdev, HCI_LE_LL_PRIVACY)))
+		update_resolve_list(req);
+
 	hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
 			   hdev->le_scan_window, own_addr_type, filter_policy);
 }
-- 
1.9.1

Re: [PATCH] Add support to use resolving list

[Marcel Holtmann]

Hi Spoorthi,

> As per Core specification 5.0, Vol 2, Part E, Section 7.8.38,
> following code changes implements LE add device to Resolving List.
> 
> Signed-off-by: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>
> ---
> include/net/bluetooth/hci.h |   1 +
> net/bluetooth/hci_request.c | 134 ++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 130 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index c36dc1e..99a38cf36 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -420,6 +420,7 @@ enum {
> #define HCI_LE_SLAVE_FEATURES		0x08
> #define HCI_LE_PING			0x10
> #define HCI_LE_DATA_LEN_EXT		0x20
> +#define HCI_LE_LL_PRIVACY		0x40
> #define HCI_LE_PHY_2M			0x01
> #define HCI_LE_PHY_CODED		0x08
> #define HCI_LE_EXT_ADV			0x10
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index ca73d36..868a592 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -672,7 +672,6 @@ static void add_to_white_list(struct hci_request *req,
> 
> 	cp.bdaddr_type = params->addr_type;
> 	bacpy(&cp.bdaddr, &params->addr);
> -
> 	hci_req_add(req, HCI_OP_LE_ADD_TO_WHITE_LIST, sizeof(cp), &cp);
> }
> 
> @@ -681,7 +680,7 @@ static u8 update_white_list(struct hci_request *req)
> 	struct hci_dev *hdev = req->hdev;
> 	struct hci_conn_params *params;
> 	struct bdaddr_list *b;
> -	uint8_t white_list_entries = 0;
> +	u8 white_list_entries = 0;

I prefer if these kind of fixes come in via separate patches. They distract from the actual parts this feature changes.

> 
> 	/* Go through the current white list programmed into the
> 	 * controller one by one and check if that address is still
> @@ -773,6 +772,111 @@ static u8 update_white_list(struct hci_request *req)
> 	return 0x01;
> }
> 
> +static void add_to_resolve_list(struct hci_request *req,
> +				struct hci_conn_params *params)
> +{
> +	struct hci_cp_le_add_to_resolv_list cp;
> +	struct bdaddr_list_with_irk *entry;
> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		return;
> +

In general we memset the cp structures first. And if there might be some missing, then that would warrant fixes this separately.

> +	cp.bdaddr_type = params->addr_type;
> +	bacpy(&cp.bdaddr, &params->addr);
> +	memcpy(entry->peer_irk, cp.peer_irk, 16);
> +	memcpy(entry->local_irk, cp.local_irk, 16);
> +	hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
> +}
> +
> +static u8 update_resolve_list(struct hci_request *req)
> +{
> +	struct hci_dev *hdev = req->hdev;
> +	struct hci_conn_params *params;
> +	struct bdaddr_list *b;
> +	u8 resolve_list_entries = 0;
> +
> +	/* Go through the current resolving list programmed into the
> +	 * controller one by one and check if that address is still
> +	 * in the list of pending connections or list of devices to
> +	 * report. If not present in either list, then queue
> +	 * command to remove it from the controller.
> +	 */
> +

What is up extra empty lines here. Please following the coding style since me having to point this out all the time is not going to make want to review patches.

> +	list_for_each_entry(b, &hdev->le_resolv_list, list) {
> +		/* If the device is neither in pend_le_conns nor
> +		 * pend_le_reports then remove it from the resolving list.
> +		 */
> +		if (!hci_pend_le_action_lookup(&hdev->pend_le_conns,
> +					       &b->bdaddr, b->bdaddr_type) &&
> +		    !hci_pend_le_action_lookup(&hdev->pend_le_reports,
> +					       &b->bdaddr, b->bdaddr_type)) {
> +			struct hci_cp_le_del_from_resolv_list cp;
> +
> +			cp.bdaddr_type = b->bdaddr_type;
> +			bacpy(&cp.bdaddr, &b->bdaddr);
> +			hci_req_add(req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
> +				    sizeof(cp), &cp);
> +			continue;
> +		}
> +		if (hci_find_irk_by_addr(hdev, &b->bdaddr, b->bdaddr_type))
> +			return 0x00;
> +
> +		resolve_list_entries++;
> +	}

Why would we bother with this at all. This is the resolving list and not the whitelist. The resolving list has nothing to do with connection or pending connections. I would leave the addresses in the controller to allow resolving.

> +
> +	/* Since all no longer valid resolve list entries have been
> +	 * removed, walk through the list of pending connections
> +	 * and ensure that any new device gets programmed into
> +	 * the controller.
> +	 *
> +	 * If the list of the devices is larger than the list of
> +	 * available resolve list entries in the controller, then
> +	 * just abort and return filer policy value to not use the
> +	 * resolve list.
> +	 */

But resolving list has nothing to do with the filter policy.

> +
> +	list_for_each_entry(params, &hdev->pend_le_conns, action) {
> +		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
> +					   &params->addr, params->addr_type))
> +			continue;
> +
> +		if (resolve_list_entries >= hdev->le_resolv_list_size) {
> +			/* Select filter policy to accept all advertising */
> +			return 0x00;
> +		}
> +
> +		if (hci_find_irk_by_addr(hdev, &params->addr,
> +					 params->addr_type)) {
> +			return 0x00;
> +		}
> +		resolve_list_entries++;
> +		add_to_resolve_list(req, params);
> +	}
> +
> +	/* After adding all new pending connections, walk through
> +	 * the list of pending reports and also add these to the
> +	 * resolving list if there is still space.
> +	 */
> +
> +	list_for_each_entry(params, &hdev->pend_le_reports, action) {
> +		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
> +					   &params->addr, params->addr_type))
> +			continue;
> +
> +		if (resolve_list_entries >= hdev->le_resolv_list_size)
> +			return 0x00;
> +
> +		if (hci_find_irk_by_addr(hdev, &params->addr,
> +					 params->addr_type)) {
> +			return 0x00;
> +		}
> +		resolve_list_entries++;
> +		add_to_resolve_list(req, params);
> +	}
> +	return 0x02;
> +}
> +
> static bool scan_use_rpa(struct hci_dev *hdev)
> {
> 	return hci_dev_test_flag(hdev, HCI_PRIVACY);
> @@ -861,6 +965,7 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
> 	struct hci_dev *hdev = req->hdev;
> 	u8 own_addr_type;
> 	u8 filter_policy;
> +	u8 ext_filter_policy;
> 
> 	/* Set require_privacy to false since no SCAN_REQ are send
> 	 * during passive scanning. Not using an non-resolvable address
> @@ -878,6 +983,12 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
> 	 */
> 	filter_policy = update_white_list(req);
> 
> +	/* Adding or removing entries from the resolve list must
> +	 * happen before enabling scanning. The controller does
> +	 * not allow resolve list modification while scanning.
> +	 */
> +	ext_filter_policy = update_resolve_list(req);
> +
> 	/* When the controller is using random resolvable addresses and
> 	 * with that having LE privacy enabled, then controllers with
> 	 * Extended Scanner Filter Policies support can now enable support
> @@ -888,11 +999,24 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
> 	 * 0x02 (no whitelist) and 0x03 (whitelist enabled).
> 	 */
> 	if (hci_dev_test_flag(hdev, HCI_PRIVACY) &&
> -	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY))
> +	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY) &&
> +	     (!(hdev->le_features[0] & HCI_LE_LL_PRIVACY)))
> 		filter_policy |= 0x02;

This is not correct. What we need to know is if we have more IRKs available than we have space in the resolving list. If that is the case, then we let the controller report up unresolved RPAs to the host to resolve. This is not a mutual exclusive setting in comparison with link layer privacy. It is complementary.

> 
> -	hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
> -			   hdev->le_scan_window, own_addr_type, filter_policy);
> +	if (!ext_filter_policy && (hdev->le_features[0] & HCI_LE_LL_PRIVACY)) {
> +		/* If resolve list can not be used then check if
> +		 * whitelist can be used and set filter policy
> +		 * accordingly.
> +		 */

I do not understand this logic.

> +		ext_filter_policy = filter_policy;
> +		hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
> +				   hdev->le_scan_window, own_addr_type,
> +				   ext_filter_policy);
> +	} else {
> +		hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
> +				   hdev->le_scan_window, own_addr_type,
> +				   filter_policy);
> +	}
> }
> 
> static u8 get_adv_instance_scan_rsp_len(struct hci_dev *hdev, u8 instance)

Regards

Marcel

[PATCH] Add support to use resolving list

[spoorthix.k]

From: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>

As per Core specification 5.0, Vol 2, Part E, Section 7.8.38,
following code changes implements LE add device to Resolving List.

Signed-off-by: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>
---
 include/net/bluetooth/hci.h |   1 +
 net/bluetooth/hci_request.c | 134 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 130 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c36dc1e..99a38cf36 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -420,6 +420,7 @@ enum {
 #define HCI_LE_SLAVE_FEATURES		0x08
 #define HCI_LE_PING			0x10
 #define HCI_LE_DATA_LEN_EXT		0x20
+#define HCI_LE_LL_PRIVACY		0x40
 #define HCI_LE_PHY_2M			0x01
 #define HCI_LE_PHY_CODED		0x08
 #define HCI_LE_EXT_ADV			0x10
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index ca73d36..868a592 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -672,7 +672,6 @@ static void add_to_white_list(struct hci_request *req,
 
 	cp.bdaddr_type = params->addr_type;
 	bacpy(&cp.bdaddr, &params->addr);
-
 	hci_req_add(req, HCI_OP_LE_ADD_TO_WHITE_LIST, sizeof(cp), &cp);
 }
 
@@ -681,7 +680,7 @@ static u8 update_white_list(struct hci_request *req)
 	struct hci_dev *hdev = req->hdev;
 	struct hci_conn_params *params;
 	struct bdaddr_list *b;
-	uint8_t white_list_entries = 0;
+	u8 white_list_entries = 0;
 
 	/* Go through the current white list programmed into the
 	 * controller one by one and check if that address is still
@@ -773,6 +772,111 @@ static u8 update_white_list(struct hci_request *req)
 	return 0x01;
 }
 
+static void add_to_resolve_list(struct hci_request *req,
+				struct hci_conn_params *params)
+{
+	struct hci_cp_le_add_to_resolv_list cp;
+	struct bdaddr_list_with_irk *entry;
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return;
+
+	cp.bdaddr_type = params->addr_type;
+	bacpy(&cp.bdaddr, &params->addr);
+	memcpy(entry->peer_irk, cp.peer_irk, 16);
+	memcpy(entry->local_irk, cp.local_irk, 16);
+	hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
+}
+
+static u8 update_resolve_list(struct hci_request *req)
+{
+	struct hci_dev *hdev = req->hdev;
+	struct hci_conn_params *params;
+	struct bdaddr_list *b;
+	u8 resolve_list_entries = 0;
+
+	/* Go through the current resolving list programmed into the
+	 * controller one by one and check if that address is still
+	 * in the list of pending connections or list of devices to
+	 * report. If not present in either list, then queue
+	 * command to remove it from the controller.
+	 */
+
+	list_for_each_entry(b, &hdev->le_resolv_list, list) {
+		/* If the device is neither in pend_le_conns nor
+		 * pend_le_reports then remove it from the resolving list.
+		 */
+		if (!hci_pend_le_action_lookup(&hdev->pend_le_conns,
+					       &b->bdaddr, b->bdaddr_type) &&
+		    !hci_pend_le_action_lookup(&hdev->pend_le_reports,
+					       &b->bdaddr, b->bdaddr_type)) {
+			struct hci_cp_le_del_from_resolv_list cp;
+
+			cp.bdaddr_type = b->bdaddr_type;
+			bacpy(&cp.bdaddr, &b->bdaddr);
+			hci_req_add(req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
+				    sizeof(cp), &cp);
+			continue;
+		}
+		if (hci_find_irk_by_addr(hdev, &b->bdaddr, b->bdaddr_type))
+			return 0x00;
+
+		resolve_list_entries++;
+	}
+
+	/* Since all no longer valid resolve list entries have been
+	 * removed, walk through the list of pending connections
+	 * and ensure that any new device gets programmed into
+	 * the controller.
+	 *
+	 * If the list of the devices is larger than the list of
+	 * available resolve list entries in the controller, then
+	 * just abort and return filer policy value to not use the
+	 * resolve list.
+	 */
+
+	list_for_each_entry(params, &hdev->pend_le_conns, action) {
+		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
+					   &params->addr, params->addr_type))
+			continue;
+
+		if (resolve_list_entries >= hdev->le_resolv_list_size) {
+			/* Select filter policy to accept all advertising */
+			return 0x00;
+		}
+
+		if (hci_find_irk_by_addr(hdev, &params->addr,
+					 params->addr_type)) {
+			return 0x00;
+		}
+		resolve_list_entries++;
+		add_to_resolve_list(req, params);
+	}
+
+	/* After adding all new pending connections, walk through
+	 * the list of pending reports and also add these to the
+	 * resolving list if there is still space.
+	 */
+
+	list_for_each_entry(params, &hdev->pend_le_reports, action) {
+		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
+					   &params->addr, params->addr_type))
+			continue;
+
+		if (resolve_list_entries >= hdev->le_resolv_list_size)
+			return 0x00;
+
+		if (hci_find_irk_by_addr(hdev, &params->addr,
+					 params->addr_type)) {
+			return 0x00;
+		}
+		resolve_list_entries++;
+		add_to_resolve_list(req, params);
+	}
+	return 0x02;
+}
+
 static bool scan_use_rpa(struct hci_dev *hdev)
 {
 	return hci_dev_test_flag(hdev, HCI_PRIVACY);
@@ -861,6 +965,7 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
 	struct hci_dev *hdev = req->hdev;
 	u8 own_addr_type;
 	u8 filter_policy;
+	u8 ext_filter_policy;
 
 	/* Set require_privacy to false since no SCAN_REQ are send
 	 * during passive scanning. Not using an non-resolvable address
@@ -878,6 +983,12 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
 	 */
 	filter_policy = update_white_list(req);
 
+	/* Adding or removing entries from the resolve list must
+	 * happen before enabling scanning. The controller does
+	 * not allow resolve list modification while scanning.
+	 */
+	ext_filter_policy = update_resolve_list(req);
+
 	/* When the controller is using random resolvable addresses and
 	 * with that having LE privacy enabled, then controllers with
 	 * Extended Scanner Filter Policies support can now enable support
@@ -888,11 +999,24 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
 	 * 0x02 (no whitelist) and 0x03 (whitelist enabled).
 	 */
 	if (hci_dev_test_flag(hdev, HCI_PRIVACY) &&
-	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY))
+	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY) &&
+	     (!(hdev->le_features[0] & HCI_LE_LL_PRIVACY)))
 		filter_policy |= 0x02;
 
-	hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
-			   hdev->le_scan_window, own_addr_type, filter_policy);
+	if (!ext_filter_policy && (hdev->le_features[0] & HCI_LE_LL_PRIVACY)) {
+		/* If resolve list can not be used then check if
+		 * whitelist can be used and set filter policy
+		 * accordingly.
+		 */
+		ext_filter_policy = filter_policy;
+		hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
+				   hdev->le_scan_window, own_addr_type,
+				   ext_filter_policy);
+	} else {
+		hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
+				   hdev->le_scan_window, own_addr_type,
+				   filter_policy);
+	}
 }
 
 static u8 get_adv_instance_scan_rsp_len(struct hci_dev *hdev, u8 instance)
-- 
1.9.1

Re: [PATCH] Add support to use resolving list

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3908 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/spoorthix-k-intel-com/Add-support-to-use-resolving-list/20190812-233201
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   net//bluetooth/hci_request.c: In function 'hci_req_add_le_passive_scan':
>> net//bluetooth/hci_request.c:1015:3: warning: 'ext_filter_policy' may be used uninitialized in this function [-Wmaybe-uninitialized]
      hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           hdev->le_scan_window, own_addr_type,
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           ext_filter_policy);
           ~~~~~~~~~~~~~~~~~~

vim +/ext_filter_policy +1015 net//bluetooth/hci_request.c

   966	
   967	void hci_req_add_le_passive_scan(struct hci_request *req)
   968	{
   969		struct hci_dev *hdev = req->hdev;
   970		u8 own_addr_type;
   971		u8 filter_policy;
   972		u8 ext_filter_policy;
   973	
   974		/* Set require_privacy to false since no SCAN_REQ are send
   975		 * during passive scanning. Not using an non-resolvable address
   976		 * here is important so that peer devices using direct
   977		 * advertising with our address will be correctly reported
   978		 * by the controller.
   979		 */
   980		if (hci_update_random_address(req, false, scan_use_rpa(hdev),
   981					      &own_addr_type))
   982			return;
   983	
   984		/* Adding or removing entries from the white list must
   985		 * happen before enabling scanning. The controller does
   986		 * not allow white list modification while scanning.
   987		 */
   988		filter_policy = update_white_list(req);
   989	
   990		if (hdev->le_features[0] & HCI_LE_LL_PRIVACY) {
   991			ext_filter_policy = update_resolve_list(req);
   992			if (!ext_filter_policy) {
   993				/* If resolve list can not be used then check if
   994				 * whitelist can be used and set filter policy
   995				 * accordingly.
   996				 */
   997				ext_filter_policy = filter_policy;
   998			}
   999		}
  1000		/* When the controller is using random resolvable addresses and
  1001		 * with that having LE privacy enabled, then controllers with
  1002		 * Extended Scanner Filter Policies support can now enable support
  1003		 * for handling directed advertising.
  1004		 *
  1005		 * So instead of using filter polices 0x00 (no whitelist)
  1006		 * and 0x01 (whitelist enabled) use the new filter policies
  1007		 * 0x02 (no whitelist) and 0x03 (whitelist enabled).
  1008		 */
  1009		if (hci_dev_test_flag(hdev, HCI_PRIVACY) &&
  1010		    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY) &&
  1011		     (!(hdev->le_features[0] & HCI_LE_LL_PRIVACY)))
  1012			filter_policy |= 0x02;
  1013	
  1014		if (hdev->le_features[0] & HCI_LE_LL_PRIVACY)
> 1015			hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
  1016					   hdev->le_scan_window, own_addr_type,
  1017					   ext_filter_policy);
  1018		else
  1019			hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
  1020					   hdev->le_scan_window, own_addr_type,
  1021					   filter_policy);
  1022	}
  1023	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69475 bytes --]

[PATCH] Add support to use resolving list

[spoorthix.k]

From: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>

As per Core specification 5.0, Vol 2, Part E, Section 7.8.38,
following code changes implements LE add device to Resolving List.

Signed-off-by: Spoorthi Ravishankar Koppad <spoorthix.k@intel.com>
---
 include/net/bluetooth/hci.h |   1 +
 net/bluetooth/hci_request.c | 131 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 127 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c36dc1e..99a38cf36 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -420,6 +420,7 @@ enum {
 #define HCI_LE_SLAVE_FEATURES		0x08
 #define HCI_LE_PING			0x10
 #define HCI_LE_DATA_LEN_EXT		0x20
+#define HCI_LE_LL_PRIVACY		0x40
 #define HCI_LE_PHY_2M			0x01
 #define HCI_LE_PHY_CODED		0x08
 #define HCI_LE_EXT_ADV			0x10
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index ca73d36..7ffc962 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -672,7 +672,6 @@ static void add_to_white_list(struct hci_request *req,
 
 	cp.bdaddr_type = params->addr_type;
 	bacpy(&cp.bdaddr, &params->addr);
-
 	hci_req_add(req, HCI_OP_LE_ADD_TO_WHITE_LIST, sizeof(cp), &cp);
 }
 
@@ -681,7 +680,7 @@ static u8 update_white_list(struct hci_request *req)
 	struct hci_dev *hdev = req->hdev;
 	struct hci_conn_params *params;
 	struct bdaddr_list *b;
-	uint8_t white_list_entries = 0;
+	u8 white_list_entries = 0;
 
 	/* Go through the current white list programmed into the
 	 * controller one by one and check if that address is still
@@ -773,6 +772,110 @@ static u8 update_white_list(struct hci_request *req)
 	return 0x01;
 }
 
+static void add_to_resolve_list(struct hci_request *req,
+				struct hci_conn_params *params)
+{
+	struct hci_cp_le_add_to_resolv_list cp;
+	struct bdaddr_list_with_irk *entry;
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return;
+
+	cp.bdaddr_type = params->addr_type;
+	bacpy(&cp.bdaddr, &params->addr);
+	memcpy(entry->peer_irk, cp.peer_irk, 16);
+	memcpy(entry->local_irk, cp.local_irk, 16);
+	hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST, sizeof(cp), &cp);
+}
+
+static u8 update_resolve_list(struct hci_request *req)
+{
+	struct hci_dev *hdev = req->hdev;
+	struct hci_conn_params *params;
+	struct bdaddr_list *b;
+	u8 resolve_list_entries = 0;
+
+	/* Go through the current resolving list programmed into the
+	 * controller one by one and check if that address is still
+	 * in the list of pending connections or list of devices to
+	 * report. If not present in either list, then queue
+	 * command to remove it from the controller.
+	 */
+
+	list_for_each_entry(b, &hdev->le_resolv_list, list) {
+		/* If the device is neither in pend_le_conns nor
+		 * pend_le_reports then remove it from the resolving list.
+		 */
+		if (!hci_pend_le_action_lookup(&hdev->pend_le_conns,
+					       &b->bdaddr, b->bdaddr_type) &&
+		    !hci_pend_le_action_lookup(&hdev->pend_le_reports,
+					       &b->bdaddr, b->bdaddr_type)) {
+			struct hci_cp_le_del_from_resolv_list cp;
+
+			cp.bdaddr_type = b->bdaddr_type;
+			bacpy(&cp.bdaddr, &b->bdaddr);
+			hci_req_add(req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
+				    sizeof(cp), &cp);
+			continue;
+		}
+		if (hci_find_irk_by_addr(hdev, &b->bdaddr, b->bdaddr_type))
+			return 0x00;
+
+		resolve_list_entries++;
+	}
+
+	/* Since all no longer valid resolve list entries have been
+	 * removed, walk through the list of pending connections
+	 * and ensure that any new device gets programmed into
+	 * the controller.
+	 *
+	 * If the list of the devices is larger than the list of
+	 * available resolve list entries in the controller, then
+	 * just abort and return filer policy value to not use the
+	 * resolve list.
+	 */
+
+	list_for_each_entry(params, &hdev->pend_le_conns, action) {
+		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
+					   &params->addr, params->addr_type))
+			continue;
+
+		if (resolve_list_entries >= hdev->le_resolv_list_size) {
+			/* Select filter policy to accept all advertising */
+			return 0x00;
+		}
+
+		if (hci_find_irk_by_addr(hdev, &params->addr,
+					 params->addr_type))
+			return 0x00;
+
+		resolve_list_entries++;
+		add_to_resolve_list(req, params);
+	}
+
+	/* After adding all new pending connections, walk through
+	 * the list of pending reports and also add these to the
+	 * resolving list if there is still space.
+	 */
+
+	list_for_each_entry(params, &hdev->pend_le_reports, action) {
+		if (hci_bdaddr_list_lookup(&hdev->le_resolv_list,
+					   &params->addr, params->addr_type))
+			continue;
+		if (resolve_list_entries >= hdev->le_resolv_list_size)
+			return 0x00;
+
+		if (hci_find_irk_by_addr(hdev, &params->addr,
+					 params->addr_type))
+			return 0x00;
+
+		resolve_list_entries++;
+		add_to_resolve_list(req, params);
+	}
+	return 0x02;
+}
+
 static bool scan_use_rpa(struct hci_dev *hdev)
 {
 	return hci_dev_test_flag(hdev, HCI_PRIVACY);
@@ -861,6 +964,7 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
 	struct hci_dev *hdev = req->hdev;
 	u8 own_addr_type;
 	u8 filter_policy;
+	u8 ext_filter_policy;
 
 	/* Set require_privacy to false since no SCAN_REQ are send
 	 * during passive scanning. Not using an non-resolvable address
@@ -878,6 +982,16 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
 	 */
 	filter_policy = update_white_list(req);
 
+	if (hdev->le_features[0] & HCI_LE_LL_PRIVACY) {
+		ext_filter_policy = update_resolve_list(req);
+		if (!ext_filter_policy) {
+			/* If resolve list can not be used then check if
+			 * whitelist can be used and set filter policy
+			 * accordingly.
+			 */
+			ext_filter_policy = filter_policy;
+		}
+	}
 	/* When the controller is using random resolvable addresses and
 	 * with that having LE privacy enabled, then controllers with
 	 * Extended Scanner Filter Policies support can now enable support
@@ -888,11 +1002,18 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
 	 * 0x02 (no whitelist) and 0x03 (whitelist enabled).
 	 */
 	if (hci_dev_test_flag(hdev, HCI_PRIVACY) &&
-	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY))
+	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY) &&
+	     (!(hdev->le_features[0] & HCI_LE_LL_PRIVACY)))
 		filter_policy |= 0x02;
 
-	hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
-			   hdev->le_scan_window, own_addr_type, filter_policy);
+	if (hdev->le_features[0] & HCI_LE_LL_PRIVACY)
+		hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
+				   hdev->le_scan_window, own_addr_type,
+				   ext_filter_policy);
+	else
+		hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
+				   hdev->le_scan_window, own_addr_type,
+				   filter_policy);
 }
 
 static u8 get_adv_instance_scan_rsp_len(struct hci_dev *hdev, u8 instance)
-- 
1.9.1