From: Archie Pusaka <[email protected]>
It is possible to have some handles not removed, for example the host
may decide not to wait for disconnection complete event when it is
suspending. In this case, when the peer device reconnected, we might
have two of the some handle assigned and create problem down the road.
This patch solves the issue by always removing any previous handles
when assigning a new handle if they are the same.
Reviewed-by: Zhengping Jiang <[email protected]>
---
Changes in v2:
* Return connection pointer when removing handle
monitor/packet.c | 79 ++++++++++++++++++++++++++----------------------
1 file changed, 43 insertions(+), 36 deletions(-)
diff --git a/monitor/packet.c b/monitor/packet.c
index 42e711cafa..164cc82bb2 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -189,59 +189,66 @@ static struct packet_conn_data *lookup_parent(uint16_t handle)
return NULL;
}
-static void assign_handle(uint16_t index, uint16_t handle, uint8_t type,
- uint8_t *dst, uint8_t dst_type)
+static struct packet_conn_data *release_handle(uint16_t handle)
{
int i;
for (i = 0; i < MAX_CONN; i++) {
- if (conn_list[i].handle == 0xffff) {
- hci_devba(index, (bdaddr_t *)conn_list[i].src);
+ struct packet_conn_data *conn = &conn_list[i];
+
+ if (conn->handle == handle) {
+ if (conn->destroy)
+ conn->destroy(conn->data);
- conn_list[i].index = index;
- conn_list[i].handle = handle;
- conn_list[i].type = type;
+ queue_destroy(conn->tx_q, free);
+ queue_destroy(conn->chan_q, free);
+ memset(conn, 0, sizeof(*conn));
+ conn->handle = 0xffff;
+ return conn;
+ }
+ }
- if (!dst) {
- struct packet_conn_data *p;
+ return NULL;
+}
- /* If destination is not set attempt to use the
- * parent one if that exists.
- */
- p = lookup_parent(handle);
- if (p) {
- memcpy(conn_list[i].dst, p->dst,
- sizeof(conn_list[i].dst));
- conn_list[i].dst_type = p->dst_type;
- }
+static void assign_handle(uint16_t index, uint16_t handle, uint8_t type,
+ uint8_t *dst, uint8_t dst_type)
+{
+ struct packet_conn_data *conn = release_handle(handle);
+ int i;
+ if (!conn) {
+ for (i = 0; i < MAX_CONN; i++) {
+ if (conn_list[i].handle == 0xffff) {
+ conn = &conn_list[i];
break;
}
-
- memcpy(conn_list[i].dst, dst, sizeof(conn_list[i].dst));
- conn_list[i].dst_type = dst_type;
- break;
}
}
-}
-static void release_handle(uint16_t handle)
-{
- int i;
+ if (!conn)
+ return;
- for (i = 0; i < MAX_CONN; i++) {
- struct packet_conn_data *conn = &conn_list[i];
+ hci_devba(index, (bdaddr_t *)conn->src);
- if (conn->handle == handle) {
- if (conn->destroy)
- conn->destroy(conn->data);
+ conn->index = index;
+ conn->handle = handle;
+ conn->type = type;
- queue_destroy(conn->tx_q, free);
- queue_destroy(conn->chan_q, free);
- memset(conn, 0, sizeof(*conn));
- conn->handle = 0xffff;
- break;
+ if (!dst) {
+ struct packet_conn_data *p;
+
+ /* If destination is not set attempt to use the parent one if
+ * that exists.
+ */
+ p = lookup_parent(handle);
+ if (p) {
+ memcpy(conn->dst, p->dst, sizeof(conn->dst));
+ conn->dst_type = p->dst_type;
}
+ } else {
+ memcpy(conn->dst, dst, sizeof(conn->dst));
+ conn->dst_type = dst_type;
}
}
--
2.43.0.429.g432eaa2c6b-goog
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=821233
---Test result---
Test Summary:
CheckPatch PASS 0.94 seconds
GitLint PASS 0.64 seconds
BuildEll PASS 24.40 seconds
BluezMake PASS 734.57 seconds
MakeCheck PASS 12.14 seconds
MakeDistcheck PASS 166.71 seconds
CheckValgrind PASS 231.79 seconds
CheckSmatch WARNING 337.51 seconds
bluezmakeextell PASS 109.66 seconds
IncrementalBuild PASS 1351.96 seconds
ScanBuild PASS 941.39 seconds
Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
monitor/packet.c: note: in included file:monitor/display.h:82:26: warning: Variable length array is used.monitor/packet.c:1867:26: warning: Variable length array is used.monitor/packet.c: note: in included file:monitor/bt.h:3606:52: warning: array of flexible structuresmonitor/bt.h:3594:40: warning: array of flexible structuresmonitor/packet.c: note: in included file:monitor/display.h:82:26: warning: Variable length array is used.monitor/packet.c:1867:26: warning: Variable length array is used.monitor/packet.c: note: in included file:monitor/bt.h:3606:52: warning: array of flexible structuresmonitor/bt.h:3594:40: warning: array of flexible structures
---
Regards,
Linux Bluetooth
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Tue, 30 Jan 2024 18:24:59 +0800 you wrote:
> From: Archie Pusaka <[email protected]>
>
> It is possible to have some handles not removed, for example the host
> may decide not to wait for disconnection complete event when it is
> suspending. In this case, when the peer device reconnected, we might
> have two of the some handle assigned and create problem down the road.
>
> [...]
Here is the summary with links:
- [Bluez,v2,1/2] Monitor: Remove handle before assigning
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=12ccf5ea0fa5
- [Bluez,v2,2/2] Monitor: Avoid printing stale address on connection event
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=e98bbe3f1cb2
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html