[PATCH 00/10] Bluetooth: Fix mgmt cmd_complete events

[PATCH 00/10] Bluetooth: Fix mgmt cmd_complete events

[Johan Hedberg]

Hi,

This patch set introduces an optional callback that can be set for
pending mgmt commands so that the generic places sending out these
events (powering off and removing adapters) can send out correct events
as specified in our mgmt-api.txt. A further benefit of having this
callback is that we fix memory leaks caused by hci_conn pointers being
stored as part of the pending command user data.

Johan

----------------------------------------------------------------
Johan Hedberg (10):
      Bluetooth: Add callback to create proper cmd_complete events
      Bluetooth: Store parameter length with pending mgmt commands
      Bluetooth: Convert Disconnect mgmt command to use cmd_complete callback
      Bluetooth: Use cmd_complete callback for authentication mgmt commands
      Bluetooth: Convert Pair Device to use cmd_complete callback
      Bluetooth: Convert Unpair Device to use cmd_complete callback
      Bluetooth: Convert discovery commands to use cmd_complete callback
      Bluetooth: Convert Get Clock Info to use cmd_complete callback
      Bluetooth: Fix initializing hci_conn RSSI to invalid value
      Bluetooth: Fix Get Conn Info to use cmd_complete callback

 net/bluetooth/hci_conn.c |   1 +
 net/bluetooth/mgmt.c     | 271 ++++++++++++++++++++++-----------------------
 2 files changed, 136 insertions(+), 136 deletions(-)

[PATCH 01/10] Bluetooth: Add callback to create proper cmd_complete events

[Johan Hedberg]

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

We've got a couple of generic scenarios where all pending mgmt commands
are processed and responses are sent to them. These scenarios are
powering off the adapter and removing the adapter. So far the code has
been generating cmd_status responses with NOT_POWERED and INVALID_INDEX
resposes respectively, but this violates the mgmt specification for
commands that should always generate a cmd_complete.

This patch adds support for specifying a callback for the pending_cmd
context that each command handler can use for command-specific
cmd_complete event generation. The actual per-command event generators
will come in subsequent patches.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 74571a4b85ec..98537b07b720 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -137,6 +137,7 @@ struct pending_cmd {
 	void *param;
 	struct sock *sk;
 	void *user_data;
+	void (*cmd_complete)(struct pending_cmd *cmd, u8 status);
 };
 
 /* HCI to MGMT error code conversion table */
@@ -1471,6 +1472,20 @@ static void cmd_status_rsp(struct pending_cmd *cmd, void *data)
 	mgmt_pending_remove(cmd);
 }
 
+static void cmd_complete_rsp(struct pending_cmd *cmd, void *data)
+{
+	if (cmd->cmd_complete) {
+		u8 *status = data;
+
+		cmd->cmd_complete(cmd, *status);
+		mgmt_pending_remove(cmd);
+
+		return;
+	}
+
+	cmd_status_rsp(cmd, data);
+}
+
 static u8 mgmt_bredr_support(struct hci_dev *hdev)
 {
 	if (!lmp_bredr_capable(hdev))
@@ -5976,7 +5991,7 @@ void mgmt_index_removed(struct hci_dev *hdev)
 	if (test_bit(HCI_QUIRK_RAW_DEVICE, &hdev->quirks))
 		return;
 
-	mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status);
+	mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &status);
 
 	if (test_bit(HCI_UNCONFIGURED, &hdev->dev_flags))
 		mgmt_event(MGMT_EV_UNCONF_INDEX_REMOVED, hdev, NULL, 0, NULL);
@@ -6111,7 +6126,7 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
 	}
 
 	mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
-	mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status_not_powered);
+	mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &status_not_powered);
 
 	if (memcmp(hdev->dev_class, zero_cod, sizeof(zero_cod)) != 0)
 		mgmt_event(MGMT_EV_CLASS_OF_DEV_CHANGED, hdev,
-- 
2.1.0

[PATCH 02/10] Bluetooth: Store parameter length with pending mgmt commands

[Johan Hedberg]

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

As preparation for making generic cmd_complete responses possible we'll
need to track the parameter length in addition to just a pointer to
them. This patch adds the necessary variable to the pending_cmd struct.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 98537b07b720..56c7838c0a41 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -135,6 +135,7 @@ struct pending_cmd {
 	u16 opcode;
 	int index;
 	void *param;
+	size_t param_len;
 	struct sock *sk;
 	void *user_data;
 	void (*cmd_complete)(struct pending_cmd *cmd, u8 status);
@@ -1205,14 +1206,13 @@ static struct pending_cmd *mgmt_pending_add(struct sock *sk, u16 opcode,
 	cmd->opcode = opcode;
 	cmd->index = hdev->id;
 
-	cmd->param = kmalloc(len, GFP_KERNEL);
+	cmd->param = kmemdup(data, len, GFP_KERNEL);
 	if (!cmd->param) {
 		kfree(cmd);
 		return NULL;
 	}
 
-	if (data)
-		memcpy(cmd->param, data, len);
+	cmd->param_len = len;
 
 	cmd->sk = sk;
 	sock_hold(sk);
-- 
2.1.0

[PATCH 03/10] Bluetooth: Convert Disconnect mgmt command to use cmd_complete callback

[Johan Hedberg]

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

This patch converts the Disconnect mgmt command to take advantage of the
new cmd_complete callback that's part of the pending_cmd struct. There
are many commands whose response parameters map 1:1 to the command
parameters and Disconnect is one of them. This patch adds a
generic_cmd_complete() function for such commands that can be reused in
subsequent patches.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 56c7838c0a41..81b2886f64b8 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1486,6 +1486,12 @@ static void cmd_complete_rsp(struct pending_cmd *cmd, void *data)
 	cmd_status_rsp(cmd, data);
 }
 
+static void generic_cmd_complete(struct pending_cmd *cmd, u8 status)
+{
+	cmd_complete(cmd->sk, cmd->index, cmd->opcode, status, cmd->param,
+		     cmd->param_len);
+}
+
 static u8 mgmt_bredr_support(struct hci_dev *hdev)
 {
 	if (!lmp_bredr_capable(hdev))
@@ -2872,6 +2878,8 @@ static int disconnect(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto failed;
 	}
 
+	cmd->cmd_complete = generic_cmd_complete;
+
 	err = hci_disconnect(conn, HCI_ERROR_REMOTE_USER_TERM);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
@@ -6396,15 +6404,9 @@ void mgmt_device_connected(struct hci_dev *hdev, struct hci_conn *conn,
 
 static void disconnect_rsp(struct pending_cmd *cmd, void *data)
 {
-	struct mgmt_cp_disconnect *cp = cmd->param;
 	struct sock **sk = data;
-	struct mgmt_rp_disconnect rp;
 
-	bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
-	rp.addr.type = cp->addr.type;
-
-	cmd_complete(cmd->sk, cmd->index, MGMT_OP_DISCONNECT, 0, &rp,
-		     sizeof(rp));
+	cmd->cmd_complete(cmd, 0);
 
 	*sk = cmd->sk;
 	sock_hold(*sk);
@@ -6486,7 +6488,6 @@ void mgmt_disconnect_failed(struct hci_dev *hdev, bdaddr_t *bdaddr,
 {
 	u8 bdaddr_type = link_to_bdaddr(link_type, addr_type);
 	struct mgmt_cp_disconnect *cp;
-	struct mgmt_rp_disconnect rp;
 	struct pending_cmd *cmd;
 
 	mgmt_pending_foreach(MGMT_OP_UNPAIR_DEVICE, hdev, unpair_device_rsp,
@@ -6504,12 +6505,7 @@ void mgmt_disconnect_failed(struct hci_dev *hdev, bdaddr_t *bdaddr,
 	if (cp->addr.type != bdaddr_type)
 		return;
 
-	bacpy(&rp.addr.bdaddr, bdaddr);
-	rp.addr.type = bdaddr_type;
-
-	cmd_complete(cmd->sk, cmd->index, MGMT_OP_DISCONNECT,
-		     mgmt_status(status), &rp, sizeof(rp));
-
+	cmd->cmd_complete(cmd, mgmt_status(status));
 	mgmt_pending_remove(cmd);
 }
 
-- 
2.1.0

[PATCH 04/10] Bluetooth: Use cmd_complete callback for authentication mgmt commands

[Johan Hedberg]

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

This patch converts the user confirmation & PIN code mgmt commands to
take advantage of the new cmd_complete callback for pending mgmt
commands. The patch also adds a new generic addr_cmd_complete() helper
function to be used with commands that send a mgmt_addr_info response
based on a mgmt_addr_info in the beginning of the command parameters.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 81b2886f64b8..0fc3d6914ef0 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1492,6 +1492,12 @@ static void generic_cmd_complete(struct pending_cmd *cmd, u8 status)
 		     cmd->param_len);
 }
 
+static void addr_cmd_complete(struct pending_cmd *cmd, u8 status)
+{
+	cmd_complete(cmd->sk, cmd->index, cmd->opcode, status, cmd->param,
+		     sizeof(struct mgmt_addr_info));
+}
+
 static u8 mgmt_bredr_support(struct hci_dev *hdev)
 {
 	if (!lmp_bredr_capable(hdev))
@@ -3032,6 +3038,8 @@ static int pin_code_reply(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto failed;
 	}
 
+	cmd->cmd_complete = addr_cmd_complete;
+
 	bacpy(&reply.bdaddr, &cp->addr.bdaddr);
 	reply.pin_len = cp->pin_len;
 	memcpy(reply.pin_code, cp->pin_code, sizeof(reply.pin_code));
@@ -3363,6 +3371,8 @@ static int user_pairing_resp(struct sock *sk, struct hci_dev *hdev,
 		goto done;
 	}
 
+	cmd->cmd_complete = addr_cmd_complete;
+
 	/* Continue with pairing via HCI */
 	if (hci_op == HCI_OP_USER_PASSKEY_REPLY) {
 		struct hci_cp_user_passkey_reply cp;
@@ -6544,18 +6554,12 @@ void mgmt_pin_code_reply_complete(struct hci_dev *hdev, bdaddr_t *bdaddr,
 				  u8 status)
 {
 	struct pending_cmd *cmd;
-	struct mgmt_rp_pin_code_reply rp;
 
 	cmd = mgmt_pending_find(MGMT_OP_PIN_CODE_REPLY, hdev);
 	if (!cmd)
 		return;
 
-	bacpy(&rp.addr.bdaddr, bdaddr);
-	rp.addr.type = BDADDR_BREDR;
-
-	cmd_complete(cmd->sk, hdev->id, MGMT_OP_PIN_CODE_REPLY,
-		     mgmt_status(status), &rp, sizeof(rp));
-
+	cmd->cmd_complete(cmd, mgmt_status(status));
 	mgmt_pending_remove(cmd);
 }
 
@@ -6563,18 +6567,12 @@ void mgmt_pin_code_neg_reply_complete(struct hci_dev *hdev, bdaddr_t *bdaddr,
 				      u8 status)
 {
 	struct pending_cmd *cmd;
-	struct mgmt_rp_pin_code_reply rp;
 
 	cmd = mgmt_pending_find(MGMT_OP_PIN_CODE_NEG_REPLY, hdev);
 	if (!cmd)
 		return;
 
-	bacpy(&rp.addr.bdaddr, bdaddr);
-	rp.addr.type = BDADDR_BREDR;
-
-	cmd_complete(cmd->sk, hdev->id, MGMT_OP_PIN_CODE_NEG_REPLY,
-		     mgmt_status(status), &rp, sizeof(rp));
-
+	cmd->cmd_complete(cmd, mgmt_status(status));
 	mgmt_pending_remove(cmd);
 }
 
@@ -6614,21 +6612,15 @@ static int user_pairing_resp_complete(struct hci_dev *hdev, bdaddr_t *bdaddr,
 				      u8 opcode)
 {
 	struct pending_cmd *cmd;
-	struct mgmt_rp_user_confirm_reply rp;
-	int err;
 
 	cmd = mgmt_pending_find(opcode, hdev);
 	if (!cmd)
 		return -ENOENT;
 
-	bacpy(&rp.addr.bdaddr, bdaddr);
-	rp.addr.type = link_to_bdaddr(link_type, addr_type);
-	err = cmd_complete(cmd->sk, hdev->id, opcode, mgmt_status(status),
-			   &rp, sizeof(rp));
-
+	cmd->cmd_complete(cmd, mgmt_status(status));
 	mgmt_pending_remove(cmd);
 
-	return err;
+	return 0;
 }
 
 int mgmt_user_confirm_reply_complete(struct hci_dev *hdev, bdaddr_t *bdaddr,
-- 
2.1.0

[PATCH 05/10] Bluetooth: Convert Pair Device to use cmd_complete callback

[Johan Hedberg]

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

This patch converts the Pair Device mgmt command to use the new
cmd_complete callback for pending mgmt commands. The already existing
pairing_complete() function is exactly what's needed and doesn't need
changing.

In addition to getting the return parameters always right this patch
actually fixes a reference counting bug and memory leak with the
hci_conn that's attached to the pending mgmt command - something that
would occur when powering off or unplugging the adapter while pairing is
in progress.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0fc3d6914ef0..d3ee7285c303 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3129,7 +3129,7 @@ void mgmt_smp_complete(struct hci_conn *conn, bool complete)
 
 	cmd = find_pairing(conn);
 	if (cmd)
-		pairing_complete(cmd, status);
+		cmd->cmd_complete(cmd, status);
 }
 
 static void pairing_complete_cb(struct hci_conn *conn, u8 status)
@@ -3142,7 +3142,7 @@ static void pairing_complete_cb(struct hci_conn *conn, u8 status)
 	if (!cmd)
 		BT_DBG("Unable to find a pending command");
 	else
-		pairing_complete(cmd, mgmt_status(status));
+		cmd->cmd_complete(cmd, mgmt_status(status));
 }
 
 static void le_pairing_complete_cb(struct hci_conn *conn, u8 status)
@@ -3158,7 +3158,7 @@ static void le_pairing_complete_cb(struct hci_conn *conn, u8 status)
 	if (!cmd)
 		BT_DBG("Unable to find a pending command");
 	else
-		pairing_complete(cmd, mgmt_status(status));
+		cmd->cmd_complete(cmd, mgmt_status(status));
 }
 
 static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
@@ -3255,6 +3255,8 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto unlock;
 	}
 
+	cmd->cmd_complete = pairing_complete;
+
 	/* For LE, just connecting isn't a proof that the pairing finished */
 	if (cp->addr.type == BDADDR_BREDR) {
 		conn->connect_cfm_cb = pairing_complete_cb;
-- 
2.1.0

[PATCH 06/10] Bluetooth: Convert Unpair Device to use cmd_complete callback

[Johan Hedberg]

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

This patch updates the Unpair Device code to take advantage of the
cmd_complete callback of struct pending_cmd.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d3ee7285c303..1accbb9d1a36 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2821,6 +2821,8 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto unlock;
 	}
 
+	cmd->cmd_complete = addr_cmd_complete;
+
 	dc.handle = cpu_to_le16(conn->handle);
 	dc.reason = 0x13; /* Remote User Terminated Connection */
 	err = hci_send_cmd(hdev, HCI_OP_DISCONNECT, sizeof(dc), &dc);
@@ -6430,16 +6432,10 @@ static void unpair_device_rsp(struct pending_cmd *cmd, void *data)
 {
 	struct hci_dev *hdev = data;
 	struct mgmt_cp_unpair_device *cp = cmd->param;
-	struct mgmt_rp_unpair_device rp;
-
-	memset(&rp, 0, sizeof(rp));
-	bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
-	rp.addr.type = cp->addr.type;
 
 	device_unpaired(hdev, &cp->addr.bdaddr, cp->addr.type, cmd->sk);
 
-	cmd_complete(cmd->sk, cmd->index, cmd->opcode, 0, &rp, sizeof(rp));
-
+	cmd->cmd_complete(cmd, 0);
 	mgmt_pending_remove(cmd);
 }
 
-- 
2.1.0

[PATCH 07/10] Bluetooth: Convert discovery commands to use cmd_complete callback

[Johan Hedberg]

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

This patch converts the Start/Stop Discovery mgmt commands to use the
cmd_complete callback of struct pending_cmd. Since both of these
commands return the same parameters as they take as input we can use the
existing generic_cmd_complete() helper for this.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1accbb9d1a36..ddaeebbccfba 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3835,10 +3835,7 @@ static void start_discovery_complete(struct hci_dev *hdev, u8 status)
 		cmd = mgmt_pending_find(MGMT_OP_START_SERVICE_DISCOVERY, hdev);
 
 	if (cmd) {
-		u8 type = hdev->discovery.type;
-
-		cmd_complete(cmd->sk, hdev->id, cmd->opcode,
-			     mgmt_status(status), &type, sizeof(type));
+		cmd->cmd_complete(cmd, mgmt_status(status));
 		mgmt_pending_remove(cmd);
 	}
 
@@ -3901,12 +3898,14 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 		goto failed;
 	}
 
-	cmd = mgmt_pending_add(sk, MGMT_OP_START_DISCOVERY, hdev, NULL, 0);
+	cmd = mgmt_pending_add(sk, MGMT_OP_START_DISCOVERY, hdev, data, len);
 	if (!cmd) {
 		err = -ENOMEM;
 		goto failed;
 	}
 
+	cmd->cmd_complete = generic_cmd_complete;
+
 	/* Clear the discovery filter first to free any previously
 	 * allocated memory for the UUID list.
 	 */
@@ -3936,6 +3935,11 @@ failed:
 	return err;
 }
 
+static void service_discovery_cmd_complete(struct pending_cmd *cmd, u8 status)
+{
+	cmd_complete(cmd->sk, cmd->index, cmd->opcode, status, cmd->param, 1);
+}
+
 static int start_service_discovery(struct sock *sk, struct hci_dev *hdev,
 				   void *data, u16 len)
 {
@@ -3991,12 +3995,14 @@ static int start_service_discovery(struct sock *sk, struct hci_dev *hdev,
 	}
 
 	cmd = mgmt_pending_add(sk, MGMT_OP_START_SERVICE_DISCOVERY,
-			       hdev, NULL, 0);
+			       hdev, data, len);
 	if (!cmd) {
 		err = -ENOMEM;
 		goto failed;
 	}
 
+	cmd->cmd_complete = service_discovery_cmd_complete;
+
 	/* Clear the discovery filter first to free any previously
 	 * allocated memory for the UUID list.
 	 */
@@ -4052,10 +4058,7 @@ static void stop_discovery_complete(struct hci_dev *hdev, u8 status)
 
 	cmd = mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, hdev);
 	if (cmd) {
-		u8 type = hdev->discovery.type;
-
-		cmd_complete(cmd->sk, hdev->id, cmd->opcode,
-			     mgmt_status(status), &type, sizeof(type));
+		cmd->cmd_complete(cmd, mgmt_status(status));
 		mgmt_pending_remove(cmd);
 	}
 
@@ -4091,12 +4094,14 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto unlock;
 	}
 
-	cmd = mgmt_pending_add(sk, MGMT_OP_STOP_DISCOVERY, hdev, NULL, 0);
+	cmd = mgmt_pending_add(sk, MGMT_OP_STOP_DISCOVERY, hdev, data, len);
 	if (!cmd) {
 		err = -ENOMEM;
 		goto unlock;
 	}
 
+	cmd->cmd_complete = generic_cmd_complete;
+
 	hci_req_init(&req, hdev);
 
 	hci_stop_discovery(&req);
-- 
2.1.0

[PATCH 08/10] Bluetooth: Convert Get Clock Info to use cmd_complete callback

[Johan Hedberg]

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

This patch converts the Get Clock Information mgmt command to take
advantage of the new cmd_complete callback for pending commands.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ddaeebbccfba..8467d3552aa9 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5287,10 +5287,40 @@ unlock:
 	return err;
 }
 
-static void get_clock_info_complete(struct hci_dev *hdev, u8 status)
+static void clock_info_cmd_complete(struct pending_cmd *cmd, u8 status)
 {
-	struct mgmt_cp_get_clock_info *cp;
+	struct hci_conn *conn = cmd->user_data;
 	struct mgmt_rp_get_clock_info rp;
+	struct hci_dev *hdev;
+
+	memset(&rp, 0, sizeof(rp));
+	memcpy(&rp.addr, &cmd->param, sizeof(rp.addr));
+
+	if (status)
+		goto complete;
+
+	hdev = hci_dev_get(cmd->index);
+	if (hdev) {
+		rp.local_clock = cpu_to_le32(hdev->clock);
+		hci_dev_put(hdev);
+	}
+
+	if (conn) {
+		rp.piconet_clock = cpu_to_le32(conn->clock);
+		rp.accuracy = cpu_to_le16(conn->clock_accuracy);
+	}
+
+complete:
+	cmd_complete(cmd->sk, cmd->index, cmd->opcode, status, &rp, sizeof(rp));
+
+	if (conn) {
+		hci_conn_drop(conn);
+		hci_conn_put(conn);
+	}
+}
+
+static void get_clock_info_complete(struct hci_dev *hdev, u8 status)
+{
 	struct hci_cp_read_clock *hci_cp;
 	struct pending_cmd *cmd;
 	struct hci_conn *conn;
@@ -5314,29 +5344,8 @@ static void get_clock_info_complete(struct hci_dev *hdev, u8 status)
 	if (!cmd)
 		goto unlock;
 
-	cp = cmd->param;
-
-	memset(&rp, 0, sizeof(rp));
-	memcpy(&rp.addr, &cp->addr, sizeof(rp.addr));
-
-	if (status)
-		goto send_rsp;
-
-	rp.local_clock = cpu_to_le32(hdev->clock);
-
-	if (conn) {
-		rp.piconet_clock = cpu_to_le32(conn->clock);
-		rp.accuracy = cpu_to_le16(conn->clock_accuracy);
-	}
-
-send_rsp:
-	cmd_complete(cmd->sk, cmd->index, cmd->opcode, mgmt_status(status),
-		     &rp, sizeof(rp));
+	cmd->cmd_complete(cmd, mgmt_status(status));
 	mgmt_pending_remove(cmd);
-	if (conn) {
-		hci_conn_drop(conn);
-		hci_conn_put(conn);
-	}
 
 unlock:
 	hci_dev_unlock(hdev);
@@ -5392,6 +5401,8 @@ static int get_clock_info(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto unlock;
 	}
 
+	cmd->cmd_complete = clock_info_cmd_complete;
+
 	hci_req_init(&req, hdev);
 
 	memset(&hci_cp, 0, sizeof(hci_cp));
-- 
2.1.0

[PATCH 09/10] Bluetooth: Fix initializing hci_conn RSSI to invalid value

[Johan Hedberg]

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

When we create the hci_conn object we should properly initialize the
RSSI to HCI_RSSI_INVALID.

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

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 96887ae8375b..79d84b88b8f0 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -449,6 +449,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
 	conn->io_capability = hdev->io_capability;
 	conn->remote_auth = 0xff;
 	conn->key_type = 0xff;
+	conn->rssi = HCI_RSSI_INVALID;
 	conn->tx_power = HCI_TX_POWER_INVALID;
 	conn->max_tx_power = HCI_TX_POWER_INVALID;
 
-- 
2.1.0

[PATCH 10/10] Bluetooth: Fix Get Conn Info to use cmd_complete callback

[Johan Hedberg]

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

This patch fixes the Get Connection Information mgmt command to take
advantage of the new cmd_complete callback. This allows for great
simplifications in the logic for constructing the cmd_complete event.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 8467d3552aa9..3bbc93ed4206 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5074,67 +5074,42 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
 	return err;
 }
 
-struct cmd_conn_lookup {
-	struct hci_conn *conn;
-	bool valid_tx_power;
-	u8 mgmt_status;
-};
-
-static void get_conn_info_complete(struct pending_cmd *cmd, void *data)
+static void conn_info_cmd_complete(struct pending_cmd *cmd, u8 status)
 {
-	struct cmd_conn_lookup *match = data;
-	struct mgmt_cp_get_conn_info *cp;
-	struct mgmt_rp_get_conn_info rp;
 	struct hci_conn *conn = cmd->user_data;
+	struct mgmt_rp_get_conn_info rp;
 
-	if (conn != match->conn)
-		return;
-
-	cp = (struct mgmt_cp_get_conn_info *) cmd->param;
-
-	memset(&rp, 0, sizeof(rp));
-	bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
-	rp.addr.type = cp->addr.type;
+	memcpy(&rp.addr, cmd->param, sizeof(rp.addr));
 
-	if (!match->mgmt_status) {
+	if (status == MGMT_STATUS_SUCCESS) {
 		rp.rssi = conn->rssi;
-
-		if (match->valid_tx_power) {
-			rp.tx_power = conn->tx_power;
-			rp.max_tx_power = conn->max_tx_power;
-		} else {
-			rp.tx_power = HCI_TX_POWER_INVALID;
-			rp.max_tx_power = HCI_TX_POWER_INVALID;
-		}
+		rp.tx_power = conn->tx_power;
+		rp.max_tx_power = conn->max_tx_power;
+	} else {
+		rp.rssi = HCI_RSSI_INVALID;
+		rp.tx_power = HCI_TX_POWER_INVALID;
+		rp.max_tx_power = HCI_TX_POWER_INVALID;
 	}
 
-	cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO,
-		     match->mgmt_status, &rp, sizeof(rp));
+	cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO, status,
+		     &rp, sizeof(rp));
 
 	hci_conn_drop(conn);
 	hci_conn_put(conn);
-
-	mgmt_pending_remove(cmd);
 }
 
-static void conn_info_refresh_complete(struct hci_dev *hdev, u8 status)
+static void conn_info_refresh_complete(struct hci_dev *hdev, u8 hci_status)
 {
 	struct hci_cp_read_rssi *cp;
+	struct pending_cmd *cmd;
 	struct hci_conn *conn;
-	struct cmd_conn_lookup match;
 	u16 handle;
+	u8 status;
 
 	BT_DBG("status 0x%02x", status);
 
 	hci_dev_lock(hdev);
 
-	/* TX power data is valid in case request completed successfully,
-	 * otherwise we assume it's not valid. At the moment we assume that
-	 * either both or none of current and max values are valid to keep code
-	 * simple.
-	 */
-	match.valid_tx_power = !status;
-
 	/* Commands sent in request are either Read RSSI or Read Transmit Power
 	 * Level so we check which one was last sent to retrieve connection
 	 * handle.  Both commands have handle as first parameter so it's safe to
@@ -5147,29 +5122,29 @@ static void conn_info_refresh_complete(struct hci_dev *hdev, u8 status)
 	cp = hci_sent_cmd_data(hdev, HCI_OP_READ_RSSI);
 	if (!cp) {
 		cp = hci_sent_cmd_data(hdev, HCI_OP_READ_TX_POWER);
-		status = 0;
+		status = MGMT_STATUS_SUCCESS;
+	} else {
+		status = mgmt_status(hci_status);
 	}
 
 	if (!cp) {
-		BT_ERR("invalid sent_cmd in response");
+		BT_ERR("invalid sent_cmd in conn_info response");
 		goto unlock;
 	}
 
 	handle = __le16_to_cpu(cp->handle);
 	conn = hci_conn_hash_lookup_handle(hdev, handle);
 	if (!conn) {
-		BT_ERR("unknown handle (%d) in response", handle);
+		BT_ERR("unknown handle (%d) in conn_info response", handle);
 		goto unlock;
 	}
 
-	match.conn = conn;
-	match.mgmt_status = mgmt_status(status);
+	cmd = mgmt_pending_find_data(MGMT_OP_GET_CONN_INFO, hdev, conn);
+	if (!cmd)
+		goto unlock;
 
-	/* Cache refresh is complete, now reply for mgmt request for given
-	 * connection only.
-	 */
-	mgmt_pending_foreach(MGMT_OP_GET_CONN_INFO, hdev,
-			     get_conn_info_complete, &match);
+	cmd->cmd_complete(cmd, status);
+	mgmt_pending_remove(cmd);
 
 unlock:
 	hci_dev_unlock(hdev);
@@ -5215,6 +5190,12 @@ static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto unlock;
 	}
 
+	if (mgmt_pending_find_data(MGMT_OP_GET_CONN_INFO, hdev, conn)) {
+		err = cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
+				   MGMT_STATUS_BUSY, &rp, sizeof(rp));
+		goto unlock;
+	}
+
 	/* To avoid client trying to guess when to poll again for information we
 	 * calculate conn info age as random value between min/max set in hdev.
 	 */
@@ -5270,6 +5251,7 @@ static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
 
 		hci_conn_hold(conn);
 		cmd->user_data = hci_conn_get(conn);
+		cmd->cmd_complete = conn_info_cmd_complete;
 
 		conn->conn_info_timestamp = jiffies;
 	} else {
-- 
2.1.0

[PATCH v2 10/10] Bluetooth: Fix Get Conn Info to use cmd_complete callback

[Johan Hedberg]

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

This patch fixes the Get Connection Information mgmt command to take
advantage of the new cmd_complete callback. This allows for great
simplifications in the logic for constructing the cmd_complete event.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
v2: Fixed s/status/hci_status/ in BT_DBG() call

 net/bluetooth/mgmt.c | 84 +++++++++++++++++++++-------------------------------
 1 file changed, 33 insertions(+), 51 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 8467d3552aa9..61a04a3de7ad 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5074,67 +5074,42 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
 	return err;
 }
 
-struct cmd_conn_lookup {
-	struct hci_conn *conn;
-	bool valid_tx_power;
-	u8 mgmt_status;
-};
-
-static void get_conn_info_complete(struct pending_cmd *cmd, void *data)
+static void conn_info_cmd_complete(struct pending_cmd *cmd, u8 status)
 {
-	struct cmd_conn_lookup *match = data;
-	struct mgmt_cp_get_conn_info *cp;
-	struct mgmt_rp_get_conn_info rp;
 	struct hci_conn *conn = cmd->user_data;
+	struct mgmt_rp_get_conn_info rp;
 
-	if (conn != match->conn)
-		return;
-
-	cp = (struct mgmt_cp_get_conn_info *) cmd->param;
+	memcpy(&rp.addr, cmd->param, sizeof(rp.addr));
 
-	memset(&rp, 0, sizeof(rp));
-	bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
-	rp.addr.type = cp->addr.type;
-
-	if (!match->mgmt_status) {
+	if (status == MGMT_STATUS_SUCCESS) {
 		rp.rssi = conn->rssi;
-
-		if (match->valid_tx_power) {
-			rp.tx_power = conn->tx_power;
-			rp.max_tx_power = conn->max_tx_power;
-		} else {
-			rp.tx_power = HCI_TX_POWER_INVALID;
-			rp.max_tx_power = HCI_TX_POWER_INVALID;
-		}
+		rp.tx_power = conn->tx_power;
+		rp.max_tx_power = conn->max_tx_power;
+	} else {
+		rp.rssi = HCI_RSSI_INVALID;
+		rp.tx_power = HCI_TX_POWER_INVALID;
+		rp.max_tx_power = HCI_TX_POWER_INVALID;
 	}
 
-	cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO,
-		     match->mgmt_status, &rp, sizeof(rp));
+	cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO, status,
+		     &rp, sizeof(rp));
 
 	hci_conn_drop(conn);
 	hci_conn_put(conn);
-
-	mgmt_pending_remove(cmd);
 }
 
-static void conn_info_refresh_complete(struct hci_dev *hdev, u8 status)
+static void conn_info_refresh_complete(struct hci_dev *hdev, u8 hci_status)
 {
 	struct hci_cp_read_rssi *cp;
+	struct pending_cmd *cmd;
 	struct hci_conn *conn;
-	struct cmd_conn_lookup match;
 	u16 handle;
+	u8 status;
 
-	BT_DBG("status 0x%02x", status);
+	BT_DBG("status 0x%02x", hci_status);
 
 	hci_dev_lock(hdev);
 
-	/* TX power data is valid in case request completed successfully,
-	 * otherwise we assume it's not valid. At the moment we assume that
-	 * either both or none of current and max values are valid to keep code
-	 * simple.
-	 */
-	match.valid_tx_power = !status;
-
 	/* Commands sent in request are either Read RSSI or Read Transmit Power
 	 * Level so we check which one was last sent to retrieve connection
 	 * handle.  Both commands have handle as first parameter so it's safe to
@@ -5147,29 +5122,29 @@ static void conn_info_refresh_complete(struct hci_dev *hdev, u8 status)
 	cp = hci_sent_cmd_data(hdev, HCI_OP_READ_RSSI);
 	if (!cp) {
 		cp = hci_sent_cmd_data(hdev, HCI_OP_READ_TX_POWER);
-		status = 0;
+		status = MGMT_STATUS_SUCCESS;
+	} else {
+		status = mgmt_status(hci_status);
 	}
 
 	if (!cp) {
-		BT_ERR("invalid sent_cmd in response");
+		BT_ERR("invalid sent_cmd in conn_info response");
 		goto unlock;
 	}
 
 	handle = __le16_to_cpu(cp->handle);
 	conn = hci_conn_hash_lookup_handle(hdev, handle);
 	if (!conn) {
-		BT_ERR("unknown handle (%d) in response", handle);
+		BT_ERR("unknown handle (%d) in conn_info response", handle);
 		goto unlock;
 	}
 
-	match.conn = conn;
-	match.mgmt_status = mgmt_status(status);
+	cmd = mgmt_pending_find_data(MGMT_OP_GET_CONN_INFO, hdev, conn);
+	if (!cmd)
+		goto unlock;
 
-	/* Cache refresh is complete, now reply for mgmt request for given
-	 * connection only.
-	 */
-	mgmt_pending_foreach(MGMT_OP_GET_CONN_INFO, hdev,
-			     get_conn_info_complete, &match);
+	cmd->cmd_complete(cmd, status);
+	mgmt_pending_remove(cmd);
 
 unlock:
 	hci_dev_unlock(hdev);
@@ -5215,6 +5190,12 @@ static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto unlock;
 	}
 
+	if (mgmt_pending_find_data(MGMT_OP_GET_CONN_INFO, hdev, conn)) {
+		err = cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
+				   MGMT_STATUS_BUSY, &rp, sizeof(rp));
+		goto unlock;
+	}
+
 	/* To avoid client trying to guess when to poll again for information we
 	 * calculate conn info age as random value between min/max set in hdev.
 	 */
@@ -5270,6 +5251,7 @@ static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
 
 		hci_conn_hold(conn);
 		cmd->user_data = hci_conn_get(conn);
+		cmd->cmd_complete = conn_info_cmd_complete;
 
 		conn->conn_info_timestamp = jiffies;
 	} else {
-- 
2.1.0

Re: [PATCH 00/10] Bluetooth: Fix mgmt cmd_complete events

[Marcel Holtmann]

Hi Johan,

> This patch set introduces an optional callback that can be set for
> pending mgmt commands so that the generic places sending out these
> events (powering off and removing adapters) can send out correct events
> as specified in our mgmt-api.txt. A further benefit of having this
> callback is that we fix memory leaks caused by hci_conn pointers being
> stored as part of the pending command user data.
> 
> Johan
> 
> ----------------------------------------------------------------
> Johan Hedberg (10):
>      Bluetooth: Add callback to create proper cmd_complete events
>      Bluetooth: Store parameter length with pending mgmt commands
>      Bluetooth: Convert Disconnect mgmt command to use cmd_complete callback
>      Bluetooth: Use cmd_complete callback for authentication mgmt commands
>      Bluetooth: Convert Pair Device to use cmd_complete callback
>      Bluetooth: Convert Unpair Device to use cmd_complete callback
>      Bluetooth: Convert discovery commands to use cmd_complete callback
>      Bluetooth: Convert Get Clock Info to use cmd_complete callback
>      Bluetooth: Fix initializing hci_conn RSSI to invalid value
>      Bluetooth: Fix Get Conn Info to use cmd_complete callback
> 
> net/bluetooth/hci_conn.c |   1 +
> net/bluetooth/mgmt.c     | 271 ++++++++++++++++++++++-----------------------
> 2 files changed, 136 insertions(+), 136 deletions(-)

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

Regards

Marcel