[PATCH 1/7] Bluetooth: Add automated SSP user confirmation responses

[PATCH 1/7] Bluetooth: Add automated SSP user confirmation responses

[johan.hedberg]

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

This patch adds automated negative and positive (auto-accept) responses
for Secure Simple Pairing user confirmation requests. The responses are
only sent if the HCI_MGMT flag is set in order not to confuse older user
space versions (without management interface support).

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
 net/bluetooth/hci_event.c |   37 +++++++++++++++++++++++++++++++++++--
 1 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3c90736..038d70d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2447,14 +2447,47 @@ static inline void hci_user_confirm_request_evt(struct hci_dev *hdev,
 							struct sk_buff *skb)
 {
 	struct hci_ev_user_confirm_req *ev = (void *) skb->data;
+	int loc_mitm, rem_mitm;
+	struct hci_conn *conn;
 
 	BT_DBG("%s", hdev->name);
 
 	hci_dev_lock(hdev);
 
-	if (test_bit(HCI_MGMT, &hdev->flags))
-		mgmt_user_confirm_request(hdev->id, &ev->bdaddr, ev->passkey);
+	if (!test_bit(HCI_MGMT, &hdev->flags))
+		goto unlock;
 
+	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
+	if (!conn)
+		goto unlock;
+
+	loc_mitm = (conn->auth_type & 0x01);
+	rem_mitm = (conn->remote_auth & 0x01);
+
+	/* If we require MITM but the remote device can't provide that
+	 * (it has NoInputNoOutput) then reject the confirmation
+	 * request. The only exception is when we're dedicated bonding
+	 * initiators (connect_cfm_cb set) since then we always have the MITM
+	 * bit set. */
+	if (!conn->connect_cfm_cb && loc_mitm && conn->remote_cap == 0x03) {
+		BT_DBG("Rejecting request: remote device can't provide MITM");
+		hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_NEG_REPLY,
+					sizeof(ev->bdaddr), &ev->bdaddr);
+		goto unlock;
+	}
+
+	/* If no side requires MITM protection; auto-accept */
+	if ((!loc_mitm || conn->remote_cap == 0x03) &&
+				(!rem_mitm || conn->io_capability == 0x03)) {
+		BT_DBG("Auto-accept of user confirmation");
+		hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_REPLY,
+						sizeof(ev->bdaddr), &ev->bdaddr);
+		goto unlock;
+	}
+
+	mgmt_user_confirm_request(hdev->id, &ev->bdaddr, ev->passkey);
+
+unlock:
 	hci_dev_unlock(hdev);
 }
 
-- 
1.7.4.4

[PATCH 2/7] Bluetooth: Add variable SSP auto-accept delay support

[johan.hedberg]

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

Some test systems require an arbitrary delay to the auto-accept test
cases for Secure Simple Pairing in order for the tests to pass.
Previously when this was handled in user space it was worked around by
code modifications and recompilation, but now that it's on the kernel
side it's more convenient if there's a debugfs interface for it.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
 include/net/bluetooth/hci_core.h |    3 +++
 net/bluetooth/hci_conn.c         |   17 +++++++++++++++++
 net/bluetooth/hci_event.c        |   10 +++++++++-
 net/bluetooth/hci_sysfs.c        |   31 +++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 02e7256..09740c3 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -126,6 +126,8 @@ struct hci_dev {
 	__u16		sniff_min_interval;
 	__u16		sniff_max_interval;
 
+	unsigned int	auto_accept_delay;
+
 	unsigned long	quirks;
 
 	atomic_t	cmd_cnt;
@@ -246,6 +248,7 @@ struct hci_conn {
 
 	struct timer_list disc_timer;
 	struct timer_list idle_timer;
+	struct timer_list auto_accept_timer;
 
 	struct work_struct work_add;
 	struct work_struct work_del;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 74cd755..7f5ad8a 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -269,6 +269,19 @@ static void hci_conn_idle(unsigned long arg)
 	hci_conn_enter_sniff_mode(conn);
 }
 
+static void hci_conn_auto_accept(unsigned long arg)
+{
+	struct hci_conn *conn = (void *) arg;
+	struct hci_dev *hdev = conn->hdev;
+
+	hci_dev_lock(hdev);
+
+	hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_REPLY, sizeof(conn->dst),
+								&conn->dst);
+
+	hci_dev_unlock(hdev);
+}
+
 struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
 {
 	struct hci_conn *conn;
@@ -312,6 +325,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
 
 	setup_timer(&conn->disc_timer, hci_conn_timeout, (unsigned long)conn);
 	setup_timer(&conn->idle_timer, hci_conn_idle, (unsigned long)conn);
+	setup_timer(&conn->auto_accept_timer, hci_conn_auto_accept,
+							(unsigned long) conn);
 
 	atomic_set(&conn->refcnt, 0);
 
@@ -342,6 +357,8 @@ int hci_conn_del(struct hci_conn *conn)
 
 	del_timer(&conn->disc_timer);
 
+	del_timer(&conn->auto_accept_timer);
+
 	if (conn->type == ACL_LINK) {
 		struct hci_conn *sco = conn->link;
 		if (sco)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 038d70d..4f1e695 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2479,7 +2479,15 @@ static inline void hci_user_confirm_request_evt(struct hci_dev *hdev,
 	/* If no side requires MITM protection; auto-accept */
 	if ((!loc_mitm || conn->remote_cap == 0x03) &&
 				(!rem_mitm || conn->io_capability == 0x03)) {
-		BT_DBG("Auto-accept of user confirmation");
+		BT_DBG("Auto-accept of user confirmation with %ums delay",
+						hdev->auto_accept_delay);
+
+		if (hdev->auto_accept_delay > 0) {
+			int delay = msecs_to_jiffies(hdev->auto_accept_delay);
+			mod_timer(&conn->auto_accept_timer, jiffies + delay);
+			goto unlock;
+		}
+
 		hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_REPLY,
 						sizeof(ev->bdaddr), &ev->bdaddr);
 		goto unlock;
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 8775933..a6c3aa8 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -511,6 +511,35 @@ static const struct file_operations uuids_fops = {
 	.release	= single_release,
 };
 
+static int auto_accept_delay_set(void *data, u64 val)
+{
+	struct hci_dev *hdev = data;
+
+	hci_dev_lock_bh(hdev);
+
+	hdev->auto_accept_delay = val;
+
+	hci_dev_unlock_bh(hdev);
+
+	return 0;
+}
+
+static int auto_accept_delay_get(void *data, u64 *val)
+{
+	struct hci_dev *hdev = data;
+
+	hci_dev_lock_bh(hdev);
+
+	*val = hdev->auto_accept_delay;
+
+	hci_dev_unlock_bh(hdev);
+
+	return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
+					auto_accept_delay_set, "%llu\n");
+
 int hci_register_sysfs(struct hci_dev *hdev)
 {
 	struct device *dev = &hdev->dev;
@@ -545,6 +574,8 @@ int hci_register_sysfs(struct hci_dev *hdev)
 
 	debugfs_create_file("uuids", 0444, hdev->debugfs, hdev, &uuids_fops);
 
+	debugfs_create_file("auto_accept_delay", 0444, hdev->debugfs, hdev,
+						&auto_accept_delay_fops);
 	return 0;
 }
 
-- 
1.7.4.4

[PATCH 3/7] Bluetooth: Fix HCI_CONN_AUTH_PEND flag for all authentication requests

[johan.hedberg]

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

The HCI_CONN_AUTH_PEND flag should be set whenever requesting
authentication so that multiple pending requests can't occur.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
 net/bluetooth/hci_event.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 4f1e695..12a9bb9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1013,12 +1013,19 @@ static void hci_cs_remote_name_req(struct hci_dev *hdev, __u8 status)
 	hci_dev_lock(hdev);
 
 	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->bdaddr);
-	if (conn && hci_outgoing_auth_needed(hdev, conn)) {
+	if (!conn)
+		goto unlock;
+
+	if (!hci_outgoing_auth_needed(hdev, conn))
+		goto unlock;
+
+	if (!test_and_set_bit(HCI_CONN_AUTH_PEND, &conn->pend)) {
 		struct hci_cp_auth_requested cp;
-		cp.handle = __cpu_to_le16(conn->handle);
+		cp.handle = cpu_to_le16(conn->handle);
 		hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED, sizeof(cp), &cp);
 	}
 
+unlock:
 	hci_dev_unlock(hdev);
 }
 
@@ -1500,12 +1507,19 @@ static inline void hci_remote_name_evt(struct hci_dev *hdev, struct sk_buff *skb
 		mgmt_remote_name(hdev->id, &ev->bdaddr, ev->name);
 
 	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
-	if (conn && hci_outgoing_auth_needed(hdev, conn)) {
+	if (!conn)
+		goto unlock;
+
+	if (!hci_outgoing_auth_needed(hdev, conn))
+		goto unlock;
+
+	if (!test_and_set_bit(HCI_CONN_AUTH_PEND, &conn->pend)) {
 		struct hci_cp_auth_requested cp;
-		cp.handle = __cpu_to_le16(conn->handle);
+		cp.handle = cpu_to_le16(conn->handle);
 		hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED, sizeof(cp), &cp);
 	}
 
+unlock:
 	hci_dev_unlock(hdev);
 }
 
-- 
1.7.4.4

[PATCH 4/7] Bluetooth: Add confirm_hint parameter to user confirmation requests

[johan.hedberg]

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

When accepting a pairing request which fulfills the SSP auto-accept
criteria we need to push the request all the way to the user for
confirmation. This patch adds a new hint to the user_confirm_request
management event so user space can know when to show a numeric
comparison dialog and when to show a simple yes/no confirmation dialog.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
 include/net/bluetooth/hci_core.h |    3 ++-
 include/net/bluetooth/mgmt.h     |    1 +
 net/bluetooth/hci_event.c        |   16 ++++++++++++++--
 net/bluetooth/mgmt.c             |    4 +++-
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 09740c3..490b43b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -783,7 +783,8 @@ int mgmt_connect_failed(u16 index, bdaddr_t *bdaddr, u8 status);
 int mgmt_pin_code_request(u16 index, bdaddr_t *bdaddr);
 int mgmt_pin_code_reply_complete(u16 index, bdaddr_t *bdaddr, u8 status);
 int mgmt_pin_code_neg_reply_complete(u16 index, bdaddr_t *bdaddr, u8 status);
-int mgmt_user_confirm_request(u16 index, bdaddr_t *bdaddr, __le32 value);
+int mgmt_user_confirm_request(u16 index, bdaddr_t *bdaddr, __le32 value,
+							u8 confirm_hint);
 int mgmt_user_confirm_reply_complete(u16 index, bdaddr_t *bdaddr, u8 status);
 int mgmt_user_confirm_neg_reply_complete(u16 index, bdaddr_t *bdaddr,
 								u8 status);
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 6b6ff92..8ea6b08 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -254,6 +254,7 @@ struct mgmt_ev_pin_code_request {
 #define MGMT_EV_USER_CONFIRM_REQUEST	0x000F
 struct mgmt_ev_user_confirm_request {
 	bdaddr_t bdaddr;
+	__u8 confirm_hint;
 	__le32 value;
 } __packed;
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 12a9bb9..003d731 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2461,7 +2461,7 @@ static inline void hci_user_confirm_request_evt(struct hci_dev *hdev,
 							struct sk_buff *skb)
 {
 	struct hci_ev_user_confirm_req *ev = (void *) skb->data;
-	int loc_mitm, rem_mitm;
+	int loc_mitm, rem_mitm, confirm_hint = 0;
 	struct hci_conn *conn;
 
 	BT_DBG("%s", hdev->name);
@@ -2493,6 +2493,16 @@ static inline void hci_user_confirm_request_evt(struct hci_dev *hdev,
 	/* If no side requires MITM protection; auto-accept */
 	if ((!loc_mitm || conn->remote_cap == 0x03) &&
 				(!rem_mitm || conn->io_capability == 0x03)) {
+
+		/* If we're not the initiators request authorization to
+		 * proceed from user space (mgmt_user_confirm with
+		 * confirm_hint set to 1). */
+		if (!test_bit(HCI_CONN_AUTH_PEND, &conn->pend)) {
+			BT_DBG("Confirming auto-accept as acceptor");
+			confirm_hint = 1;
+			goto confirm;
+		}
+
 		BT_DBG("Auto-accept of user confirmation with %ums delay",
 						hdev->auto_accept_delay);
 
@@ -2507,7 +2517,9 @@ static inline void hci_user_confirm_request_evt(struct hci_dev *hdev,
 		goto unlock;
 	}
 
-	mgmt_user_confirm_request(hdev->id, &ev->bdaddr, ev->passkey);
+confirm:
+	mgmt_user_confirm_request(hdev->id, &ev->bdaddr, ev->passkey,
+								confirm_hint);
 
 unlock:
 	hci_dev_unlock(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index c304688..5ae581a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1920,13 +1920,15 @@ int mgmt_pin_code_neg_reply_complete(u16 index, bdaddr_t *bdaddr, u8 status)
 	return err;
 }
 
-int mgmt_user_confirm_request(u16 index, bdaddr_t *bdaddr, __le32 value)
+int mgmt_user_confirm_request(u16 index, bdaddr_t *bdaddr, __le32 value,
+							u8 confirm_hint)
 {
 	struct mgmt_ev_user_confirm_request ev;
 
 	BT_DBG("hci%u", index);
 
 	bacpy(&ev.bdaddr, bdaddr);
+	ev.confirm_hint = confirm_hint;
 	put_unaligned_le32(value, &ev.value);
 
 	return mgmt_event(MGMT_EV_USER_CONFIRM_REQUEST, index, &ev, sizeof(ev),
-- 
1.7.4.4

[PATCH 5/7] Bluetooth: Fix reason code for pairing rejection

[johan.hedberg]

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

"Pairing not allowed" is 0x18 and not 0x16.

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

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 003d731..0e8ccda 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2426,7 +2426,7 @@ static inline void hci_io_capa_request_evt(struct hci_dev *hdev, struct sk_buff
 		struct hci_cp_io_capability_neg_reply cp;
 
 		bacpy(&cp.bdaddr, &ev->bdaddr);
-		cp.reason = 0x16; /* Pairing not allowed */
+		cp.reason = 0x18; /* Pairing not allowed */
 
 		hci_send_cmd(hdev, HCI_OP_IO_CAPABILITY_NEG_REPLY,
 							sizeof(cp), &cp);
-- 
1.7.4.4

[PATCH 6/7] Bluetooth: Fix logic in hci_pin_code_request_evt

[johan.hedberg]

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

The mgmt_ev_pin_code_request event should not be sent to user space if
the request gets rejected by the kernel due to the pairable flag not
being set.

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

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0e8ccda..acb4e64 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2019,8 +2019,7 @@ static inline void hci_pin_code_request_evt(struct hci_dev *hdev, struct sk_buff
 	if (!test_bit(HCI_PAIRABLE, &hdev->flags))
 		hci_send_cmd(hdev, HCI_OP_PIN_CODE_NEG_REPLY,
 					sizeof(ev->bdaddr), &ev->bdaddr);
-
-	if (test_bit(HCI_MGMT, &hdev->flags))
+	else if (test_bit(HCI_MGMT, &hdev->flags))
 		mgmt_pin_code_request(hdev->id, &ev->bdaddr);
 
 	hci_dev_unlock(hdev);
-- 
1.7.4.4

[PATCH 7/7] Bluetooth: Fix link key persistent storage criteria

[johan.hedberg]

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

Link keys should only be stored if very specific criteria of the
authentication process are fulfilled. This patch essentially copies the
criteria that user space has so far been using to the kernel side so
that the management interface works properly.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 490b43b..5a794b7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -515,8 +515,8 @@ int hci_uuids_clear(struct hci_dev *hdev);
 
 int hci_link_keys_clear(struct hci_dev *hdev);
 struct link_key *hci_find_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr);
-int hci_add_link_key(struct hci_dev *hdev, int new_key, bdaddr_t *bdaddr,
-						u8 *key, u8 type, u8 pin_len);
+int hci_add_link_key(struct hci_dev *hdev, struct hci_conn *conn, int new_key,
+			bdaddr_t *bdaddr, u8 *val, u8 type, u8 pin_len);
 int hci_remove_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr);
 
 int hci_remote_oob_data_clear(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 07d0ba3..85fae57 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1022,8 +1022,44 @@ struct link_key *hci_find_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr)
 	return NULL;
 }
 
-int hci_add_link_key(struct hci_dev *hdev, int new_key, bdaddr_t *bdaddr,
-						u8 *val, u8 type, u8 pin_len)
+int hci_persistent_key(struct hci_dev *hdev, struct hci_conn *conn,
+						u8 key_type, u8 old_key_type)
+{
+	/* Legacy key */
+	if (key_type < 0x03)
+		return 1;
+
+	/* Debug keys are insecure so don't store them persistently */
+	if (key_type == HCI_LK_DEBUG_COMBINATION)
+		return 0;
+
+	/* Changed combination key and there's no previous one */
+	if (key_type == HCI_LK_CHANGED_COMBINATION && old_key_type == 0xff)
+		return 0;
+
+	/* Security mode 3 case */
+	if (!conn)
+		return 1;
+
+	/* Neither local nor remote side had no-bonding as requirement */
+	if (conn->auth_type > 0x01 && conn->remote_auth > 0x01)
+		return 1;
+
+	/* Local side had dedicated bonding as requirement */
+	if (conn->auth_type == 0x02 || conn->auth_type == 0x03)
+		return 1;
+
+	/* Remote side had dedicated bonding as requirement */
+	if (conn->remote_auth == 0x02 || conn->remote_auth == 0x03)
+		return 1;
+
+	/* If none of the above criteria match, then don't store the key
+	 * persistently */
+	return 0;
+}
+
+int hci_add_link_key(struct hci_dev *hdev, struct hci_conn *conn, int new_key,
+				bdaddr_t *bdaddr, u8 *val, u8 type, u8 pin_len)
 {
 	struct link_key *key, *old_key;
 	u8 old_key_type;
@@ -1042,6 +1078,20 @@ int hci_add_link_key(struct hci_dev *hdev, int new_key, bdaddr_t *bdaddr,
 
 	BT_DBG("%s key for %s type %u", hdev->name, batostr(bdaddr), type);
 
+	/* Some buggy controller combinations generate a changed
+	 * combination key for legacy pairing even when there's no
+	 * previous key */
+	if (type == HCI_LK_CHANGED_COMBINATION &&
+					(!conn || conn->remote_auth == 0xff) &&
+					old_key_type == 0xff)
+		type = HCI_LK_COMBINATION;
+
+	if (new_key && !hci_persistent_key(hdev, conn, type, old_key_type)) {
+		list_del(&key->list);
+		kfree(key);
+		return 0;
+	}
+
 	bacpy(&key->bdaddr, bdaddr);
 	memcpy(key->val, val, 16);
 	key->type = type;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index acb4e64..6982f41 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2112,7 +2112,7 @@ static inline void hci_link_key_notify_evt(struct hci_dev *hdev, struct sk_buff
 	}
 
 	if (test_bit(HCI_LINK_KEYS, &hdev->flags))
-		hci_add_link_key(hdev, 1, &ev->bdaddr, ev->link_key,
+		hci_add_link_key(hdev, conn, 1, &ev->bdaddr, ev->link_key,
 							ev->key_type, pin_len);
 
 	hci_dev_unlock(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 5ae581a..0262f79 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -945,7 +945,7 @@ static int load_keys(struct sock *sk, u16 index, unsigned char *data, u16 len)
 	for (i = 0; i < key_count; i++) {
 		struct mgmt_key_info *key = &cp->keys[i];
 
-		hci_add_link_key(hdev, 0, &key->bdaddr, key->val, key->type,
+		hci_add_link_key(hdev, NULL, 0, &key->bdaddr, key->val, key->type,
 								key->pin_len);
 	}
 
-- 
1.7.4.4

Re: [PATCH 7/7] Bluetooth: Fix link key persistent storage criteria

[Anderson Lizardo]

Hi Johan,

On Wed, Apr 27, 2011 at 7:04 PM,  <johan.hedberg@gmail.com> wrote:
> @@ -1042,6 +1078,20 @@ int hci_add_link_key(struct hci_dev *hdev, int new_key, bdaddr_t *bdaddr,
>
>        BT_DBG("%s key for %s type %u", hdev->name, batostr(bdaddr), type);
>
> +       /* Some buggy controller combinations generate a changed
> +        * combination key for legacy pairing even when there's no
> +        * previous key */
> +       if (type == HCI_LK_CHANGED_COMBINATION &&
> +                                       (!conn || conn->remote_auth == 0xff) &&
> +                                       old_key_type == 0xff)
> +               type = HCI_LK_COMBINATION;
> +
> +       if (new_key && !hci_persistent_key(hdev, conn, type, old_key_type)) {
> +               list_del(&key->list);
> +               kfree(key);
> +               return 0;

Just to make sure, is this condition possible?

* new_key parameter not zero
* old_key variable not NULL

If so, "key" will point to an entry from hdev->link_keys , which will
be freed on the if() above without removing it from the list.

> +       }
> +
>        bacpy(&key->bdaddr, bdaddr);
>        memcpy(key->val, val, 16);
>        key->type = type;

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 7/7] Bluetooth: Fix link key persistent storage criteria

[Johan Hedberg]

Hi Lizardo,

On Wed, Apr 27, 2011, Anderson Lizardo wrote:
> > +       if (new_key && !hci_persistent_key(hdev, conn, type, old_key_type)) {
> > +               list_del(&key->list);
> > +               kfree(key);
> > +               return 0;
> 
> Just to make sure, is this condition possible?
> 
> * new_key parameter not zero
> * old_key variable not NULL
> 
> If so, "key" will point to an entry from hdev->link_keys , which will
> be freed on the if() above without removing it from the list.

Do you see the list_del call there? :)

Johan

Re: [PATCH 7/7] Bluetooth: Fix link key persistent storage criteria

[Anderson Lizardo]

Hi Johan,

On Wed, Apr 27, 2011 at 7:57 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lizardo,
>
> On Wed, Apr 27, 2011, Anderson Lizardo wrote:
>> > +       if (new_key && !hci_persistent_key(hdev, conn, type, old_key_type)) {
>> > +               list_del(&key->list);
>> > +               kfree(key);
>> > +               return 0;
>>
>> Just to make sure, is this condition possible?
>>
>> * new_key parameter not zero
>> * old_key variable not NULL
>>
>> If so, "key" will point to an entry from hdev->link_keys , which will
>> be freed on the if() above without removing it from the list.
>
> Do you see the list_del call there? :)

Ok, I failed to see it :)

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 1/7] Bluetooth: Add automated SSP user confirmation responses

[Anderson Lizardo]

Hi Johan,

On Wed, Apr 27, 2011 at 7:04 PM,  <johan.hedberg@gmail.com> wrote:
> @@ -2447,14 +2447,47 @@ static inline void hci_user_confirm_request_evt(struct hci_dev *hdev,
>                                                        struct sk_buff *skb)
>  {
>        struct hci_ev_user_confirm_req *ev = (void *) skb->data;
> +       int loc_mitm, rem_mitm;
> +       struct hci_conn *conn;
>
>        BT_DBG("%s", hdev->name);
>
>        hci_dev_lock(hdev);
>
> -       if (test_bit(HCI_MGMT, &hdev->flags))
> -               mgmt_user_confirm_request(hdev->id, &ev->bdaddr, ev->passkey);
> +       if (!test_bit(HCI_MGMT, &hdev->flags))
> +               goto unlock;

You lock hdev for checking hdev->flags, but not for reading hdev->name
on the BT_DBG() call. Is that ok?

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 1/7] Bluetooth: Add automated SSP user confirmation responses

[Johan Hedberg]

Hi Lizardo,

On Wed, Apr 27, 2011, Anderson Lizardo wrote:
> On Wed, Apr 27, 2011 at 7:04 PM,  <johan.hedberg@gmail.com> wrote:
> > @@ -2447,14 +2447,47 @@ static inline void hci_user_confirm_request_evt(struct hci_dev *hdev,
> >                                                        struct sk_buff *skb)
> >  {
> >        struct hci_ev_user_confirm_req *ev = (void *) skb->data;
> > +       int loc_mitm, rem_mitm;
> > +       struct hci_conn *conn;
> >
> >        BT_DBG("%s", hdev->name);
> >
> >        hci_dev_lock(hdev);
> >
> > -       if (test_bit(HCI_MGMT, &hdev->flags))
> > -               mgmt_user_confirm_request(hdev->id, &ev->bdaddr, ev->passkey);
> > +       if (!test_bit(HCI_MGMT, &hdev->flags))
> > +               goto unlock;
> 
> You lock hdev for checking hdev->flags, but not for reading hdev->name
> on the BT_DBG() call. Is that ok?

I suppose hdev->name can be considered a read-only value as it never
changes after the HCI dev is created. There's also lots of other places
in hci_event.c that access it without locking, so I think this should be
fine.

Johan