[PATCH v2 0/7] Bluetooth: White list support for BR/EDR

[PATCH v2 0/7] Bluetooth: White list support for BR/EDR

[johan.hedberg]

Hi,

Here's a v2 with fixes for single-mode controllers and updating page
scan when clearing all entries.

>From original cover letter:

This patch set extends the Add/Remove Device commands with the
possibility of passing BR/EDR addresses to them. For now only action
0x01 ("connect") is supported. It has the behavior of enabling page scan
even if the adapter is not set as connectable and will accept
connections from devices which are in the white list.

Johan

----------------------------------------------------------------
Johan Hedberg (7):
      Bluetooth: Unify helpers for bdaddr_list manipulations
      Bluetooth: Introduce a whitelist for BR/EDR devices
      Bluetooth: Update page scan when necessary for Add/Remove Device
      Bluetooth: Fix incorrectly setting HCI_CONNECTABLE
      Bluetooth: Refactor connection request handling
      Bluetooth: Enable page scan also if there are white list entries
      Bluetooth: Add white list lookup for incoming connection requests

 include/net/bluetooth/hci_core.h |  16 ++---
 net/bluetooth/hci_core.c         | 114 +++++++++++--------------------
 net/bluetooth/hci_event.c        | 133 ++++++++++++++++++++----------------
 net/bluetooth/hci_sock.c         |   4 +-
 net/bluetooth/l2cap_core.c       |   7 +-
 net/bluetooth/mgmt.c             |  99 +++++++++++++++++++++++++--
 6 files changed, 222 insertions(+), 151 deletions(-)

[PATCH v2 1/7] Bluetooth: Unify helpers for bdaddr_list manipulations

[johan.hedberg]

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

We already have several lists with struct bdaddr_list entries, and there
will be more in the future. Since the operations for adding, removing,
looking up and clearing entries in these lists are exactly the same it
doesn't make sense to define new functions for every single list. This
patch unifies the functions by passing the list_head to them instead of
a hci_dev pointer.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h | 15 +++----
 net/bluetooth/hci_core.c         | 85 ++++++----------------------------------
 net/bluetooth/hci_event.c        | 15 ++++---
 net/bluetooth/hci_sock.c         |  4 +-
 net/bluetooth/l2cap_core.c       |  7 ++--
 net/bluetooth/mgmt.c             |  6 ++-
 6 files changed, 36 insertions(+), 96 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c98de309967e..3a1caf10cc8d 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -849,16 +849,11 @@ int hci_get_conn_info(struct hci_dev *hdev, void __user *arg);
 int hci_get_auth_info(struct hci_dev *hdev, void __user *arg);
 int hci_inquiry(void __user *arg);
 
-struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev,
-					 bdaddr_t *bdaddr, u8 type);
-int hci_blacklist_add(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
-int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
-
-struct bdaddr_list *hci_white_list_lookup(struct hci_dev *hdev,
-					  bdaddr_t *bdaddr, u8 type);
-void hci_white_list_clear(struct hci_dev *hdev);
-int hci_white_list_add(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
-int hci_white_list_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
+struct bdaddr_list *hci_bdaddr_list_lookup(struct list_head *list,
+					   bdaddr_t *bdaddr, u8 type);
+int hci_bdaddr_list_add(struct list_head *list, bdaddr_t *bdaddr, u8 type);
+int hci_bdaddr_list_del(struct list_head *list, bdaddr_t *bdaddr, u8 type);
+void hci_bdaddr_list_clear(struct list_head *list);
 
 struct hci_conn_params *hci_conn_params_lookup(struct hci_dev *hdev,
 					       bdaddr_t *addr, u8 addr_type);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 421faf5fa1f5..705f8df7af96 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3351,12 +3351,12 @@ int hci_add_remote_oob_ext_data(struct hci_dev *hdev, bdaddr_t *bdaddr,
 	return 0;
 }
 
-struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev,
+struct bdaddr_list *hci_bdaddr_list_lookup(struct list_head *bdaddr_list,
 					 bdaddr_t *bdaddr, u8 type)
 {
 	struct bdaddr_list *b;
 
-	list_for_each_entry(b, &hdev->blacklist, list) {
+	list_for_each_entry(b, bdaddr_list, list) {
 		if (!bacmp(&b->bdaddr, bdaddr) && b->bdaddr_type == type)
 			return b;
 	}
@@ -3364,11 +3364,11 @@ struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev,
 	return NULL;
 }
 
-static void hci_blacklist_clear(struct hci_dev *hdev)
+void hci_bdaddr_list_clear(struct list_head *bdaddr_list)
 {
 	struct list_head *p, *n;
 
-	list_for_each_safe(p, n, &hdev->blacklist) {
+	list_for_each_safe(p, n, bdaddr_list) {
 		struct bdaddr_list *b = list_entry(p, struct bdaddr_list, list);
 
 		list_del(p);
@@ -3376,14 +3376,14 @@ static void hci_blacklist_clear(struct hci_dev *hdev)
 	}
 }
 
-int hci_blacklist_add(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
+int hci_bdaddr_list_add(struct list_head *list, bdaddr_t *bdaddr, u8 type)
 {
 	struct bdaddr_list *entry;
 
 	if (!bacmp(bdaddr, BDADDR_ANY))
 		return -EBADF;
 
-	if (hci_blacklist_lookup(hdev, bdaddr, type))
+	if (hci_bdaddr_list_lookup(list, bdaddr, type))
 		return -EEXIST;
 
 	entry = kzalloc(sizeof(struct bdaddr_list), GFP_KERNEL);
@@ -3393,82 +3393,21 @@ int hci_blacklist_add(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
 	bacpy(&entry->bdaddr, bdaddr);
 	entry->bdaddr_type = type;
 
-	list_add(&entry->list, &hdev->blacklist);
+	list_add(&entry->list, list);
 
 	return 0;
 }
 
-int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
+int hci_bdaddr_list_del(struct list_head *list, bdaddr_t *bdaddr, u8 type)
 {
 	struct bdaddr_list *entry;
 
 	if (!bacmp(bdaddr, BDADDR_ANY)) {
-		hci_blacklist_clear(hdev);
+		hci_bdaddr_list_clear(list);
 		return 0;
 	}
 
-	entry = hci_blacklist_lookup(hdev, bdaddr, type);
-	if (!entry)
-		return -ENOENT;
-
-	list_del(&entry->list);
-	kfree(entry);
-
-	return 0;
-}
-
-struct bdaddr_list *hci_white_list_lookup(struct hci_dev *hdev,
-					  bdaddr_t *bdaddr, u8 type)
-{
-	struct bdaddr_list *b;
-
-	list_for_each_entry(b, &hdev->le_white_list, list) {
-		if (!bacmp(&b->bdaddr, bdaddr) && b->bdaddr_type == type)
-			return b;
-	}
-
-	return NULL;
-}
-
-void hci_white_list_clear(struct hci_dev *hdev)
-{
-	struct list_head *p, *n;
-
-	list_for_each_safe(p, n, &hdev->le_white_list) {
-		struct bdaddr_list *b = list_entry(p, struct bdaddr_list, list);
-
-		list_del(p);
-		kfree(b);
-	}
-}
-
-int hci_white_list_add(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
-{
-	struct bdaddr_list *entry;
-
-	if (!bacmp(bdaddr, BDADDR_ANY))
-		return -EBADF;
-
-	entry = kzalloc(sizeof(struct bdaddr_list), GFP_KERNEL);
-	if (!entry)
-		return -ENOMEM;
-
-	bacpy(&entry->bdaddr, bdaddr);
-	entry->bdaddr_type = type;
-
-	list_add(&entry->list, &hdev->le_white_list);
-
-	return 0;
-}
-
-int hci_white_list_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
-{
-	struct bdaddr_list *entry;
-
-	if (!bacmp(bdaddr, BDADDR_ANY))
-		return -EBADF;
-
-	entry = hci_white_list_lookup(hdev, bdaddr, type);
+	entry = hci_bdaddr_list_lookup(list, bdaddr, type);
 	if (!entry)
 		return -ENOENT;
 
@@ -4096,13 +4035,13 @@ void hci_unregister_dev(struct hci_dev *hdev)
 	destroy_workqueue(hdev->req_workqueue);
 
 	hci_dev_lock(hdev);
-	hci_blacklist_clear(hdev);
+	hci_bdaddr_list_clear(&hdev->blacklist);
 	hci_uuids_clear(hdev);
 	hci_link_keys_clear(hdev);
 	hci_smp_ltks_clear(hdev);
 	hci_smp_irks_clear(hdev);
 	hci_remote_oob_data_clear(hdev);
-	hci_white_list_clear(hdev);
+	hci_bdaddr_list_clear(&hdev->le_white_list);
 	hci_conn_params_clear_all(hdev);
 	hci_dev_unlock(hdev);
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index f6997901bdd7..381c631423f1 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1222,7 +1222,7 @@ static void hci_cc_le_clear_white_list(struct hci_dev *hdev,
 	if (status)
 		return;
 
-	hci_white_list_clear(hdev);
+	hci_bdaddr_list_clear(&hdev->le_white_list);
 }
 
 static void hci_cc_le_add_to_white_list(struct hci_dev *hdev,
@@ -1240,7 +1240,8 @@ static void hci_cc_le_add_to_white_list(struct hci_dev *hdev,
 	if (!sent)
 		return;
 
-	hci_white_list_add(hdev, &sent->bdaddr, sent->bdaddr_type);
+	hci_bdaddr_list_add(&hdev->le_white_list, &sent->bdaddr,
+			   sent->bdaddr_type);
 }
 
 static void hci_cc_le_del_from_white_list(struct hci_dev *hdev,
@@ -1258,7 +1259,8 @@ static void hci_cc_le_del_from_white_list(struct hci_dev *hdev,
 	if (!sent)
 		return;
 
-	hci_white_list_del(hdev, &sent->bdaddr, sent->bdaddr_type);
+	hci_bdaddr_list_del(&hdev->le_white_list, &sent->bdaddr,
+			    sent->bdaddr_type);
 }
 
 static void hci_cc_le_read_supported_states(struct hci_dev *hdev,
@@ -2132,7 +2134,8 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 				      &flags);
 
 	if ((mask & HCI_LM_ACCEPT) &&
-	    !hci_blacklist_lookup(hdev, &ev->bdaddr, BDADDR_BREDR)) {
+	    !hci_bdaddr_list_lookup(&hdev->blacklist, &ev->bdaddr,
+				    BDADDR_BREDR)) {
 		/* Connection accepted */
 		struct inquiry_entry *ie;
 		struct hci_conn *conn;
@@ -4184,7 +4187,7 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		addr_type = BDADDR_LE_RANDOM;
 
 	/* Drop the connection if he device is blocked */
-	if (hci_blacklist_lookup(hdev, &conn->dst, addr_type)) {
+	if (hci_bdaddr_list_lookup(&hdev->blacklist, &conn->dst, addr_type)) {
 		hci_conn_drop(conn);
 		goto unlock;
 	}
@@ -4253,7 +4256,7 @@ static void check_pending_le_conn(struct hci_dev *hdev, bdaddr_t *addr,
 		return;
 
 	/* Ignore if the device is blocked */
-	if (hci_blacklist_lookup(hdev, addr, addr_type))
+	if (hci_bdaddr_list_lookup(&hdev->blacklist, addr, addr_type))
 		return;
 
 	/* If we're connectable, always connect any ADV_DIRECT_IND event */
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 802665751cc4..c64728d571ae 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -481,7 +481,7 @@ static int hci_sock_blacklist_add(struct hci_dev *hdev, void __user *arg)
 
 	hci_dev_lock(hdev);
 
-	err = hci_blacklist_add(hdev, &bdaddr, BDADDR_BREDR);
+	err = hci_bdaddr_list_add(&hdev->blacklist, &bdaddr, BDADDR_BREDR);
 
 	hci_dev_unlock(hdev);
 
@@ -498,7 +498,7 @@ static int hci_sock_blacklist_del(struct hci_dev *hdev, void __user *arg)
 
 	hci_dev_lock(hdev);
 
-	err = hci_blacklist_del(hdev, &bdaddr, BDADDR_BREDR);
+	err = hci_bdaddr_list_del(&hdev->blacklist, &bdaddr, BDADDR_BREDR);
 
 	hci_dev_unlock(hdev);
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index bf379a379fa0..d006e6c0e3b4 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1460,6 +1460,7 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
 static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 {
 	struct hci_conn *hcon = conn->hcon;
+	struct hci_dev *hdev = hcon->hdev;
 	struct l2cap_chan *chan, *pchan;
 	u8 dst_type;
 
@@ -1478,7 +1479,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 	dst_type = bdaddr_type(hcon, hcon->dst_type);
 
 	/* If device is blocked, do not create a channel for it */
-	if (hci_blacklist_lookup(hcon->hdev, &hcon->dst, dst_type))
+	if (hci_bdaddr_list_lookup(&hdev->blacklist, &hcon->dst, dst_type))
 		return;
 
 	/* For LE slave connections, make sure the connection interval
@@ -6918,8 +6919,8 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
 	 * at least ensure that we ignore incoming data from them.
 	 */
 	if (hcon->type == LE_LINK &&
-	    hci_blacklist_lookup(hcon->hdev, &hcon->dst,
-				 bdaddr_type(hcon, hcon->dst_type))) {
+	    hci_bdaddr_list_lookup(&hcon->hdev->blacklist, &hcon->dst,
+				   bdaddr_type(hcon, hcon->dst_type))) {
 		kfree_skb(skb);
 		return;
 	}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 216aa93514b6..592e73eea76d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3956,7 +3956,8 @@ static int block_device(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_dev_lock(hdev);
 
-	err = hci_blacklist_add(hdev, &cp->addr.bdaddr, cp->addr.type);
+	err = hci_bdaddr_list_add(&hdev->blacklist, &cp->addr.bdaddr,
+				  cp->addr.type);
 	if (err < 0) {
 		status = MGMT_STATUS_FAILED;
 		goto done;
@@ -3991,7 +3992,8 @@ static int unblock_device(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_dev_lock(hdev);
 
-	err = hci_blacklist_del(hdev, &cp->addr.bdaddr, cp->addr.type);
+	err = hci_bdaddr_list_del(&hdev->blacklist, &cp->addr.bdaddr,
+				  cp->addr.type);
 	if (err < 0) {
 		status = MGMT_STATUS_INVALID_PARAMS;
 		goto done;
-- 
1.9.3

[PATCH v2 2/7] Bluetooth: Introduce a whitelist for BR/EDR devices

[johan.hedberg]

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

This patch extends the Add/Remove device commands by letting user space
pass BR/EDR addresses to them. The resulting entries get stored in a new
hdev->whitelist list. The idea is that we can now selectively accept
connections from devices in the list even though HCI_CONNECTABLE is not
set (the actual implementation of this is coming in a subsequent patch).

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 3a1caf10cc8d..cba4837dcaa5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -305,6 +305,7 @@ struct hci_dev {
 
 	struct list_head	mgmt_pending;
 	struct list_head	blacklist;
+	struct list_head	whitelist;
 	struct list_head	uuids;
 	struct list_head	link_keys;
 	struct list_head	long_term_keys;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 705f8df7af96..728a6ee471ea 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -191,6 +191,31 @@ static const struct file_operations blacklist_fops = {
 	.release	= single_release,
 };
 
+static int whitelist_show(struct seq_file *f, void *p)
+{
+	struct hci_dev *hdev = f->private;
+	struct bdaddr_list *b;
+
+	hci_dev_lock(hdev);
+	list_for_each_entry(b, &hdev->whitelist, list)
+		seq_printf(f, "%pMR (type %u)\n", &b->bdaddr, b->bdaddr_type);
+	hci_dev_unlock(hdev);
+
+	return 0;
+}
+
+static int whitelist_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, whitelist_show, inode->i_private);
+}
+
+static const struct file_operations whitelist_fops = {
+	.open		= whitelist_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
 static int uuids_show(struct seq_file *f, void *p)
 {
 	struct hci_dev *hdev = f->private;
@@ -1707,6 +1732,8 @@ static int __hci_init(struct hci_dev *hdev)
 	debugfs_create_u16("hci_revision", 0444, hdev->debugfs, &hdev->hci_rev);
 	debugfs_create_file("blacklist", 0444, hdev->debugfs, hdev,
 			    &blacklist_fops);
+	debugfs_create_file("whitelist", 0444, hdev->debugfs, hdev,
+			    &whitelist_fops);
 	debugfs_create_file("uuids", 0444, hdev->debugfs, hdev, &uuids_fops);
 
 	debugfs_create_file("conn_info_min_age", 0644, hdev->debugfs, hdev,
@@ -3825,6 +3852,7 @@ struct hci_dev *hci_alloc_dev(void)
 
 	INIT_LIST_HEAD(&hdev->mgmt_pending);
 	INIT_LIST_HEAD(&hdev->blacklist);
+	INIT_LIST_HEAD(&hdev->whitelist);
 	INIT_LIST_HEAD(&hdev->uuids);
 	INIT_LIST_HEAD(&hdev->link_keys);
 	INIT_LIST_HEAD(&hdev->long_term_keys);
@@ -4036,6 +4064,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
 
 	hci_dev_lock(hdev);
 	hci_bdaddr_list_clear(&hdev->blacklist);
+	hci_bdaddr_list_clear(&hdev->whitelist);
 	hci_uuids_clear(hdev);
 	hci_link_keys_clear(hdev);
 	hci_smp_ltks_clear(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 592e73eea76d..431d5e4aa34b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5219,7 +5219,7 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
 
 	BT_DBG("%s", hdev->name);
 
-	if (!bdaddr_type_is_le(cp->addr.type) ||
+	if (!bdaddr_type_is_valid(cp->addr.type) ||
 	    !bacmp(&cp->addr.bdaddr, BDADDR_ANY))
 		return cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
 				    MGMT_STATUS_INVALID_PARAMS,
@@ -5230,8 +5230,26 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
 				    MGMT_STATUS_INVALID_PARAMS,
 				    &cp->addr, sizeof(cp->addr));
 
+	if (cp->action != 0x00 && cp->action != 0x01)
+
 	hci_dev_lock(hdev);
 
+	if (cp->addr.type == BDADDR_BREDR) {
+		/* Only "connect" action supported for now */
+		if (cp->action != 0x01) {
+			err = cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
+					   MGMT_STATUS_INVALID_PARAMS,
+					   &cp->addr, sizeof(cp->addr));
+			goto unlock;
+		}
+
+		err = hci_bdaddr_list_add(&hdev->whitelist, &cp->addr.bdaddr,
+					  cp->addr.type);
+		if (err)
+			goto unlock;
+		goto added;
+	}
+
 	if (cp->addr.type == BDADDR_LE_PUBLIC)
 		addr_type = ADDR_LE_DEV_PUBLIC;
 	else
@@ -5253,6 +5271,7 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
 		goto unlock;
 	}
 
+added:
 	device_added(sk, hdev, &cp->addr.bdaddr, cp->addr.type, cp->action);
 
 	err = cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
@@ -5288,13 +5307,30 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
 		struct hci_conn_params *params;
 		u8 addr_type;
 
-		if (!bdaddr_type_is_le(cp->addr.type)) {
+		if (!bdaddr_type_is_valid(cp->addr.type)) {
 			err = cmd_complete(sk, hdev->id, MGMT_OP_REMOVE_DEVICE,
 					   MGMT_STATUS_INVALID_PARAMS,
 					   &cp->addr, sizeof(cp->addr));
 			goto unlock;
 		}
 
+		if (cp->addr.type == BDADDR_BREDR) {
+			err = hci_bdaddr_list_del(&hdev->whitelist,
+						  &cp->addr.bdaddr,
+						  cp->addr.type);
+			if (err) {
+				err = cmd_complete(sk, hdev->id,
+						   MGMT_OP_REMOVE_DEVICE,
+						   MGMT_STATUS_INVALID_PARAMS,
+						   &cp->addr, sizeof(cp->addr));
+				goto unlock;
+			}
+
+			device_removed(sk, hdev, &cp->addr.bdaddr,
+				       cp->addr.type);
+			goto complete;
+		}
+
 		if (cp->addr.type == BDADDR_LE_PUBLIC)
 			addr_type = ADDR_LE_DEV_PUBLIC;
 		else
@@ -5324,6 +5360,7 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
 		device_removed(sk, hdev, &cp->addr.bdaddr, cp->addr.type);
 	} else {
 		struct hci_conn_params *p, *tmp;
+		struct bdaddr_list *b, *btmp;
 
 		if (cp->addr.type) {
 			err = cmd_complete(sk, hdev->id, MGMT_OP_REMOVE_DEVICE,
@@ -5332,6 +5369,12 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
 			goto unlock;
 		}
 
+		list_for_each_entry_safe(b, btmp, &hdev->whitelist, list) {
+			device_removed(sk, hdev, &b->bdaddr, b->bdaddr_type);
+			list_del(&b->list);
+			kfree(b);
+		}
+
 		list_for_each_entry_safe(p, tmp, &hdev->le_conn_params, list) {
 			if (p->auto_connect == HCI_AUTO_CONN_DISABLED)
 				continue;
@@ -5346,6 +5389,7 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
 		hci_update_background_scan(hdev);
 	}
 
+complete:
 	err = cmd_complete(sk, hdev->id, MGMT_OP_REMOVE_DEVICE,
 			   MGMT_STATUS_SUCCESS, &cp->addr, sizeof(cp->addr));
 
-- 
1.9.3

[PATCH v2 3/7] Bluetooth: Update page scan when necessary for Add/Remove Device

[johan.hedberg]

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

When we're removing the last item in the white list or adding the first
one to it and HCI_CONNECTABLE is not set we need to update the current
page scan. This patch adds a simple helper function for the purpose and
calls it from the respective mgmt command handlers.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 431d5e4aa34b..f5f0763266e0 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5198,6 +5198,24 @@ unlock:
 	return err;
 }
 
+/* Helper for Add/Remove Device commands */
+static void update_page_scan(struct hci_dev *hdev, u8 scan)
+{
+	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
+		return;
+
+	if (!hdev_is_powered(hdev))
+		return;
+
+	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
+		return;
+
+	if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
+		scan |= SCAN_INQUIRY;
+
+	hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+}
+
 static void device_added(struct sock *sk, struct hci_dev *hdev,
 			 bdaddr_t *bdaddr, u8 type, u8 action)
 {
@@ -5235,6 +5253,8 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
 	hci_dev_lock(hdev);
 
 	if (cp->addr.type == BDADDR_BREDR) {
+		bool update_scan;
+
 		/* Only "connect" action supported for now */
 		if (cp->action != 0x01) {
 			err = cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
@@ -5243,10 +5263,16 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
 			goto unlock;
 		}
 
+		update_scan = list_empty(&hdev->whitelist);
+
 		err = hci_bdaddr_list_add(&hdev->whitelist, &cp->addr.bdaddr,
 					  cp->addr.type);
 		if (err)
 			goto unlock;
+
+		if (update_scan)
+			update_page_scan(hdev, SCAN_PAGE);
+
 		goto added;
 	}
 
@@ -5326,6 +5352,9 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
 				goto unlock;
 			}
 
+			if (list_empty(&hdev->whitelist))
+				update_page_scan(hdev, SCAN_DISABLED);
+
 			device_removed(sk, hdev, &cp->addr.bdaddr,
 				       cp->addr.type);
 			goto complete;
@@ -5375,6 +5404,8 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
 			kfree(b);
 		}
 
+		update_page_scan(hdev, SCAN_DISABLED);
+
 		list_for_each_entry_safe(p, tmp, &hdev->le_conn_params, list) {
 			if (p->auto_connect == HCI_AUTO_CONN_DISABLED)
 				continue;
-- 
1.9.3

[PATCH v2 4/7] Bluetooth: Fix incorrectly setting HCI_CONNECTABLE

[johan.hedberg]

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

Since page scan might be enabled by Add Device we should not implicitly
set connectable whenever something else than Set Connectable changes it.
This patch makes sure that we don't set HCI_CONNECTABLE for these cases
if there are any entries in the white list.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f5f0763266e0..83435a08b0c0 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6075,6 +6075,14 @@ void mgmt_connectable(struct hci_dev *hdev, u8 connectable)
 	if (!connectable && mgmt_pending_find(MGMT_OP_SET_POWERED, hdev))
 		return;
 
+	/* If something else than mgmt changed the page scan state we
+	 * can't differentiate this from a change triggered by adding
+	 * the first element to the whitelist. Therefore, avoid
+	 * incorrectly setting HCI_CONNECTABLE.
+	 */
+	if (connectable && !list_empty(&hdev->whitelist))
+		return;
+
 	if (connectable)
 		changed = !test_and_set_bit(HCI_CONNECTABLE, &hdev->dev_flags);
 	else
-- 
1.9.3

[PATCH v2 5/7] Bluetooth: Refactor connection request handling

[johan.hedberg]

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

The conditions for accepting an incoming connections are already
non-trivial and will become more so once a white list is added. This
patch breaks up the checks for when to reject the request by creating a
helper function for it.

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

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 381c631423f1..6d1d5b3e9cd4 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2121,10 +2121,21 @@ unlock:
 	hci_conn_check_pending(hdev);
 }
 
+static void hci_reject_conn(struct hci_dev *hdev, bdaddr_t *bdaddr)
+{
+	struct hci_cp_reject_conn_req cp;
+
+	bacpy(&cp.bdaddr, bdaddr);
+	cp.reason = HCI_ERROR_REJ_BAD_ADDR;
+	hci_send_cmd(hdev, HCI_OP_REJECT_CONN_REQ, sizeof(cp), &cp);
+}
+
 static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_conn_request *ev = (void *) skb->data;
 	int mask = hdev->link_mode;
+	struct inquiry_entry *ie;
+	struct hci_conn *conn;
 	__u8 flags = 0;
 
 	BT_DBG("%s bdaddr %pMR type 0x%x", hdev->name, &ev->bdaddr,
@@ -2133,74 +2144,71 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type,
 				      &flags);
 
-	if ((mask & HCI_LM_ACCEPT) &&
-	    !hci_bdaddr_list_lookup(&hdev->blacklist, &ev->bdaddr,
+	if (!(mask & HCI_LM_ACCEPT)) {
+		hci_reject_conn(hdev, &ev->bdaddr);
+		return;
+	}
+
+	if (!hci_bdaddr_list_lookup(&hdev->blacklist, &ev->bdaddr,
 				    BDADDR_BREDR)) {
-		/* Connection accepted */
-		struct inquiry_entry *ie;
-		struct hci_conn *conn;
+		hci_reject_conn(hdev, &ev->bdaddr);
+		return;
+	}
 
-		hci_dev_lock(hdev);
+	/* Connection accepted */
 
-		ie = hci_inquiry_cache_lookup(hdev, &ev->bdaddr);
-		if (ie)
-			memcpy(ie->data.dev_class, ev->dev_class, 3);
+	hci_dev_lock(hdev);
 
-		conn = hci_conn_hash_lookup_ba(hdev, ev->link_type,
-					       &ev->bdaddr);
+	ie = hci_inquiry_cache_lookup(hdev, &ev->bdaddr);
+	if (ie)
+		memcpy(ie->data.dev_class, ev->dev_class, 3);
+
+	conn = hci_conn_hash_lookup_ba(hdev, ev->link_type,
+			&ev->bdaddr);
+	if (!conn) {
+		conn = hci_conn_add(hdev, ev->link_type, &ev->bdaddr);
 		if (!conn) {
-			conn = hci_conn_add(hdev, ev->link_type, &ev->bdaddr);
-			if (!conn) {
-				BT_ERR("No memory for new connection");
-				hci_dev_unlock(hdev);
-				return;
-			}
+			BT_ERR("No memory for new connection");
+			hci_dev_unlock(hdev);
+			return;
 		}
+	}
 
-		memcpy(conn->dev_class, ev->dev_class, 3);
+	memcpy(conn->dev_class, ev->dev_class, 3);
 
-		hci_dev_unlock(hdev);
+	hci_dev_unlock(hdev);
 
-		if (ev->link_type == ACL_LINK ||
-		    (!(flags & HCI_PROTO_DEFER) && !lmp_esco_capable(hdev))) {
-			struct hci_cp_accept_conn_req cp;
-			conn->state = BT_CONNECT;
+	if (ev->link_type == ACL_LINK ||
+	    (!(flags & HCI_PROTO_DEFER) && !lmp_esco_capable(hdev))) {
+		struct hci_cp_accept_conn_req cp;
+		conn->state = BT_CONNECT;
 
-			bacpy(&cp.bdaddr, &ev->bdaddr);
+		bacpy(&cp.bdaddr, &ev->bdaddr);
 
-			if (lmp_rswitch_capable(hdev) && (mask & HCI_LM_MASTER))
-				cp.role = 0x00; /* Become master */
-			else
-				cp.role = 0x01; /* Remain slave */
+		if (lmp_rswitch_capable(hdev) && (mask & HCI_LM_MASTER))
+			cp.role = 0x00; /* Become master */
+		else
+			cp.role = 0x01; /* Remain slave */
 
-			hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp),
-				     &cp);
-		} else if (!(flags & HCI_PROTO_DEFER)) {
-			struct hci_cp_accept_sync_conn_req cp;
-			conn->state = BT_CONNECT;
+		hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp), &cp);
+	} else if (!(flags & HCI_PROTO_DEFER)) {
+		struct hci_cp_accept_sync_conn_req cp;
+		conn->state = BT_CONNECT;
 
-			bacpy(&cp.bdaddr, &ev->bdaddr);
-			cp.pkt_type = cpu_to_le16(conn->pkt_type);
+		bacpy(&cp.bdaddr, &ev->bdaddr);
+		cp.pkt_type = cpu_to_le16(conn->pkt_type);
 
-			cp.tx_bandwidth   = cpu_to_le32(0x00001f40);
-			cp.rx_bandwidth   = cpu_to_le32(0x00001f40);
-			cp.max_latency    = cpu_to_le16(0xffff);
-			cp.content_format = cpu_to_le16(hdev->voice_setting);
-			cp.retrans_effort = 0xff;
+		cp.tx_bandwidth   = cpu_to_le32(0x00001f40);
+		cp.rx_bandwidth   = cpu_to_le32(0x00001f40);
+		cp.max_latency    = cpu_to_le16(0xffff);
+		cp.content_format = cpu_to_le16(hdev->voice_setting);
+		cp.retrans_effort = 0xff;
 
-			hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
-				     sizeof(cp), &cp);
-		} else {
-			conn->state = BT_CONNECT2;
-			hci_proto_connect_cfm(conn, 0);
-		}
+		hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ, sizeof(cp),
+			     &cp);
 	} else {
-		/* Connection rejected */
-		struct hci_cp_reject_conn_req cp;
-
-		bacpy(&cp.bdaddr, &ev->bdaddr);
-		cp.reason = HCI_ERROR_REJ_BAD_ADDR;
-		hci_send_cmd(hdev, HCI_OP_REJECT_CONN_REQ, sizeof(cp), &cp);
+		conn->state = BT_CONNECT2;
+		hci_proto_connect_cfm(conn, 0);
 	}
 }
 
-- 
1.9.3

[PATCH v2 6/7] Bluetooth: Enable page scan also if there are white list entries

[johan.hedberg]

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

Page scan should be enabled either if the connectable setting is set or
if there are any entries in the BR/EDR white list. This patch implements
such behavior by updating the two places that were making decisions on
whether to enable page scan or not.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 83435a08b0c0..cac663f89cd0 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4357,7 +4357,8 @@ static void set_bredr_scan(struct hci_request *req)
 	 */
 	write_fast_connectable(req, false);
 
-	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
+	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags) ||
+	    !list_empty(&hdev->whitelist))
 		scan |= SCAN_PAGE;
 	if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
 		scan |= SCAN_INQUIRY;
@@ -4471,7 +4472,8 @@ static int set_bredr(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 
 	hci_req_init(&req, hdev);
 
-	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
+	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags) ||
+	    !list_empty(&hdev->whitelist))
 		set_bredr_scan(&req);
 
 	/* Since only the advertising data flags will change, there
-- 
1.9.3

[PATCH v2 7/7] Bluetooth: Add white list lookup for incoming connection requests

[johan.hedberg]

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

This patch adds support for looking up entries in the white list when
HCI_CONNECTABLE is not set. The logic is fairly simple: if we're
connectable check the black list, if we're not connectable check the
white list.

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

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 6d1d5b3e9cd4..c8ae9ee3cb12 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2149,10 +2149,18 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		return;
 	}
 
-	if (!hci_bdaddr_list_lookup(&hdev->blacklist, &ev->bdaddr,
-				    BDADDR_BREDR)) {
-		hci_reject_conn(hdev, &ev->bdaddr);
-		return;
+	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags)) {
+		if (hci_bdaddr_list_lookup(&hdev->blacklist, &ev->bdaddr,
+					   BDADDR_BREDR)) {
+			hci_reject_conn(hdev, &ev->bdaddr);
+			return;
+		}
+	} else {
+		if (!hci_bdaddr_list_lookup(&hdev->whitelist, &ev->bdaddr,
+					    BDADDR_BREDR)) {
+			hci_reject_conn(hdev, &ev->bdaddr);
+			return;
+		}
 	}
 
 	/* Connection accepted */
-- 
1.9.3

Re: [PATCH v2 2/7] Bluetooth: Introduce a whitelist for BR/EDR devices

[Marcel Holtmann]

Hi Johan,

> This patch extends the Add/Remove device commands by letting user space
> pass BR/EDR addresses to them. The resulting entries get stored in a new
> hdev->whitelist list. The idea is that we can now selectively accept
> connections from devices in the list even though HCI_CONNECTABLE is not
> set (the actual implementation of this is coming in a subsequent patch).
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h |  1 +
> net/bluetooth/hci_core.c         | 29 ++++++++++++++++++++++++
> net/bluetooth/mgmt.c             | 48 ++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 3a1caf10cc8d..cba4837dcaa5 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -305,6 +305,7 @@ struct hci_dev {
> 
> 	struct list_head	mgmt_pending;
> 	struct list_head	blacklist;
> +	struct list_head	whitelist;
> 	struct list_head	uuids;
> 	struct list_head	link_keys;
> 	struct list_head	long_term_keys;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 705f8df7af96..728a6ee471ea 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -191,6 +191,31 @@ static const struct file_operations blacklist_fops = {
> 	.release	= single_release,
> };
> 
> +static int whitelist_show(struct seq_file *f, void *p)
> +{
> +	struct hci_dev *hdev = f->private;
> +	struct bdaddr_list *b;
> +
> +	hci_dev_lock(hdev);
> +	list_for_each_entry(b, &hdev->whitelist, list)
> +		seq_printf(f, "%pMR (type %u)\n", &b->bdaddr, b->bdaddr_type);
> +	hci_dev_unlock(hdev);
> +
> +	return 0;
> +}
> +
> +static int whitelist_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, whitelist_show, inode->i_private);
> +}
> +
> +static const struct file_operations whitelist_fops = {
> +	.open		= whitelist_open,
> +	.read		= seq_read,
> +	.llseek		= seq_lseek,
> +	.release	= single_release,
> +};
> +
> static int uuids_show(struct seq_file *f, void *p)
> {
> 	struct hci_dev *hdev = f->private;
> @@ -1707,6 +1732,8 @@ static int __hci_init(struct hci_dev *hdev)
> 	debugfs_create_u16("hci_revision", 0444, hdev->debugfs, &hdev->hci_rev);
> 	debugfs_create_file("blacklist", 0444, hdev->debugfs, hdev,
> 			    &blacklist_fops);
> +	debugfs_create_file("whitelist", 0444, hdev->debugfs, hdev,
> +			    &whitelist_fops);
> 	debugfs_create_file("uuids", 0444, hdev->debugfs, hdev, &uuids_fops);
> 
> 	debugfs_create_file("conn_info_min_age", 0644, hdev->debugfs, hdev,
> @@ -3825,6 +3852,7 @@ struct hci_dev *hci_alloc_dev(void)
> 
> 	INIT_LIST_HEAD(&hdev->mgmt_pending);
> 	INIT_LIST_HEAD(&hdev->blacklist);
> +	INIT_LIST_HEAD(&hdev->whitelist);
> 	INIT_LIST_HEAD(&hdev->uuids);
> 	INIT_LIST_HEAD(&hdev->link_keys);
> 	INIT_LIST_HEAD(&hdev->long_term_keys);
> @@ -4036,6 +4064,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
> 
> 	hci_dev_lock(hdev);
> 	hci_bdaddr_list_clear(&hdev->blacklist);
> +	hci_bdaddr_list_clear(&hdev->whitelist);
> 	hci_uuids_clear(hdev);
> 	hci_link_keys_clear(hdev);
> 	hci_smp_ltks_clear(hdev);
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 592e73eea76d..431d5e4aa34b 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -5219,7 +5219,7 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
> 
> 	BT_DBG("%s", hdev->name);
> 
> -	if (!bdaddr_type_is_le(cp->addr.type) ||
> +	if (!bdaddr_type_is_valid(cp->addr.type) ||
> 	    !bacmp(&cp->addr.bdaddr, BDADDR_ANY))
> 		return cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
> 				    MGMT_STATUS_INVALID_PARAMS,
> @@ -5230,8 +5230,26 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
> 				    MGMT_STATUS_INVALID_PARAMS,
> 				    &cp->addr, sizeof(cp->addr));
> 
> +	if (cp->action != 0x00 && cp->action != 0x01)
> +

something is wrong here. Shouldn't this return invalid params?

> 	hci_dev_lock(hdev);
> 
> +	if (cp->addr.type == BDADDR_BREDR) {
> +		/* Only "connect" action supported for now */
> +		if (cp->action != 0x01) {
> +			err = cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
> +					   MGMT_STATUS_INVALID_PARAMS,
> +					   &cp->addr, sizeof(cp->addr));
> +			goto unlock;
> +		}
> +
> +		err = hci_bdaddr_list_add(&hdev->whitelist, &cp->addr.bdaddr,
> +					  cp->addr.type);
> +		if (err)
> +			goto unlock;
> +		goto added;
> +	}
> +

Regards

Marcel

Re: [PATCH v2 3/7] Bluetooth: Update page scan when necessary for Add/Remove Device

[Marcel Holtmann]

Hi Johan,

> When we're removing the last item in the white list or adding the first
> one to it and HCI_CONNECTABLE is not set we need to update the current
> page scan. This patch adds a simple helper function for the purpose and
> calls it from the respective mgmt command handlers.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 431d5e4aa34b..f5f0763266e0 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -5198,6 +5198,24 @@ unlock:
> 	return err;
> }
> 
> +/* Helper for Add/Remove Device commands */
> +static void update_page_scan(struct hci_dev *hdev, u8 scan)
> +{
> +	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
> +		return;
> +
> +	if (!hdev_is_powered(hdev))
> +		return;
> +
> +	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
> +		return;

wouldn't it be better to check the HCI_PSCAN flag here to see if page scan is actually enabled on the controller?

> +
> +	if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
> +		scan |= SCAN_INQUIRY;
> +
> +	hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
> +}
> +

Regards

Marcel

Re: [PATCH v2 2/7] Bluetooth: Introduce a whitelist for BR/EDR devices

[Johan Hedberg]

Hi Marcel,

On Wed, Jul 09, 2014, Marcel Holtmann wrote:
> > @@ -5230,8 +5230,26 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
> > 				    MGMT_STATUS_INVALID_PARAMS,
> > 				    &cp->addr, sizeof(cp->addr));
> > 
> > +	if (cp->action != 0x00 && cp->action != 0x01)
> > +
> 
> something is wrong here. Shouldn't this return invalid params?

Yes, very good catch. Seems I did cut&paste instead of copy&paste and
neither I nor the compiler noticed it.

Johan

Re: [PATCH v2 3/7] Bluetooth: Update page scan when necessary for Add/Remove Device

[Johan Hedberg]

Hi Marcel,

On Wed, Jul 09, 2014, Marcel Holtmann wrote:
> > When we're removing the last item in the white list or adding the first
> > one to it and HCI_CONNECTABLE is not set we need to update the current
> > page scan. This patch adds a simple helper function for the purpose and
> > calls it from the respective mgmt command handlers.
> > 
> > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> > ---
> > net/bluetooth/mgmt.c | 31 +++++++++++++++++++++++++++++++
> > 1 file changed, 31 insertions(+)
> > 
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 431d5e4aa34b..f5f0763266e0 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -5198,6 +5198,24 @@ unlock:
> > 	return err;
> > }
> > 
> > +/* Helper for Add/Remove Device commands */
> > +static void update_page_scan(struct hci_dev *hdev, u8 scan)
> > +{
> > +	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
> > +		return;
> > +
> > +	if (!hdev_is_powered(hdev))
> > +		return;
> > +
> > +	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
> > +		return;
> 
> wouldn't it be better to check the HCI_PSCAN flag here to see if page
> scan is actually enabled on the controller?

Maybe I was a bit too smart here, but the code is actually correct. We
also call update_page_scan() with SCAN_DISABLE (0x00) for the
remove_device case and there if HCI_CONNECTABLE is set we should return
from the function. Checking for HCI_PSCAN would do the wrong thing.

I can leave the patch as-is, or add a bit more complex logic there:

	if (PSCAN && scan)
		return;

	if (!PSCAN && !scan)
		return;

Let me know what you prefer.

Johan

Re: [PATCH v2 3/7] Bluetooth: Update page scan when necessary for Add/Remove Device

[Marcel Holtmann]

Hi Johan,

>>> When we're removing the last item in the white list or adding the first
>>> one to it and HCI_CONNECTABLE is not set we need to update the current
>>> page scan. This patch adds a simple helper function for the purpose and
>>> calls it from the respective mgmt command handlers.
>>> 
>>> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
>>> ---
>>> net/bluetooth/mgmt.c | 31 +++++++++++++++++++++++++++++++
>>> 1 file changed, 31 insertions(+)
>>> 
>>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>>> index 431d5e4aa34b..f5f0763266e0 100644
>>> --- a/net/bluetooth/mgmt.c
>>> +++ b/net/bluetooth/mgmt.c
>>> @@ -5198,6 +5198,24 @@ unlock:
>>> 	return err;
>>> }
>>> 
>>> +/* Helper for Add/Remove Device commands */
>>> +static void update_page_scan(struct hci_dev *hdev, u8 scan)
>>> +{
>>> +	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
>>> +		return;
>>> +
>>> +	if (!hdev_is_powered(hdev))
>>> +		return;
>>> +
>>> +	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
>>> +		return;
>> 
>> wouldn't it be better to check the HCI_PSCAN flag here to see if page
>> scan is actually enabled on the controller?
> 
> Maybe I was a bit too smart here, but the code is actually correct. We
> also call update_page_scan() with SCAN_DISABLE (0x00) for the
> remove_device case and there if HCI_CONNECTABLE is set we should return
> from the function. Checking for HCI_PSCAN would do the wrong thing.
> 
> I can leave the patch as-is, or add a bit more complex logic there:
> 
> 	if (PSCAN && scan)
> 		return;
> 
> 	if (!PSCAN && !scan)
> 		return;
> 
> Let me know what you prefer.

maybe just add a comment explaining why checking HCI_CONNECTABLE is the right thing to do here.

Regards

Marcel