Skip to contentSkip to conversation

Bluetooth: SMP cleanups

PATCH In review [email protected] 2 participants 8 messages May 20, 2014

Subsystems (2)
Bluetooth drivers Bluetooth
lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover Bluetooth: SMP cleanups

Hi,

Here's a set of cleanups for the SMP code.

Johan

----------------------------------------------------------------
Johan Hedberg (6):
Bluetooth: Make SMP context private to smp.c
Bluetooth: Fix setting initial local auth_req value
Bluetooth: Remove unnecessary work structs from SMP code
Bluetooth: Rename smp->smp_flags to smp->flags
Bluetooth: Update smp_random to return a response code
Bluetooth: Update smp_confirm to return a response code

 net/bluetooth/hci_conn.c |   1 +
 net/bluetooth/hci_core.c |   1 +
 net/bluetooth/mgmt.c     |   1 +
 net/bluetooth/smp.c      | 133 ++++++++++++++++++++-------------------
 net/bluetooth/smp.h      |  29 ---------
 5 files changed, 70 insertions(+), 95 deletions(-)
author
#1 lore raw

PATCH 1/6 Bluetooth: Make SMP context private to smp.c

From: Johan Hedberg <[email protected]>

There are no users of the smp_chan struct outside of smp.c so move it away from smp.h. The addition of the l2cap.h include to hci_core.c, hci_conn.c and mgmt.c is something that should have been there already previously to avoid warnings of undeclared struct l2cap_conn, but the compiler warning was apparently shadowed away by the mention of l2cap_conn in the struct smp_chan definition.

Signed-off-by Johan Hedberg <[email protected]>
---
 net/bluetooth/hci_conn.c |  1 +
 net/bluetooth/hci_core.c |  1 +
 net/bluetooth/mgmt.c     |  1 +
 net/bluetooth/smp.c      | 29 +++++++++++++++++++++++++++++
 net/bluetooth/smp.h      | 29 -----------------------------
 5 files changed, 32 insertions(+), 29 deletions(-)
net/bluetooth/hci_conn.c +1 -0
index a987e7def025..97198e32f0c4 100644
@@ -27,6 +27,7 @@
 #include <linux/export.h>
 
 #include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/l2cap.h>
 #include <net/bluetooth/hci_core.h>
 
 #include "smp.h"
net/bluetooth/hci_core.c +1 -0
index 313bd1c164d6..b748effc6aea 100644
@@ -33,6 +33,7 @@
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/l2cap.h>
 #include <net/bluetooth/hci_core.h>
 
 #include "smp.h"
net/bluetooth/mgmt.c +1 -0
index 370cd42f488b..f8ca69dd1984 100644
@@ -29,6 +29,7 @@
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
+#include <net/bluetooth/l2cap.h>
 #include <net/bluetooth/mgmt.h>
 
 #include "smp.h"
net/bluetooth/smp.c +29 -0
index b5b926399e76..df91ed28084f 100644
@@ -35,6 +35,35 @@
 
 #define AUTH_REQ_MASK   0x07
 
+#define SMP_FLAG_TK_VALID	1
+#define SMP_FLAG_CFM_PENDING	2
+#define SMP_FLAG_MITM_AUTH	3
+#define SMP_FLAG_COMPLETE	4
+#define SMP_FLAG_INITIATOR	5
+
+struct smp_chan {
+	struct l2cap_conn *conn;
+	u8		preq[7]; /* SMP Pairing Request */
+	u8		prsp[7]; /* SMP Pairing Response */
+	u8		prnd[16]; /* SMP Pairing Random (local) */
+	u8		rrnd[16]; /* SMP Pairing Random (remote) */
+	u8		pcnf[16]; /* SMP Pairing Confirm */
+	u8		tk[16]; /* SMP Temporary Key */
+	u8		enc_key_size;
+	u8		remote_key_dist;
+	bdaddr_t	id_addr;
+	u8		id_addr_type;
+	u8		irk[16];
+	struct smp_csrk	*csrk;
+	struct smp_csrk	*slave_csrk;
+	struct smp_ltk	*ltk;
+	struct smp_ltk	*slave_ltk;
+	struct smp_irk	*remote_irk;
+	unsigned long	smp_flags;
+	struct work_struct confirm;
+	struct work_struct random;
+};
+
 static inline void swap128(const u8 src[16], u8 dst[16])
 {
 	int i;
net/bluetooth/smp.h +0 -29
index afd16231db13..5a8dc36460a1 100644
@@ -116,35 +116,6 @@ struct smp_cmd_security_req {
 #define SMP_MIN_ENC_KEY_SIZE		7
 #define SMP_MAX_ENC_KEY_SIZE		16
 
-#define SMP_FLAG_TK_VALID	1
-#define SMP_FLAG_CFM_PENDING	2
-#define SMP_FLAG_MITM_AUTH	3
-#define SMP_FLAG_COMPLETE	4
-#define SMP_FLAG_INITIATOR	5
-
-struct smp_chan {
-	struct l2cap_conn *conn;
-	u8		preq[7]; /* SMP Pairing Request */
-	u8		prsp[7]; /* SMP Pairing Response */
-	u8		prnd[16]; /* SMP Pairing Random (local) */
-	u8		rrnd[16]; /* SMP Pairing Random (remote) */
-	u8		pcnf[16]; /* SMP Pairing Confirm */
-	u8		tk[16]; /* SMP Temporary Key */
-	u8		enc_key_size;
-	u8		remote_key_dist;
-	bdaddr_t	id_addr;
-	u8		id_addr_type;
-	u8		irk[16];
-	struct smp_csrk	*csrk;
-	struct smp_csrk	*slave_csrk;
-	struct smp_ltk	*ltk;
-	struct smp_ltk	*slave_ltk;
-	struct smp_irk	*remote_irk;
-	unsigned long	smp_flags;
-	struct work_struct confirm;
-	struct work_struct random;
-};
-
 /* SMP Commands */
 bool smp_sufficient_security(struct hci_conn *hcon, u8 sec_level);
 int smp_conn_security(struct hci_conn *hcon, __u8 sec_level);
Signature · 1 line
1.9.0
author
#2 lore raw

PATCH 2/6 Bluetooth: Fix setting initial local auth_req value

In reply to [email protected]

From: Johan Hedberg <[email protected]>

There is no reason to have the initial local value conditional to whether the remote value has bonding set or not. We can either way start off with the value we received.

Signed-off-by Johan Hedberg <[email protected]>
---
 net/bluetooth/smp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
net/bluetooth/smp.c +2 -4
index df91ed28084f..72743f87b22f 100644
@@ -685,8 +685,7 @@ static u8 smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
 {
 	struct smp_cmd_pairing rsp, *req = (void *) skb->data;
 	struct smp_chan *smp;
-	u8 key_size;
-	u8 auth = SMP_AUTH_NONE;
+	u8 key_size, auth;
 	int ret;
 
 	BT_DBG("conn %p", conn);

@@ -710,8 +709,7 @@ static u8 smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
 	skb_pull(skb, sizeof(*req));
 
 	/* We didn't start the pairing, so match remote */
-	if (req->auth_req & SMP_AUTH_BONDING)
-		auth = req->auth_req;
+	auth = req->auth_req;
 
 	conn->hcon->pending_sec_level = authreq_to_seclevel(auth);
Signature · 1 line
1.9.0
author
#3 lore raw

PATCH 3/6 Bluetooth: Remove unnecessary work structs from SMP code

In reply to [email protected]

From: Johan Hedberg <[email protected]>

When the SMP code was initially created (mid-2011) parts of the Bluetooth subsystem were still not converted to use workqueues. This meant that the crypto calls, which could sleep, couldn't be called directly. Because of this the "confirm" and "random" work structs were introduced.

These days the entire Bluetooth subsystem runs through workqueues which makes these structs unnecessary. This patch removes them and converts the calls to queue them to use direct function calls instead.

Signed-off-by Johan Hedberg <[email protected]>
---
 net/bluetooth/smp.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)
net/bluetooth/smp.c +6 -16
index 72743f87b22f..efb73fdf710d 100644
@@ -60,8 +60,6 @@ struct smp_chan {
 	struct smp_ltk	*slave_ltk;
 	struct smp_irk	*remote_irk;
 	unsigned long	smp_flags;
-	struct work_struct confirm;
-	struct work_struct random;
 };
 
 static inline void swap128(const u8 src[16], u8 dst[16])

@@ -470,9 +468,8 @@ static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth,
 	return ret;
 }
 
-static void confirm_work(struct work_struct *work)
+static void smp_confirm(struct smp_chan *smp)
 {
-	struct smp_chan *smp = container_of(work, struct smp_chan, confirm);
 	struct l2cap_conn *conn = smp->conn;
 	struct hci_dev *hdev = conn->hcon->hdev;
 	struct crypto_blkcipher *tfm = hdev->tfm_aes;

@@ -507,9 +504,8 @@ error:
 	smp_failure(conn, reason);
 }
 
-static void random_work(struct work_struct *work)
+static void smp_random(struct smp_chan *smp)
 {
-	struct smp_chan *smp = container_of(work, struct smp_chan, random);
 	struct l2cap_conn *conn = smp->conn;
 	struct hci_conn *hcon = conn->hcon;
 	struct hci_dev *hdev = hcon->hdev;

@@ -593,9 +589,6 @@ static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)
 	if (!smp)
 		return NULL;
 
-	INIT_WORK(&smp->confirm, confirm_work);
-	INIT_WORK(&smp->random, random_work);
-
 	smp->conn = conn;
 	conn->smp_chan = smp;
 	conn->hcon->smp_conn = conn;

@@ -676,7 +669,7 @@ int smp_user_confirm_reply(struct hci_conn *hcon, u16 mgmt_op, __le32 passkey)
 
 	/* If it is our turn to send Pairing Confirm, do so now */
 	if (test_bit(SMP_FLAG_CFM_PENDING, &smp->smp_flags))
-		queue_work(hcon->hdev->workqueue, &smp->confirm);
+		smp_confirm(smp);
 
 	return 0;
 }

@@ -740,7 +733,6 @@ static u8 smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
 {
 	struct smp_cmd_pairing *req, *rsp = (void *) skb->data;
 	struct smp_chan *smp = conn->smp_chan;
-	struct hci_dev *hdev = conn->hcon->hdev;
 	u8 key_size, auth = SMP_AUTH_NONE;
 	int ret;
 

@@ -784,7 +776,7 @@ static u8 smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
 
 	/* Can't compose response until we have been confirmed */
 	if (test_bit(SMP_FLAG_TK_VALID, &smp->smp_flags))
-		queue_work(hdev->workqueue, &smp->confirm);
+		smp_confirm(smp);
 
 	return 0;
 }

@@ -792,7 +784,6 @@ static u8 smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
 static u8 smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
 {
 	struct smp_chan *smp = conn->smp_chan;
-	struct hci_dev *hdev = conn->hcon->hdev;
 
 	BT_DBG("conn %p %s", conn, conn->hcon->out ? "master" : "slave");
 

@@ -806,7 +797,7 @@ static u8 smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
 		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(smp->prnd),
 			     smp->prnd);
 	else if (test_bit(SMP_FLAG_TK_VALID, &smp->smp_flags))
-		queue_work(hdev->workqueue, &smp->confirm);
+		smp_confirm(smp);
 	else
 		set_bit(SMP_FLAG_CFM_PENDING, &smp->smp_flags);
 

@@ -816,7 +807,6 @@ static u8 smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
 static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 {
 	struct smp_chan *smp = conn->smp_chan;
-	struct hci_dev *hdev = conn->hcon->hdev;
 
 	BT_DBG("conn %p", conn);
 

@@ -826,7 +816,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 	memcpy(smp->rrnd, skb->data, sizeof(smp->rrnd));
 	skb_pull(skb, sizeof(smp->rrnd));
 
-	queue_work(hdev->workqueue, &smp->random);
+	smp_random(smp);
 
 	return 0;
 }
Signature · 1 line
1.9.0
author
#4 lore raw

PATCH 4/6 Bluetooth: Rename smp->smp_flags to smp->flags

In reply to [email protected]

From: Johan Hedberg <[email protected]>

There's no reason to have "smp" in this variable name since it is already part of the SMP struct which provides sufficient context.

Signed-off-by Johan Hedberg <[email protected]>
---
 net/bluetooth/smp.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)
net/bluetooth/smp.c +18 -19
index efb73fdf710d..8d1a1903e458 100644
@@ -59,7 +59,7 @@ struct smp_chan {
 	struct smp_ltk	*ltk;
 	struct smp_ltk	*slave_ltk;
 	struct smp_irk	*remote_irk;
-	unsigned long	smp_flags;
+	unsigned long	flags;
 };
 
 static inline void swap128(const u8 src[16], u8 dst[16])

@@ -396,7 +396,7 @@ static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth,
 
 	/* Initialize key for JUST WORKS */
 	memset(smp->tk, 0, sizeof(smp->tk));
-	clear_bit(SMP_FLAG_TK_VALID, &smp->smp_flags);
+	clear_bit(SMP_FLAG_TK_VALID, &smp->flags);
 
 	BT_DBG("tk_request: auth:%d lcl:%d rem:%d", auth, local_io, remote_io);
 

@@ -415,19 +415,18 @@ static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth,
 		method = JUST_WORKS;
 
 	/* Don't confirm locally initiated pairing attempts */
-	if (method == JUST_CFM && test_bit(SMP_FLAG_INITIATOR,
-					   &smp->smp_flags))
+	if (method == JUST_CFM && test_bit(SMP_FLAG_INITIATOR, &smp->flags))
 		method = JUST_WORKS;
 
 	/* If Just Works, Continue with Zero TK */
 	if (method == JUST_WORKS) {
-		set_bit(SMP_FLAG_TK_VALID, &smp->smp_flags);
+		set_bit(SMP_FLAG_TK_VALID, &smp->flags);
 		return 0;
 	}
 
 	/* Not Just Works/Confirm results in MITM Authentication */
 	if (method != JUST_CFM)
-		set_bit(SMP_FLAG_MITM_AUTH, &smp->smp_flags);
+		set_bit(SMP_FLAG_MITM_AUTH, &smp->flags);
 
 	/* If both devices have Keyoard-Display I/O, the master
 	 * Confirms and the slave Enters the passkey.

@@ -446,7 +445,7 @@ static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth,
 		passkey %= 1000000;
 		put_unaligned_le32(passkey, smp->tk);
 		BT_DBG("PassKey: %d", passkey);
-		set_bit(SMP_FLAG_TK_VALID, &smp->smp_flags);
+		set_bit(SMP_FLAG_TK_VALID, &smp->flags);
 	}
 
 	hci_dev_lock(hcon->hdev);

@@ -494,7 +493,7 @@ static void smp_confirm(struct smp_chan *smp)
 		goto error;
 	}
 
-	clear_bit(SMP_FLAG_CFM_PENDING, &smp->smp_flags);
+	clear_bit(SMP_FLAG_CFM_PENDING, &smp->flags);
 
 	smp_send_cmd(smp->conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
 

@@ -605,7 +604,7 @@ void smp_chan_destroy(struct l2cap_conn *conn)
 
 	BUG_ON(!smp);
 
-	complete = test_bit(SMP_FLAG_COMPLETE, &smp->smp_flags);
+	complete = test_bit(SMP_FLAG_COMPLETE, &smp->flags);
 	mgmt_smp_complete(conn->hcon, complete);
 
 	kfree(smp->csrk);

@@ -656,7 +655,7 @@ int smp_user_confirm_reply(struct hci_conn *hcon, u16 mgmt_op, __le32 passkey)
 		put_unaligned_le32(value, smp->tk);
 		/* Fall Through */
 	case MGMT_OP_USER_CONFIRM_REPLY:
-		set_bit(SMP_FLAG_TK_VALID, &smp->smp_flags);
+		set_bit(SMP_FLAG_TK_VALID, &smp->flags);
 		break;
 	case MGMT_OP_USER_PASSKEY_NEG_REPLY:
 	case MGMT_OP_USER_CONFIRM_NEG_REPLY:

@@ -668,7 +667,7 @@ int smp_user_confirm_reply(struct hci_conn *hcon, u16 mgmt_op, __le32 passkey)
 	}
 
 	/* If it is our turn to send Pairing Confirm, do so now */
-	if (test_bit(SMP_FLAG_CFM_PENDING, &smp->smp_flags))
+	if (test_bit(SMP_FLAG_CFM_PENDING, &smp->flags))
 		smp_confirm(smp);
 
 	return 0;

@@ -724,7 +723,7 @@ static u8 smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
 	if (ret)
 		return SMP_UNSPECIFIED;
 
-	clear_bit(SMP_FLAG_INITIATOR, &smp->smp_flags);
+	clear_bit(SMP_FLAG_INITIATOR, &smp->flags);
 
 	return 0;
 }

@@ -772,10 +771,10 @@ static u8 smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
 	if (ret)
 		return SMP_UNSPECIFIED;
 
-	set_bit(SMP_FLAG_CFM_PENDING, &smp->smp_flags);
+	set_bit(SMP_FLAG_CFM_PENDING, &smp->flags);
 
 	/* Can't compose response until we have been confirmed */
-	if (test_bit(SMP_FLAG_TK_VALID, &smp->smp_flags))
+	if (test_bit(SMP_FLAG_TK_VALID, &smp->flags))
 		smp_confirm(smp);
 
 	return 0;

@@ -796,10 +795,10 @@ static u8 smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
 	if (conn->hcon->out)
 		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(smp->prnd),
 			     smp->prnd);
-	else if (test_bit(SMP_FLAG_TK_VALID, &smp->smp_flags))
+	else if (test_bit(SMP_FLAG_TK_VALID, &smp->flags))
 		smp_confirm(smp);
 	else
-		set_bit(SMP_FLAG_CFM_PENDING, &smp->smp_flags);
+		set_bit(SMP_FLAG_CFM_PENDING, &smp->flags);
 
 	return 0;
 }

@@ -878,7 +877,7 @@ static u8 smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
 
 	smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
 
-	clear_bit(SMP_FLAG_INITIATOR, &smp->smp_flags);
+	clear_bit(SMP_FLAG_INITIATOR, &smp->flags);
 
 	return 0;
 }

@@ -945,7 +944,7 @@ int smp_conn_security(struct hci_conn *hcon, __u8 sec_level)
 		smp_send_cmd(conn, SMP_CMD_SECURITY_REQ, sizeof(cp), &cp);
 	}
 
-	set_bit(SMP_FLAG_INITIATOR, &smp->smp_flags);
+	set_bit(SMP_FLAG_INITIATOR, &smp->flags);
 
 done:
 	hcon->pending_sec_level = sec_level;

@@ -1375,7 +1374,7 @@ int smp_distribute_keys(struct l2cap_conn *conn)
 
 	clear_bit(HCI_CONN_LE_SMP_PEND, &hcon->flags);
 	cancel_delayed_work_sync(&conn->security_timer);
-	set_bit(SMP_FLAG_COMPLETE, &smp->smp_flags);
+	set_bit(SMP_FLAG_COMPLETE, &smp->flags);
 	smp_notify_keys(conn);
 
 	smp_chan_destroy(conn);
Signature · 1 line
1.9.0
author
#5 lore raw

PATCH 5/6 Bluetooth: Update smp_random to return a response code

In reply to [email protected]

From: Johan Hedberg <[email protected]>

Since we're now calling smp_random() "inline" we can have it directly return a response code and have the shared command handler send the response.

Signed-off-by Johan Hedberg <[email protected]>
---
 net/bluetooth/smp.c | 34 +++++++++++-----------------------
 1 file changed, 11 insertions(+), 23 deletions(-)
net/bluetooth/smp.c +11 -23
index 8d1a1903e458..fa782d7b495b 100644
@@ -503,19 +503,17 @@ error:
 	smp_failure(conn, reason);
 }
 
-static void smp_random(struct smp_chan *smp)
+static u8 smp_random(struct smp_chan *smp)
 {
 	struct l2cap_conn *conn = smp->conn;
 	struct hci_conn *hcon = conn->hcon;
 	struct hci_dev *hdev = hcon->hdev;
 	struct crypto_blkcipher *tfm = hdev->tfm_aes;
-	u8 reason, confirm[16];
+	u8 confirm[16];
 	int ret;
 
-	if (IS_ERR_OR_NULL(tfm)) {
-		reason = SMP_UNSPECIFIED;
-		goto error;
-	}
+	if (IS_ERR_OR_NULL(tfm))
+		return SMP_UNSPECIFIED;
 
 	BT_DBG("conn %p %s", conn, conn->hcon->out ? "master" : "slave");
 

@@ -528,15 +526,12 @@ static void smp_random(struct smp_chan *smp)
 
 	hci_dev_unlock(hdev);
 
-	if (ret) {
-		reason = SMP_UNSPECIFIED;
-		goto error;
-	}
+	if (ret)
+		return SMP_UNSPECIFIED;
 
 	if (memcmp(smp->pcnf, confirm, sizeof(smp->pcnf)) != 0) {
 		BT_ERR("Pairing failed (confirmation values mismatch)");
-		reason = SMP_CONFIRM_FAILED;
-		goto error;
+		return SMP_CONFIRM_FAILED;
 	}
 
 	if (hcon->out) {

@@ -549,10 +544,8 @@ static void smp_random(struct smp_chan *smp)
 		memset(stk + smp->enc_key_size, 0,
 		       SMP_MAX_ENC_KEY_SIZE - smp->enc_key_size);
 
-		if (test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &hcon->flags)) {
-			reason = SMP_UNSPECIFIED;
-			goto error;
-		}
+		if (test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &hcon->flags))
+			return SMP_UNSPECIFIED;
 
 		hci_le_start_enc(hcon, ediv, rand, stk);
 		hcon->enc_key_size = smp->enc_key_size;

@@ -574,10 +567,7 @@ static void smp_random(struct smp_chan *smp)
 			    ediv, rand);
 	}
 
-	return;
-
-error:
-	smp_failure(conn, reason);
+	return 0;
 }
 
 static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)

@@ -815,9 +805,7 @@ static u8 smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
 	memcpy(smp->rrnd, skb->data, sizeof(smp->rrnd));
 	skb_pull(skb, sizeof(smp->rrnd));
 
-	smp_random(smp);
-
-	return 0;
+	return smp_random(smp);
 }
 
 static u8 smp_ltk_encrypt(struct l2cap_conn *conn, u8 sec_level)
Signature · 1 line
1.9.0
author
#6 lore raw

PATCH 6/6 Bluetooth: Update smp_confirm to return a response code

In reply to [email protected]

From: Johan Hedberg <[email protected]>

Now that smp_confirm() is called "inline" we can have it return a response code and have the sending of it be done in the shared place for command handlers. One exception is when we're entering smp.c from mgmt.c when user space responds to authentication, in which case we still need our own code to call smp_failuer().

Signed-off-by Johan Hedberg <[email protected]>
---
 net/bluetooth/smp.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)
net/bluetooth/smp.c +11 -14
index fa782d7b495b..4f9662d0fd81 100644
@@ -467,14 +467,13 @@ static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth,
 	return ret;
 }
 
-static void smp_confirm(struct smp_chan *smp)
+static u8 smp_confirm(struct smp_chan *smp)
 {
 	struct l2cap_conn *conn = smp->conn;
 	struct hci_dev *hdev = conn->hcon->hdev;
 	struct crypto_blkcipher *tfm = hdev->tfm_aes;
 	struct smp_cmd_pairing_confirm cp;
 	int ret;
-	u8 reason;
 
 	BT_DBG("conn %p", conn);
 

@@ -488,19 +487,14 @@ static void smp_confirm(struct smp_chan *smp)
 
 	hci_dev_unlock(hdev);
 
-	if (ret) {
-		reason = SMP_UNSPECIFIED;
-		goto error;
-	}
+	if (ret)
+		return SMP_UNSPECIFIED;
 
 	clear_bit(SMP_FLAG_CFM_PENDING, &smp->flags);
 
 	smp_send_cmd(smp->conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
 
-	return;
-
-error:
-	smp_failure(conn, reason);
+	return 0;
 }
 
 static u8 smp_random(struct smp_chan *smp)

@@ -657,8 +651,11 @@ int smp_user_confirm_reply(struct hci_conn *hcon, u16 mgmt_op, __le32 passkey)
 	}
 
 	/* If it is our turn to send Pairing Confirm, do so now */
-	if (test_bit(SMP_FLAG_CFM_PENDING, &smp->flags))
-		smp_confirm(smp);
+	if (test_bit(SMP_FLAG_CFM_PENDING, &smp->flags)) {
+		u8 rsp = smp_confirm(smp);
+		if (rsp)
+			smp_failure(conn, rsp);
+	}
 
 	return 0;
 }

@@ -765,7 +762,7 @@ static u8 smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
 
 	/* Can't compose response until we have been confirmed */
 	if (test_bit(SMP_FLAG_TK_VALID, &smp->flags))
-		smp_confirm(smp);
+		return smp_confirm(smp);
 
 	return 0;
 }

@@ -786,7 +783,7 @@ static u8 smp_cmd_pairing_confirm(struct l2cap_conn *conn, struct sk_buff *skb)
 		smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(smp->prnd),
 			     smp->prnd);
 	else if (test_bit(SMP_FLAG_TK_VALID, &smp->flags))
-		smp_confirm(smp);
+		return smp_confirm(smp);
 	else
 		set_bit(SMP_FLAG_CFM_PENDING, &smp->flags);
Signature · 1 line
1.9.0
maintainer
#7 lore raw
In reply to [email protected]

Hi Johan,

Quoted 13 lines

Here's a set of cleanups for the SMP code.

Johan

----------------------------------------------------------------
Johan Hedberg (6):
Bluetooth: Make SMP context private to smp.c
Bluetooth: Fix setting initial local auth_req value
Bluetooth: Remove unnecessary work structs from SMP code
Bluetooth: Rename smp->smp_flags to smp->flags
Bluetooth: Update smp_random to return a response code
Bluetooth: Update smp_confirm to return a response code

Quoted 6 lines
net/bluetooth/hci_conn.c |   1 +
net/bluetooth/hci_core.c |   1 +
net/bluetooth/mgmt.c     |   1 +
net/bluetooth/smp.c      | 133 ++++++++++++++++++++-------------------
net/bluetooth/smp.h      |  29 ---------
5 files changed, 70 insertions(+), 95 deletions(-)

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

Regards

Marcel