2022-07-22 00:29:18

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH v2] Bluetooth: MGMT: Fix helding hci_conn reference while command is queued

From: Luiz Augusto von Dentz <[email protected]>

This removes the use of hci_conn_hold from Get Conn Info and Get Clock
Info since the callback can just do a lookup by address using the cmd
data and only then set cmd->user_data to pass to the complete callback.

Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
v2: Fix CI findings

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0c6878095709..aa651129b714 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6711,11 +6711,6 @@ static void get_conn_info_complete(struct hci_dev *hdev, void *data, int err)
mgmt_cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO, status,
&rp, sizeof(rp));

- if (conn) {
- hci_conn_drop(conn);
- hci_conn_put(conn);
- }
-
mgmt_pending_free(cmd);
}

@@ -6734,15 +6729,10 @@ static int get_conn_info_sync(struct hci_dev *hdev, void *data)
else
conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);

- if (!conn || conn != cmd->user_data || conn->state != BT_CONNECTED) {
- if (cmd->user_data) {
- hci_conn_drop(cmd->user_data);
- hci_conn_put(cmd->user_data);
- cmd->user_data = NULL;
- }
+ if (!conn || conn->state != BT_CONNECTED)
return MGMT_STATUS_NOT_CONNECTED;
- }

+ cmd->user_data = conn;
handle = cpu_to_le16(conn->handle);

/* Refresh RSSI each time */
@@ -6824,8 +6814,6 @@ static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
if (!cmd) {
err = -ENOMEM;
} else {
- hci_conn_hold(conn);
- cmd->user_data = hci_conn_get(conn);
err = hci_cmd_sync_queue(hdev, get_conn_info_sync,
cmd, get_conn_info_complete);
}
@@ -6878,8 +6866,6 @@ static void get_clock_info_complete(struct hci_dev *hdev, void *data, int err)
if (conn) {
rp.piconet_clock = cpu_to_le32(conn->clock);
rp.accuracy = cpu_to_le16(conn->clock_accuracy);
- hci_conn_drop(conn);
- hci_conn_put(conn);
}

complete:
@@ -6894,30 +6880,21 @@ static int get_clock_info_sync(struct hci_dev *hdev, void *data)
struct mgmt_pending_cmd *cmd = data;
struct mgmt_cp_get_clock_info *cp = cmd->param;
struct hci_cp_read_clock hci_cp;
- struct hci_conn *conn = cmd->user_data;
- int err;
+ struct hci_conn *conn;

memset(&hci_cp, 0, sizeof(hci_cp));
- err = hci_read_clock_sync(hdev, &hci_cp);
+ hci_read_clock_sync(hdev, &hci_cp);

- if (conn) {
- /* Make sure connection still exists */
- conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
- &cp->addr.bdaddr);
+ /* Make sure connection still exists */
+ conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->addr.bdaddr);
+ if (!conn || conn->state != BT_CONNECTED)
+ return MGMT_STATUS_NOT_CONNECTED;

- if (conn && conn == cmd->user_data &&
- conn->state == BT_CONNECTED) {
- hci_cp.handle = cpu_to_le16(conn->handle);
- hci_cp.which = 0x01; /* Piconet clock */
- err = hci_read_clock_sync(hdev, &hci_cp);
- } else if (cmd->user_data) {
- hci_conn_drop(cmd->user_data);
- hci_conn_put(cmd->user_data);
- cmd->user_data = NULL;
- }
- }
+ cmd->user_data = conn;
+ hci_cp.handle = cpu_to_le16(conn->handle);
+ hci_cp.which = 0x01; /* Piconet clock */

- return err;
+ return hci_read_clock_sync(hdev, &hci_cp);
}

static int get_clock_info(struct sock *sk, struct hci_dev *hdev, void *data,
@@ -6976,10 +6953,6 @@ static int get_clock_info(struct sock *sk, struct hci_dev *hdev, void *data,

if (cmd)
mgmt_pending_free(cmd);
-
- } else if (conn) {
- hci_conn_hold(conn);
- cmd->user_data = hci_conn_get(conn);
}


--
2.36.1


2022-07-22 03:21:48

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v2] Bluetooth: MGMT: Fix helding hci_conn reference while command is queued

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=662034

---Test result---

Test Summary:
CheckPatch PASS 1.54 seconds
GitLint PASS 0.86 seconds
SubjectPrefix PASS 0.63 seconds
BuildKernel PASS 36.42 seconds
BuildKernel32 PASS 30.98 seconds
Incremental Build with patchesPASS 51.65 seconds
TestRunner: Setup PASS 542.91 seconds
TestRunner: l2cap-tester PASS 17.68 seconds
TestRunner: bnep-tester PASS 5.95 seconds
TestRunner: mgmt-tester PASS 100.44 seconds
TestRunner: rfcomm-tester PASS 9.54 seconds
TestRunner: sco-tester PASS 9.29 seconds
TestRunner: smp-tester PASS 9.35 seconds
TestRunner: userchan-tester PASS 6.11 seconds



---
Regards,
Linux Bluetooth

2022-07-22 07:43:32

by Paul Menzel

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: MGMT: Fix helding hci_conn reference while command is queued

Dear Luiz,


Thank you for the patch.

I’d write holding instead of helding in the commit message summary.


Kind regards,

Paul

2022-07-22 18:28:49

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: MGMT: Fix helding hci_conn reference while command is queued

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Thu, 21 Jul 2022 17:26:00 -0700 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This removes the use of hci_conn_hold from Get Conn Info and Get Clock
> Info since the callback can just do a lookup by address using the cmd
> data and only then set cmd->user_data to pass to the complete callback.
>
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
>
> [...]

Here is the summary with links:
- [v2] Bluetooth: MGMT: Fix helding hci_conn reference while command is queued
https://git.kernel.org/bluetooth/bluetooth-next/c/7b445e220db9

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html