[PATCH v2 0/9] Bluetooth: SMP fixes & cleanups

[PATCH v2 0/9] Bluetooth: SMP fixes & cleanups

[johan.hedberg]

Hi,

Here's a v2 of all my earlier three patch sets combined. The main change
is renaming l2cap_conn_drop to l2cap_conn_shutdown in order to not be
confused with the slightly different semantics of hci_conn_drop.

Johan

----------------------------------------------------------------
Johan Hedberg (9):
      Bluetooth: Use L2CAP resume callback to call smp_distribute_keys
      Bluetooth: Add public l2cap_conn_shutdown() API to request disconnection
      Bluetooth: Call l2cap_conn_shutdown() when SMP recv callback fails
      Bluetooth: Fix double free of SMP data skb
      Bluetooth: Add SMP-internal timeout callback
      Bluetooth: Remove unused l2cap_conn->security_timer
      Bluetooth: Move canceling security_timer into smp_chan_destroy()
      Bluetooth: Always call smp_distribute_keys() from a workqueue
      Bluetooth: Make smp_chan_destroy() private to smp.c

 include/net/bluetooth/l2cap.h |   5 +-
 net/bluetooth/l2cap_core.c    |  35 +--
 net/bluetooth/smp.c           | 541 +++++++++++++++++++++-----------------
 net/bluetooth/smp.h           |   3 -
 4 files changed, 330 insertions(+), 254 deletions(-)

[PATCH v2 1/9] Bluetooth: Use L2CAP resume callback to call smp_distribute_keys

[johan.hedberg]

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

There's no need to export the smp_distribute_keys() function since the
resume callback is called in the same scenario. This patch makes the
smp_notify_keys function private (at the same time moving it higher up
in smp.c to avoid forward declarations) and adds a resume callback for
SMP to call it from there instead.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c |   6 -
 net/bluetooth/smp.c        | 380 +++++++++++++++++++++++----------------------
 net/bluetooth/smp.h        |   1 -
 3 files changed, 196 insertions(+), 191 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 40b6c06ab2c0..4de1e1827055 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7281,12 +7281,6 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 
 	BT_DBG("conn %p status 0x%2.2x encrypt %u", conn, status, encrypt);
 
-	if (hcon->type == LE_LINK) {
-		if (!status && encrypt)
-			smp_distribute_keys(conn);
-		cancel_delayed_work(&conn->security_timer);
-	}
-
 	mutex_lock(&conn->chan_lock);
 
 	list_for_each_entry(chan, &conn->chan_l, list) {
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 744f678ac3e8..28014ad3d2d3 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -575,6 +575,189 @@ static u8 smp_random(struct smp_chan *smp)
 	return 0;
 }
 
+static void smp_notify_keys(struct l2cap_conn *conn)
+{
+	struct l2cap_chan *chan = conn->smp;
+	struct smp_chan *smp = chan->data;
+	struct hci_conn *hcon = conn->hcon;
+	struct hci_dev *hdev = hcon->hdev;
+	struct smp_cmd_pairing *req = (void *) &smp->preq[1];
+	struct smp_cmd_pairing *rsp = (void *) &smp->prsp[1];
+	bool persistent;
+
+	if (smp->remote_irk) {
+		mgmt_new_irk(hdev, smp->remote_irk);
+		/* Now that user space can be considered to know the
+		 * identity address track the connection based on it
+		 * from now on.
+		 */
+		bacpy(&hcon->dst, &smp->remote_irk->bdaddr);
+		hcon->dst_type = smp->remote_irk->addr_type;
+		l2cap_conn_update_id_addr(hcon);
+
+		/* When receiving an indentity resolving key for
+		 * a remote device that does not use a resolvable
+		 * private address, just remove the key so that
+		 * it is possible to use the controller white
+		 * list for scanning.
+		 *
+		 * Userspace will have been told to not store
+		 * this key at this point. So it is safe to
+		 * just remove it.
+		 */
+		if (!bacmp(&smp->remote_irk->rpa, BDADDR_ANY)) {
+			list_del(&smp->remote_irk->list);
+			kfree(smp->remote_irk);
+			smp->remote_irk = NULL;
+		}
+	}
+
+	/* The LTKs and CSRKs should be persistent only if both sides
+	 * had the bonding bit set in their authentication requests.
+	 */
+	persistent = !!((req->auth_req & rsp->auth_req) & SMP_AUTH_BONDING);
+
+	if (smp->csrk) {
+		smp->csrk->bdaddr_type = hcon->dst_type;
+		bacpy(&smp->csrk->bdaddr, &hcon->dst);
+		mgmt_new_csrk(hdev, smp->csrk, persistent);
+	}
+
+	if (smp->slave_csrk) {
+		smp->slave_csrk->bdaddr_type = hcon->dst_type;
+		bacpy(&smp->slave_csrk->bdaddr, &hcon->dst);
+		mgmt_new_csrk(hdev, smp->slave_csrk, persistent);
+	}
+
+	if (smp->ltk) {
+		smp->ltk->bdaddr_type = hcon->dst_type;
+		bacpy(&smp->ltk->bdaddr, &hcon->dst);
+		mgmt_new_ltk(hdev, smp->ltk, persistent);
+	}
+
+	if (smp->slave_ltk) {
+		smp->slave_ltk->bdaddr_type = hcon->dst_type;
+		bacpy(&smp->slave_ltk->bdaddr, &hcon->dst);
+		mgmt_new_ltk(hdev, smp->slave_ltk, persistent);
+	}
+}
+
+static int smp_distribute_keys(struct l2cap_conn *conn)
+{
+	struct smp_cmd_pairing *req, *rsp;
+	struct l2cap_chan *chan = conn->smp;
+	struct smp_chan *smp = chan->data;
+	struct hci_conn *hcon = conn->hcon;
+	struct hci_dev *hdev = hcon->hdev;
+	__u8 *keydist;
+
+	BT_DBG("conn %p", conn);
+
+	if (!test_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags))
+		return 0;
+
+	rsp = (void *) &smp->prsp[1];
+
+	/* The responder sends its keys first */
+	if (hcon->out && (smp->remote_key_dist & 0x07))
+		return 0;
+
+	req = (void *) &smp->preq[1];
+
+	if (hcon->out) {
+		keydist = &rsp->init_key_dist;
+		*keydist &= req->init_key_dist;
+	} else {
+		keydist = &rsp->resp_key_dist;
+		*keydist &= req->resp_key_dist;
+	}
+
+	BT_DBG("keydist 0x%x", *keydist);
+
+	if (*keydist & SMP_DIST_ENC_KEY) {
+		struct smp_cmd_encrypt_info enc;
+		struct smp_cmd_master_ident ident;
+		struct smp_ltk *ltk;
+		u8 authenticated;
+		__le16 ediv;
+		__le64 rand;
+
+		get_random_bytes(enc.ltk, sizeof(enc.ltk));
+		get_random_bytes(&ediv, sizeof(ediv));
+		get_random_bytes(&rand, sizeof(rand));
+
+		smp_send_cmd(conn, SMP_CMD_ENCRYPT_INFO, sizeof(enc), &enc);
+
+		authenticated = hcon->sec_level == BT_SECURITY_HIGH;
+		ltk = hci_add_ltk(hdev, &hcon->dst, hcon->dst_type,
+				  SMP_LTK_SLAVE, authenticated, enc.ltk,
+				  smp->enc_key_size, ediv, rand);
+		smp->slave_ltk = ltk;
+
+		ident.ediv = ediv;
+		ident.rand = rand;
+
+		smp_send_cmd(conn, SMP_CMD_MASTER_IDENT, sizeof(ident), &ident);
+
+		*keydist &= ~SMP_DIST_ENC_KEY;
+	}
+
+	if (*keydist & SMP_DIST_ID_KEY) {
+		struct smp_cmd_ident_addr_info addrinfo;
+		struct smp_cmd_ident_info idinfo;
+
+		memcpy(idinfo.irk, hdev->irk, sizeof(idinfo.irk));
+
+		smp_send_cmd(conn, SMP_CMD_IDENT_INFO, sizeof(idinfo), &idinfo);
+
+		/* The hci_conn contains the local identity address
+		 * after the connection has been established.
+		 *
+		 * This is true even when the connection has been
+		 * established using a resolvable random address.
+		 */
+		bacpy(&addrinfo.bdaddr, &hcon->src);
+		addrinfo.addr_type = hcon->src_type;
+
+		smp_send_cmd(conn, SMP_CMD_IDENT_ADDR_INFO, sizeof(addrinfo),
+			     &addrinfo);
+
+		*keydist &= ~SMP_DIST_ID_KEY;
+	}
+
+	if (*keydist & SMP_DIST_SIGN) {
+		struct smp_cmd_sign_info sign;
+		struct smp_csrk *csrk;
+
+		/* Generate a new random key */
+		get_random_bytes(sign.csrk, sizeof(sign.csrk));
+
+		csrk = kzalloc(sizeof(*csrk), GFP_KERNEL);
+		if (csrk) {
+			csrk->master = 0x00;
+			memcpy(csrk->val, sign.csrk, sizeof(csrk->val));
+		}
+		smp->slave_csrk = csrk;
+
+		smp_send_cmd(conn, SMP_CMD_SIGN_INFO, sizeof(sign), &sign);
+
+		*keydist &= ~SMP_DIST_SIGN;
+	}
+
+	/* If there are still keys to be received wait for them */
+	if ((smp->remote_key_dist & 0x07))
+		return 0;
+
+	clear_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags);
+	cancel_delayed_work_sync(&conn->security_timer);
+	set_bit(SMP_FLAG_COMPLETE, &smp->flags);
+	smp_notify_keys(conn);
+
+	smp_chan_destroy(conn);
+
+	return 0;
+}
+
 static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)
 {
 	struct l2cap_chan *chan = conn->smp;
@@ -1294,189 +1477,6 @@ done:
 	return err;
 }
 
-static void smp_notify_keys(struct l2cap_conn *conn)
-{
-	struct l2cap_chan *chan = conn->smp;
-	struct smp_chan *smp = chan->data;
-	struct hci_conn *hcon = conn->hcon;
-	struct hci_dev *hdev = hcon->hdev;
-	struct smp_cmd_pairing *req = (void *) &smp->preq[1];
-	struct smp_cmd_pairing *rsp = (void *) &smp->prsp[1];
-	bool persistent;
-
-	if (smp->remote_irk) {
-		mgmt_new_irk(hdev, smp->remote_irk);
-		/* Now that user space can be considered to know the
-		 * identity address track the connection based on it
-		 * from now on.
-		 */
-		bacpy(&hcon->dst, &smp->remote_irk->bdaddr);
-		hcon->dst_type = smp->remote_irk->addr_type;
-		l2cap_conn_update_id_addr(hcon);
-
-		/* When receiving an indentity resolving key for
-		 * a remote device that does not use a resolvable
-		 * private address, just remove the key so that
-		 * it is possible to use the controller white
-		 * list for scanning.
-		 *
-		 * Userspace will have been told to not store
-		 * this key at this point. So it is safe to
-		 * just remove it.
-		 */
-		if (!bacmp(&smp->remote_irk->rpa, BDADDR_ANY)) {
-			list_del(&smp->remote_irk->list);
-			kfree(smp->remote_irk);
-			smp->remote_irk = NULL;
-		}
-	}
-
-	/* The LTKs and CSRKs should be persistent only if both sides
-	 * had the bonding bit set in their authentication requests.
-	 */
-	persistent = !!((req->auth_req & rsp->auth_req) & SMP_AUTH_BONDING);
-
-	if (smp->csrk) {
-		smp->csrk->bdaddr_type = hcon->dst_type;
-		bacpy(&smp->csrk->bdaddr, &hcon->dst);
-		mgmt_new_csrk(hdev, smp->csrk, persistent);
-	}
-
-	if (smp->slave_csrk) {
-		smp->slave_csrk->bdaddr_type = hcon->dst_type;
-		bacpy(&smp->slave_csrk->bdaddr, &hcon->dst);
-		mgmt_new_csrk(hdev, smp->slave_csrk, persistent);
-	}
-
-	if (smp->ltk) {
-		smp->ltk->bdaddr_type = hcon->dst_type;
-		bacpy(&smp->ltk->bdaddr, &hcon->dst);
-		mgmt_new_ltk(hdev, smp->ltk, persistent);
-	}
-
-	if (smp->slave_ltk) {
-		smp->slave_ltk->bdaddr_type = hcon->dst_type;
-		bacpy(&smp->slave_ltk->bdaddr, &hcon->dst);
-		mgmt_new_ltk(hdev, smp->slave_ltk, persistent);
-	}
-}
-
-int smp_distribute_keys(struct l2cap_conn *conn)
-{
-	struct smp_cmd_pairing *req, *rsp;
-	struct l2cap_chan *chan = conn->smp;
-	struct smp_chan *smp = chan->data;
-	struct hci_conn *hcon = conn->hcon;
-	struct hci_dev *hdev = hcon->hdev;
-	__u8 *keydist;
-
-	BT_DBG("conn %p", conn);
-
-	if (!test_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags))
-		return 0;
-
-	rsp = (void *) &smp->prsp[1];
-
-	/* The responder sends its keys first */
-	if (hcon->out && (smp->remote_key_dist & 0x07))
-		return 0;
-
-	req = (void *) &smp->preq[1];
-
-	if (hcon->out) {
-		keydist = &rsp->init_key_dist;
-		*keydist &= req->init_key_dist;
-	} else {
-		keydist = &rsp->resp_key_dist;
-		*keydist &= req->resp_key_dist;
-	}
-
-	BT_DBG("keydist 0x%x", *keydist);
-
-	if (*keydist & SMP_DIST_ENC_KEY) {
-		struct smp_cmd_encrypt_info enc;
-		struct smp_cmd_master_ident ident;
-		struct smp_ltk *ltk;
-		u8 authenticated;
-		__le16 ediv;
-		__le64 rand;
-
-		get_random_bytes(enc.ltk, sizeof(enc.ltk));
-		get_random_bytes(&ediv, sizeof(ediv));
-		get_random_bytes(&rand, sizeof(rand));
-
-		smp_send_cmd(conn, SMP_CMD_ENCRYPT_INFO, sizeof(enc), &enc);
-
-		authenticated = hcon->sec_level == BT_SECURITY_HIGH;
-		ltk = hci_add_ltk(hdev, &hcon->dst, hcon->dst_type,
-				  SMP_LTK_SLAVE, authenticated, enc.ltk,
-				  smp->enc_key_size, ediv, rand);
-		smp->slave_ltk = ltk;
-
-		ident.ediv = ediv;
-		ident.rand = rand;
-
-		smp_send_cmd(conn, SMP_CMD_MASTER_IDENT, sizeof(ident), &ident);
-
-		*keydist &= ~SMP_DIST_ENC_KEY;
-	}
-
-	if (*keydist & SMP_DIST_ID_KEY) {
-		struct smp_cmd_ident_addr_info addrinfo;
-		struct smp_cmd_ident_info idinfo;
-
-		memcpy(idinfo.irk, hdev->irk, sizeof(idinfo.irk));
-
-		smp_send_cmd(conn, SMP_CMD_IDENT_INFO, sizeof(idinfo), &idinfo);
-
-		/* The hci_conn contains the local identity address
-		 * after the connection has been established.
-		 *
-		 * This is true even when the connection has been
-		 * established using a resolvable random address.
-		 */
-		bacpy(&addrinfo.bdaddr, &hcon->src);
-		addrinfo.addr_type = hcon->src_type;
-
-		smp_send_cmd(conn, SMP_CMD_IDENT_ADDR_INFO, sizeof(addrinfo),
-			     &addrinfo);
-
-		*keydist &= ~SMP_DIST_ID_KEY;
-	}
-
-	if (*keydist & SMP_DIST_SIGN) {
-		struct smp_cmd_sign_info sign;
-		struct smp_csrk *csrk;
-
-		/* Generate a new random key */
-		get_random_bytes(sign.csrk, sizeof(sign.csrk));
-
-		csrk = kzalloc(sizeof(*csrk), GFP_KERNEL);
-		if (csrk) {
-			csrk->master = 0x00;
-			memcpy(csrk->val, sign.csrk, sizeof(csrk->val));
-		}
-		smp->slave_csrk = csrk;
-
-		smp_send_cmd(conn, SMP_CMD_SIGN_INFO, sizeof(sign), &sign);
-
-		*keydist &= ~SMP_DIST_SIGN;
-	}
-
-	/* If there are still keys to be received wait for them */
-	if ((smp->remote_key_dist & 0x07))
-		return 0;
-
-	clear_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags);
-	cancel_delayed_work_sync(&conn->security_timer);
-	set_bit(SMP_FLAG_COMPLETE, &smp->flags);
-	smp_notify_keys(conn);
-
-	smp_chan_destroy(conn);
-
-	return 0;
-}
-
 static void smp_teardown_cb(struct l2cap_chan *chan, int err)
 {
 	struct l2cap_conn *conn = chan->conn;
@@ -1492,6 +1492,18 @@ static void smp_teardown_cb(struct l2cap_chan *chan, int err)
 	l2cap_chan_put(chan);
 }
 
+static void smp_resume_cb(struct l2cap_chan *chan)
+{
+	struct l2cap_conn *conn = chan->conn;
+	struct hci_conn *hcon = conn->hcon;
+
+	BT_DBG("chan %p", chan);
+
+	if (test_bit(HCI_CONN_ENCRYPT, &hcon->flags))
+		smp_distribute_keys(conn);
+	cancel_delayed_work(&conn->security_timer);
+}
+
 static void smp_ready_cb(struct l2cap_chan *chan)
 {
 	struct l2cap_conn *conn = chan->conn;
@@ -1524,13 +1536,13 @@ static const struct l2cap_ops smp_chan_ops = {
 	.recv			= smp_recv_cb,
 	.alloc_skb		= smp_alloc_skb_cb,
 	.teardown		= smp_teardown_cb,
+	.resume			= smp_resume_cb,
 
 	.new_connection		= l2cap_chan_no_new_connection,
 	.state_change		= l2cap_chan_no_state_change,
 	.close			= l2cap_chan_no_close,
 	.defer			= l2cap_chan_no_defer,
 	.suspend		= l2cap_chan_no_suspend,
-	.resume			= l2cap_chan_no_resume,
 	.set_shutdown		= l2cap_chan_no_set_shutdown,
 	.get_sndtimeo		= l2cap_chan_no_get_sndtimeo,
 	.memcpy_fromiovec	= l2cap_chan_no_memcpy_fromiovec,
diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
index 161ace3c3234..59a594278a85 100644
--- a/net/bluetooth/smp.h
+++ b/net/bluetooth/smp.h
@@ -126,7 +126,6 @@ enum {
 /* SMP Commands */
 bool smp_sufficient_security(struct hci_conn *hcon, u8 sec_level);
 int smp_conn_security(struct hci_conn *hcon, __u8 sec_level);
-int smp_distribute_keys(struct l2cap_conn *conn);
 int smp_user_confirm_reply(struct hci_conn *conn, u16 mgmt_op, __le32 passkey);
 
 void smp_chan_destroy(struct l2cap_conn *conn);
-- 
1.9.3

[PATCH v2 2/9] Bluetooth: Add public l2cap_conn_shutdown() API to request disconnection

[johan.hedberg]

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

Since we no-longer do special handling of SMP within l2cap_core.c we
don't have any code for calling l2cap_conn_del() when smp.c doesn't like
the data it gets. At the same time we cannot simply export
l2cap_conn_del() since it will try to lock the channels it calls into
whereas we already hold the lock in the smp.c l2cap_chan callbacks (i.e.
it'd lead to a deadlock).

This patch adds a new l2cap_conn_shutdown() API which is very similar to
l2cap_conn_del() except that it defers the call to l2cap_conn_del()
through a workqueue, thereby making it safe to use it from an L2CAP
channel callback.

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

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index bda6252e3722..40f34866b6da 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -625,6 +625,9 @@ struct l2cap_conn {
 
 	struct delayed_work	info_timer;
 
+	int			disconn_err;
+	struct work_struct	disconn_work;
+
 	struct sk_buff		*rx_skb;
 	__u32			rx_len;
 	__u8			tx_ident;
@@ -944,6 +947,7 @@ void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
 		       u8 status);
 void __l2cap_physical_cfm(struct l2cap_chan *chan, int result);
 
+void l2cap_conn_shutdown(struct l2cap_conn *conn, int err);
 void l2cap_conn_get(struct l2cap_conn *conn);
 void l2cap_conn_put(struct l2cap_conn *conn);
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 4de1e1827055..404998efa7e2 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1627,6 +1627,9 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 	if (work_pending(&conn->pending_rx_work))
 		cancel_work_sync(&conn->pending_rx_work);
 
+	if (work_pending(&conn->disconn_work))
+		cancel_work_sync(&conn->disconn_work);
+
 	l2cap_unregister_all_users(conn);
 
 	mutex_lock(&conn->chan_lock);
@@ -1669,6 +1672,26 @@ static void security_timeout(struct work_struct *work)
 	}
 }
 
+static void disconn_work(struct work_struct *work)
+{
+	struct l2cap_conn *conn = container_of(work, struct l2cap_conn,
+					       disconn_work);
+
+	BT_DBG("conn %p", conn);
+
+	l2cap_conn_del(conn->hcon, conn->disconn_err);
+}
+
+void l2cap_conn_shutdown(struct l2cap_conn *conn, int err)
+{
+	struct hci_dev *hdev = conn->hcon->hdev;
+
+	BT_DBG("conn %p err %d", conn, err);
+
+	conn->disconn_err = err;
+	queue_work(hdev->workqueue, &conn->disconn_work);
+}
+
 static void l2cap_conn_free(struct kref *ref)
 {
 	struct l2cap_conn *conn = container_of(ref, struct l2cap_conn, ref);
@@ -6930,6 +6953,8 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
 	else
 		INIT_DELAYED_WORK(&conn->info_timer, l2cap_info_timeout);
 
+	INIT_WORK(&conn->disconn_work, disconn_work);
+
 	skb_queue_head_init(&conn->pending_rx);
 	INIT_WORK(&conn->pending_rx_work, process_pending_rx);
 
-- 
1.9.3

[PATCH v2 3/9] Bluetooth: Call l2cap_conn_shutdown() when SMP recv callback fails

[johan.hedberg]

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

To restore pre-l2cap_chan functionality we should be trying to
disconnect the connection when receviving garbage SMP data (i.e. when
the SMP command handler fails). This patch renames the command handler
back to smp_sig_channel() and adds a smp_recv_cb() wrapper function for
calling it. If smp_sig_channel() fails the code calls
l2cap_conn_shutdown().

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

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 28014ad3d2d3..7a295d7edc44 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -1375,7 +1375,7 @@ static int smp_cmd_sign_info(struct l2cap_conn *conn, struct sk_buff *skb)
 	return 0;
 }
 
-static int smp_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
+static int smp_sig_channel(struct l2cap_chan *chan, struct sk_buff *skb)
 {
 	struct l2cap_conn *conn = chan->conn;
 	struct hci_conn *hcon = conn->hcon;
@@ -1514,6 +1514,24 @@ static void smp_ready_cb(struct l2cap_chan *chan)
 	l2cap_chan_hold(chan);
 }
 
+static int smp_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
+{
+	int err;
+
+	BT_DBG("chan %p", chan);
+
+	err = smp_sig_channel(chan, skb);
+	if (err) {
+		struct l2cap_conn *conn = chan->conn;
+
+		cancel_delayed_work_sync(&conn->security_timer);
+
+		l2cap_conn_shutdown(chan->conn, -err);
+	}
+
+	return err;
+}
+
 static struct sk_buff *smp_alloc_skb_cb(struct l2cap_chan *chan,
 					unsigned long hdr_len,
 					unsigned long len, int nb)
-- 
1.9.3

[PATCH v2 4/9] Bluetooth: Fix double free of SMP data skb

[johan.hedberg]

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

In the case that the SMP recv callback returns error the calling code in
l2cap_core.c expects that it still owns the skb and will try to free it.
The SMP code should therefore not try to free the skb if it return an
error. This patch fixes such behavior in the SMP command handler
function.

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

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 7a295d7edc44..5103dc739a17 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -1387,10 +1387,8 @@ static int smp_sig_channel(struct l2cap_chan *chan, struct sk_buff *skb)
 		return 0;
 	}
 
-	if (skb->len < 1) {
-		kfree_skb(skb);
+	if (skb->len < 1)
 		return -EILSEQ;
-	}
 
 	if (!test_bit(HCI_LE_ENABLED, &hcon->hdev->dev_flags)) {
 		err = -EOPNOTSUPP;
@@ -1410,8 +1408,9 @@ static int smp_sig_channel(struct l2cap_chan *chan, struct sk_buff *skb)
 	if (code != SMP_CMD_PAIRING_REQ && code != SMP_CMD_SECURITY_REQ &&
 	    !test_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags)) {
 		BT_ERR("Unexpected SMP command 0x%02x. Disconnecting.", code);
-		kfree_skb(skb);
-		return -EOPNOTSUPP;
+		reason = SMP_CMD_NOTSUPP;
+		err = -EOPNOTSUPP;
+		goto done;
 	}
 
 	switch (code) {
@@ -1472,8 +1471,8 @@ static int smp_sig_channel(struct l2cap_chan *chan, struct sk_buff *skb)
 done:
 	if (reason)
 		smp_failure(conn, reason);
-
-	kfree_skb(skb);
+	if (!err)
+		kfree_skb(skb);
 	return err;
 }
 
-- 
1.9.3

[PATCH v2 5/9] Bluetooth: Add SMP-internal timeout callback

[johan.hedberg]

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

This patch adds an SMP-internal timeout callback to remove the depenency
on (the soon to be removed) l2cap_conn->security_timer. The behavior is
the same as with l2cap_conn->security_timer except that the new
l2cap_conn_shutdown() public function is used instead of the L2CAP core
internal l2cap_conn_del().

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

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 5103dc739a17..7ab5f52955cb 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -44,7 +44,9 @@ enum {
 };
 
 struct smp_chan {
-	struct l2cap_conn *conn;
+	struct l2cap_conn	*conn;
+	struct delayed_work	security_timer;
+
 	u8		preq[7]; /* SMP Pairing Request */
 	u8		prsp[7]; /* SMP Pairing Response */
 	u8		prnd[16]; /* SMP Pairing Random (local) */
@@ -251,6 +253,7 @@ static int smp_s1(struct smp_chan *smp, u8 k[16], u8 r1[16], u8 r2[16],
 static void smp_send_cmd(struct l2cap_conn *conn, u8 code, u16 len, void *data)
 {
 	struct l2cap_chan *chan = conn->smp;
+	struct smp_chan *smp;
 	struct kvec iv[2];
 	struct msghdr msg;
 
@@ -272,8 +275,14 @@ static void smp_send_cmd(struct l2cap_conn *conn, u8 code, u16 len, void *data)
 
 	l2cap_chan_send(chan, &msg, 1 + len);
 
-	cancel_delayed_work_sync(&conn->security_timer);
-	schedule_delayed_work(&conn->security_timer, SMP_TIMEOUT);
+	if (!chan->data)
+		return;
+
+	smp = chan->data;
+
+	cancel_delayed_work_sync(&smp->security_timer);
+	if (test_bit(HCI_CONN_LE_SMP_PEND, &conn->hcon->flags))
+		schedule_delayed_work(&smp->security_timer, SMP_TIMEOUT);
 }
 
 static __u8 authreq_to_seclevel(__u8 authreq)
@@ -359,6 +368,8 @@ static u8 check_enc_key_size(struct l2cap_conn *conn, __u8 max_key_size)
 static void smp_failure(struct l2cap_conn *conn, u8 reason)
 {
 	struct hci_conn *hcon = conn->hcon;
+	struct l2cap_chan *chan = conn->smp;
+	struct smp_chan *smp;
 
 	if (reason)
 		smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, sizeof(reason),
@@ -368,7 +379,12 @@ static void smp_failure(struct l2cap_conn *conn, u8 reason)
 	mgmt_auth_failed(hcon->hdev, &hcon->dst, hcon->type, hcon->dst_type,
 			 HCI_ERROR_AUTH_FAILURE);
 
-	cancel_delayed_work_sync(&conn->security_timer);
+	if (!chan->data)
+		return;
+
+	smp = chan->data;
+
+	cancel_delayed_work_sync(&smp->security_timer);
 
 	if (test_and_clear_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags))
 		smp_chan_destroy(conn);
@@ -749,7 +765,7 @@ static int smp_distribute_keys(struct l2cap_conn *conn)
 		return 0;
 
 	clear_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags);
-	cancel_delayed_work_sync(&conn->security_timer);
+	cancel_delayed_work_sync(&smp->security_timer);
 	set_bit(SMP_FLAG_COMPLETE, &smp->flags);
 	smp_notify_keys(conn);
 
@@ -758,6 +774,17 @@ static int smp_distribute_keys(struct l2cap_conn *conn)
 	return 0;
 }
 
+static void smp_timeout(struct work_struct *work)
+{
+	struct smp_chan *smp = container_of(work, struct smp_chan,
+					    security_timer.work);
+	struct l2cap_conn *conn = smp->conn;
+
+	BT_DBG("conn %p", conn);
+
+	l2cap_conn_shutdown(conn, ETIMEDOUT);
+}
+
 static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)
 {
 	struct l2cap_chan *chan = conn->smp;
@@ -780,6 +807,8 @@ static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)
 	smp->conn = conn;
 	chan->data = smp;
 
+	INIT_DELAYED_WORK(&smp->security_timer, smp_timeout);
+
 	hci_conn_hold(conn->hcon);
 
 	return smp;
@@ -1479,11 +1508,12 @@ done:
 static void smp_teardown_cb(struct l2cap_chan *chan, int err)
 {
 	struct l2cap_conn *conn = chan->conn;
+	struct smp_chan *smp = chan->data;
 
 	BT_DBG("chan %p", chan);
 
 	if (test_and_clear_bit(HCI_CONN_LE_SMP_PEND, &conn->hcon->flags)) {
-		cancel_delayed_work_sync(&conn->security_timer);
+		cancel_delayed_work_sync(&smp->security_timer);
 		smp_chan_destroy(conn);
 	}
 
@@ -1493,6 +1523,7 @@ static void smp_teardown_cb(struct l2cap_chan *chan, int err)
 
 static void smp_resume_cb(struct l2cap_chan *chan)
 {
+	struct smp_chan *smp = chan->data;
 	struct l2cap_conn *conn = chan->conn;
 	struct hci_conn *hcon = conn->hcon;
 
@@ -1500,7 +1531,9 @@ static void smp_resume_cb(struct l2cap_chan *chan)
 
 	if (test_bit(HCI_CONN_ENCRYPT, &hcon->flags))
 		smp_distribute_keys(conn);
-	cancel_delayed_work(&conn->security_timer);
+
+	if (smp)
+		cancel_delayed_work(&smp->security_timer);
 }
 
 static void smp_ready_cb(struct l2cap_chan *chan)
@@ -1521,9 +1554,10 @@ static int smp_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
 
 	err = smp_sig_channel(chan, skb);
 	if (err) {
-		struct l2cap_conn *conn = chan->conn;
+		struct smp_chan *smp = chan->data;
 
-		cancel_delayed_work_sync(&conn->security_timer);
+		if (smp)
+			cancel_delayed_work_sync(&smp->security_timer);
 
 		l2cap_conn_shutdown(chan->conn, -err);
 	}
-- 
1.9.3

[PATCH v2 6/9] Bluetooth: Remove unused l2cap_conn->security_timer

[johan.hedberg]

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

Now that there are no-longer any users for l2cap_conn->security_timer we
can go ahead and simply remove it. The patch makes initialization of the
conn->info_timer unconditional since it's better not to leave any
l2cap_conn data structures uninitialized no matter what the underlying
transport.

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

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 40f34866b6da..cedda399f9c0 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -638,7 +638,6 @@ struct l2cap_conn {
 
 	__u8			disc_reason;
 
-	struct delayed_work	security_timer;
 	struct l2cap_chan	*smp;
 
 	struct list_head	chan_l;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 404998efa7e2..0cd7ed99558b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1659,19 +1659,6 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 	l2cap_conn_put(conn);
 }
 
-static void security_timeout(struct work_struct *work)
-{
-	struct l2cap_conn *conn = container_of(work, struct l2cap_conn,
-					       security_timer.work);
-
-	BT_DBG("conn %p", conn);
-
-	if (test_and_clear_bit(HCI_CONN_LE_SMP_PEND, &conn->hcon->flags)) {
-		smp_chan_destroy(conn);
-		l2cap_conn_del(conn->hcon, ETIMEDOUT);
-	}
-}
-
 static void disconn_work(struct work_struct *work)
 {
 	struct l2cap_conn *conn = container_of(work, struct l2cap_conn,
@@ -6948,10 +6935,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
 	INIT_LIST_HEAD(&conn->chan_l);
 	INIT_LIST_HEAD(&conn->users);
 
-	if (hcon->type == LE_LINK)
-		INIT_DELAYED_WORK(&conn->security_timer, security_timeout);
-	else
-		INIT_DELAYED_WORK(&conn->info_timer, l2cap_info_timeout);
+	INIT_DELAYED_WORK(&conn->info_timer, l2cap_info_timeout);
 
 	INIT_WORK(&conn->disconn_work, disconn_work);
 
-- 
1.9.3

[PATCH v2 7/9] Bluetooth: Move canceling security_timer into smp_chan_destroy()

[johan.hedberg]

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

All places needing to cancel the security timer also call
smp_chan_destroy() in the same go. To eliminate the need to do these two
calls in multiple places simply move the timer cancellation into
smp_chan_destroy().

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

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 7ab5f52955cb..cdae40869447 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -384,8 +384,6 @@ static void smp_failure(struct l2cap_conn *conn, u8 reason)
 
 	smp = chan->data;
 
-	cancel_delayed_work_sync(&smp->security_timer);
-
 	if (test_and_clear_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags))
 		smp_chan_destroy(conn);
 }
@@ -765,7 +763,6 @@ static int smp_distribute_keys(struct l2cap_conn *conn)
 		return 0;
 
 	clear_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags);
-	cancel_delayed_work_sync(&smp->security_timer);
 	set_bit(SMP_FLAG_COMPLETE, &smp->flags);
 	smp_notify_keys(conn);
 
@@ -822,6 +819,11 @@ void smp_chan_destroy(struct l2cap_conn *conn)
 
 	BUG_ON(!smp);
 
+	cancel_delayed_work_sync(&smp->security_timer);
+	/* In case the timeout freed the SMP context */
+	if (!chan->data)
+		return;
+
 	complete = test_bit(SMP_FLAG_COMPLETE, &smp->flags);
 	mgmt_smp_complete(conn->hcon, complete);
 
@@ -1512,10 +1514,8 @@ static void smp_teardown_cb(struct l2cap_chan *chan, int err)
 
 	BT_DBG("chan %p", chan);
 
-	if (test_and_clear_bit(HCI_CONN_LE_SMP_PEND, &conn->hcon->flags)) {
-		cancel_delayed_work_sync(&smp->security_timer);
+	if (test_and_clear_bit(HCI_CONN_LE_SMP_PEND, &conn->hcon->flags))
 		smp_chan_destroy(conn);
-	}
 
 	conn->smp = NULL;
 	l2cap_chan_put(chan);
-- 
1.9.3

[PATCH v2 8/9] Bluetooth: Always call smp_distribute_keys() from a workqueue

[johan.hedberg]

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

The smp_distribute_keys() function calls smp_notify_keys() which in turn
calls l2cap_conn_update_id_addr(). The l2cap_conn_update_id_addr()
function will iterate through all L2CAP channels for the respective
connection: lock the channel, update the address information and unlock
the channel.

Since SMP is now using l2cap_chan callbacks each callback is called with
the channel lock held. Therefore, calling l2cap_conn_update_id_addr()
would cause a deadlock calling l2cap_chan_lock() on the SMP channel.
This patch moves calling smp_distribute_keys() through a workqueue so
that it is never called from an L2CAP channel callback.

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

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index cdae40869447..312ea5ec2d7d 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -46,6 +46,7 @@ enum {
 struct smp_chan {
 	struct l2cap_conn	*conn;
 	struct delayed_work	security_timer;
+	struct work_struct	distribute_work;
 
 	u8		preq[7]; /* SMP Pairing Request */
 	u8		prsp[7]; /* SMP Pairing Response */
@@ -656,11 +657,12 @@ static void smp_notify_keys(struct l2cap_conn *conn)
 	}
 }
 
-static int smp_distribute_keys(struct l2cap_conn *conn)
+static void smp_distribute_keys(struct work_struct *work)
 {
+	struct smp_chan *smp = container_of(work, struct smp_chan,
+					    distribute_work);
 	struct smp_cmd_pairing *req, *rsp;
-	struct l2cap_chan *chan = conn->smp;
-	struct smp_chan *smp = chan->data;
+	struct l2cap_conn *conn = smp->conn;
 	struct hci_conn *hcon = conn->hcon;
 	struct hci_dev *hdev = hcon->hdev;
 	__u8 *keydist;
@@ -668,13 +670,13 @@ static int smp_distribute_keys(struct l2cap_conn *conn)
 	BT_DBG("conn %p", conn);
 
 	if (!test_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags))
-		return 0;
+		return;
 
 	rsp = (void *) &smp->prsp[1];
 
 	/* The responder sends its keys first */
 	if (hcon->out && (smp->remote_key_dist & 0x07))
-		return 0;
+		return;
 
 	req = (void *) &smp->preq[1];
 
@@ -760,15 +762,13 @@ static int smp_distribute_keys(struct l2cap_conn *conn)
 
 	/* If there are still keys to be received wait for them */
 	if ((smp->remote_key_dist & 0x07))
-		return 0;
+		return;
 
 	clear_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags);
 	set_bit(SMP_FLAG_COMPLETE, &smp->flags);
 	smp_notify_keys(conn);
 
 	smp_chan_destroy(conn);
-
-	return 0;
 }
 
 static void smp_timeout(struct work_struct *work)
@@ -804,6 +804,7 @@ static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)
 	smp->conn = conn;
 	chan->data = smp;
 
+	INIT_WORK(&smp->distribute_work, smp_distribute_keys);
 	INIT_DELAYED_WORK(&smp->security_timer, smp_timeout);
 
 	hci_conn_hold(conn->hcon);
@@ -824,6 +825,12 @@ void smp_chan_destroy(struct l2cap_conn *conn)
 	if (!chan->data)
 		return;
 
+	if (work_pending(&smp->distribute_work)) {
+		cancel_work_sync(&smp->distribute_work);
+		if (!chan->data)
+			return;
+	}
+
 	complete = test_bit(SMP_FLAG_COMPLETE, &smp->flags);
 	mgmt_smp_complete(conn->hcon, complete);
 
@@ -1287,7 +1294,7 @@ static int smp_cmd_master_ident(struct l2cap_conn *conn, struct sk_buff *skb)
 			  rp->ediv, rp->rand);
 	smp->ltk = ltk;
 	if (!(smp->remote_key_dist & SMP_DIST_ID_KEY))
-		smp_distribute_keys(conn);
+		queue_work(hdev->workqueue, &smp->distribute_work);
 	hci_dev_unlock(hdev);
 
 	return 0;
@@ -1322,6 +1329,7 @@ static int smp_cmd_ident_addr_info(struct l2cap_conn *conn,
 	struct l2cap_chan *chan = conn->smp;
 	struct smp_chan *smp = chan->data;
 	struct hci_conn *hcon = conn->hcon;
+	struct hci_dev *hdev = hcon->hdev;
 	bdaddr_t rpa;
 
 	BT_DBG("");
@@ -1364,7 +1372,7 @@ static int smp_cmd_ident_addr_info(struct l2cap_conn *conn,
 				      smp->id_addr_type, smp->irk, &rpa);
 
 distribute:
-	smp_distribute_keys(conn);
+	queue_work(hdev->workqueue, &smp->distribute_work);
 
 	hci_dev_unlock(hcon->hdev);
 
@@ -1400,7 +1408,7 @@ static int smp_cmd_sign_info(struct l2cap_conn *conn, struct sk_buff *skb)
 		memcpy(csrk->val, rp->csrk, sizeof(csrk->val));
 	}
 	smp->csrk = csrk;
-	smp_distribute_keys(conn);
+	queue_work(hdev->workqueue, &smp->distribute_work);
 	hci_dev_unlock(hdev);
 
 	return 0;
@@ -1510,7 +1518,6 @@ done:
 static void smp_teardown_cb(struct l2cap_chan *chan, int err)
 {
 	struct l2cap_conn *conn = chan->conn;
-	struct smp_chan *smp = chan->data;
 
 	BT_DBG("chan %p", chan);
 
@@ -1526,14 +1533,17 @@ static void smp_resume_cb(struct l2cap_chan *chan)
 	struct smp_chan *smp = chan->data;
 	struct l2cap_conn *conn = chan->conn;
 	struct hci_conn *hcon = conn->hcon;
+	struct hci_dev *hdev = hcon->hdev;
 
 	BT_DBG("chan %p", chan);
 
-	if (test_bit(HCI_CONN_ENCRYPT, &hcon->flags))
-		smp_distribute_keys(conn);
+	if (!smp)
+		return;
 
-	if (smp)
-		cancel_delayed_work(&smp->security_timer);
+	cancel_delayed_work(&smp->security_timer);
+
+	if (test_bit(HCI_CONN_ENCRYPT, &hcon->flags))
+		queue_work(hdev->workqueue, &smp->distribute_work);
 }
 
 static void smp_ready_cb(struct l2cap_chan *chan)
-- 
1.9.3

[PATCH v2 9/9] Bluetooth: Make smp_chan_destroy() private to smp.c

[johan.hedberg]

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

There are no external users of smp_chan_destroy() so make it private to
smp.c. The patch also moves the function higher up in the c-file in
order to avoid forward declarations.

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

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 312ea5ec2d7d..07ca4ce0943b 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -366,6 +366,56 @@ static u8 check_enc_key_size(struct l2cap_conn *conn, __u8 max_key_size)
 	return 0;
 }
 
+static void smp_chan_destroy(struct l2cap_conn *conn)
+{
+	struct l2cap_chan *chan = conn->smp;
+	struct smp_chan *smp = chan->data;
+	bool complete;
+
+	BUG_ON(!smp);
+
+	cancel_delayed_work_sync(&smp->security_timer);
+	/* In case the timeout freed the SMP context */
+	if (!chan->data)
+		return;
+
+	if (work_pending(&smp->distribute_work)) {
+		cancel_work_sync(&smp->distribute_work);
+		if (!chan->data)
+			return;
+	}
+
+	complete = test_bit(SMP_FLAG_COMPLETE, &smp->flags);
+	mgmt_smp_complete(conn->hcon, complete);
+
+	kfree(smp->csrk);
+	kfree(smp->slave_csrk);
+
+	crypto_free_blkcipher(smp->tfm_aes);
+
+	/* If pairing failed clean up any keys we might have */
+	if (!complete) {
+		if (smp->ltk) {
+			list_del(&smp->ltk->list);
+			kfree(smp->ltk);
+		}
+
+		if (smp->slave_ltk) {
+			list_del(&smp->slave_ltk->list);
+			kfree(smp->slave_ltk);
+		}
+
+		if (smp->remote_irk) {
+			list_del(&smp->remote_irk->list);
+			kfree(smp->remote_irk);
+		}
+	}
+
+	chan->data = NULL;
+	kfree(smp);
+	hci_conn_drop(conn->hcon);
+}
+
 static void smp_failure(struct l2cap_conn *conn, u8 reason)
 {
 	struct hci_conn *hcon = conn->hcon;
@@ -812,56 +862,6 @@ static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)
 	return smp;
 }
 
-void smp_chan_destroy(struct l2cap_conn *conn)
-{
-	struct l2cap_chan *chan = conn->smp;
-	struct smp_chan *smp = chan->data;
-	bool complete;
-
-	BUG_ON(!smp);
-
-	cancel_delayed_work_sync(&smp->security_timer);
-	/* In case the timeout freed the SMP context */
-	if (!chan->data)
-		return;
-
-	if (work_pending(&smp->distribute_work)) {
-		cancel_work_sync(&smp->distribute_work);
-		if (!chan->data)
-			return;
-	}
-
-	complete = test_bit(SMP_FLAG_COMPLETE, &smp->flags);
-	mgmt_smp_complete(conn->hcon, complete);
-
-	kfree(smp->csrk);
-	kfree(smp->slave_csrk);
-
-	crypto_free_blkcipher(smp->tfm_aes);
-
-	/* If pairing failed clean up any keys we might have */
-	if (!complete) {
-		if (smp->ltk) {
-			list_del(&smp->ltk->list);
-			kfree(smp->ltk);
-		}
-
-		if (smp->slave_ltk) {
-			list_del(&smp->slave_ltk->list);
-			kfree(smp->slave_ltk);
-		}
-
-		if (smp->remote_irk) {
-			list_del(&smp->remote_irk->list);
-			kfree(smp->remote_irk);
-		}
-	}
-
-	chan->data = NULL;
-	kfree(smp);
-	hci_conn_drop(conn->hcon);
-}
-
 int smp_user_confirm_reply(struct hci_conn *hcon, u16 mgmt_op, __le32 passkey)
 {
 	struct l2cap_conn *conn = hcon->l2cap_data;
diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
index 59a594278a85..cf1094617c69 100644
--- a/net/bluetooth/smp.h
+++ b/net/bluetooth/smp.h
@@ -128,8 +128,6 @@ bool smp_sufficient_security(struct hci_conn *hcon, u8 sec_level);
 int smp_conn_security(struct hci_conn *hcon, __u8 sec_level);
 int smp_user_confirm_reply(struct hci_conn *conn, u16 mgmt_op, __le32 passkey);
 
-void smp_chan_destroy(struct l2cap_conn *conn);
-
 bool smp_irk_matches(struct hci_dev *hdev, u8 irk[16], bdaddr_t *bdaddr);
 int smp_generate_rpa(struct hci_dev *hdev, u8 irk[16], bdaddr_t *rpa);
 
-- 
1.9.3

Re: [PATCH v2 0/9] Bluetooth: SMP fixes & cleanups

[Marcel Holtmann]

Hi Johan,

> Here's a v2 of all my earlier three patch sets combined. The main change
> is renaming l2cap_conn_drop to l2cap_conn_shutdown in order to not be
> confused with the slightly different semantics of hci_conn_drop.
> 
> Johan
> 
> ----------------------------------------------------------------
> Johan Hedberg (9):
>      Bluetooth: Use L2CAP resume callback to call smp_distribute_keys
>      Bluetooth: Add public l2cap_conn_shutdown() API to request disconnection
>      Bluetooth: Call l2cap_conn_shutdown() when SMP recv callback fails
>      Bluetooth: Fix double free of SMP data skb
>      Bluetooth: Add SMP-internal timeout callback
>      Bluetooth: Remove unused l2cap_conn->security_timer
>      Bluetooth: Move canceling security_timer into smp_chan_destroy()
>      Bluetooth: Always call smp_distribute_keys() from a workqueue
>      Bluetooth: Make smp_chan_destroy() private to smp.c
> 
> include/net/bluetooth/l2cap.h |   5 +-
> net/bluetooth/l2cap_core.c    |  35 +--
> net/bluetooth/smp.c           | 541 +++++++++++++++++++++-----------------
> net/bluetooth/smp.h           |   3 -
> 4 files changed, 330 insertions(+), 254 deletions(-)

all 9 patches have been applied to bluetooth-next tree.

Regards

Marcel