Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Sonny Sasaka <sonnysasaka@chromium.org>
To:     linux-bluetooth@vger.kernel.org
Cc:     Sonny Sasaka <sonnysasaka@chromium.org>
Subject: [PATCH BlueZ] shared/mgmt: Provide timeout in mgmt_send()
Date:   Fri, 21 Aug 2020 00:21:55 -0700
Message-Id: <20200821072155.18834-1-sonnysasaka@chromium.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

This patch adds mgmt_send_with_timeout() to help callers who want to use
mgmt_send with timeout handling. Ideally, all mgmt_send() caller should
use timeout, but in this patch only UNPAIR_DEVICE mgmt command is known
to have no-reply problem in the kernel.

This prevents bluetoothd from getting stuck in a state of waiting for
kernel MGMT reply forever. We still need to fix the kernel side for the
missing reply.

---
 src/adapter.c     |  8 +++---
 src/shared/mgmt.c | 62 ++++++++++++++++++++++++++++++++++++++++++++---
 src/shared/mgmt.h |  6 +++++
 3 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 5e896a9f0..787d1bbb6 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -114,6 +114,8 @@ static const struct mgmt_blocked_key_info blocked_keys[] = {
 		 0x22, 0x8e, 0x07, 0x56, 0xb4, 0xe8, 0x5f, 0x01}},
 };
 
+#define DEFAULT_MGMT_TIMEOUT	2	/* Timeout for MGMT commands (secs) */
+
 static DBusConnection *dbus_conn = NULL;
 
 static bool kernel_conn_control = false;
@@ -7217,9 +7219,9 @@ int btd_adapter_remove_bonding(struct btd_adapter *adapter,
 	cp.addr.type = bdaddr_type;
 	cp.disconnect = 1;
 
-	if (mgmt_send(adapter->mgmt, MGMT_OP_UNPAIR_DEVICE,
-				adapter->dev_id, sizeof(cp), &cp,
-				NULL, NULL, NULL) > 0)
+	if (mgmt_send_with_timeout(adapter->mgmt, MGMT_OP_UNPAIR_DEVICE,
+					adapter->dev_id, sizeof(cp), &cp, NULL,
+					NULL, NULL, DEFAULT_MGMT_TIMEOUT) > 0)
 		return 0;
 
 	return -EIO;
diff --git a/src/shared/mgmt.c b/src/shared/mgmt.c
index 277e361a6..7e70aba43 100644
--- a/src/shared/mgmt.c
+++ b/src/shared/mgmt.c
@@ -38,6 +38,7 @@
 #include "src/shared/queue.h"
 #include "src/shared/util.h"
 #include "src/shared/mgmt.h"
+#include "src/shared/timeout.h"
 
 struct mgmt {
 	int ref_count;
@@ -60,6 +61,11 @@ struct mgmt {
 	void *debug_data;
 };
 
+struct mgmt_pending_request {
+	unsigned int request_id;
+	struct mgmt *mgmt;
+};
+
 struct mgmt_request {
 	unsigned int id;
 	uint16_t opcode;
@@ -69,6 +75,8 @@ struct mgmt_request {
 	mgmt_request_func_t callback;
 	mgmt_destroy_func_t destroy;
 	void *user_data;
+	/* Timeout for the request if > zero, otherwise timer is not used. */
+	int timeout_seconds;
 };
 
 struct mgmt_notify {
@@ -157,6 +165,8 @@ static void write_watch_destroy(void *user_data)
 	mgmt->writer_active = false;
 }
 
+bool mgmt_pending_timeout(void *user_data);
+
 static bool send_request(struct mgmt *mgmt, struct mgmt_request *request)
 {
 	struct iovec iov;
@@ -184,6 +194,14 @@ static bool send_request(struct mgmt *mgmt, struct mgmt_request *request)
 							mgmt->debug_data);
 
 	queue_push_tail(mgmt->pending_list, request);
+	if (request->timeout_seconds > 0) {
+		struct mgmt_pending_request *pending_request =
+			malloc(sizeof(struct mgmt_pending_request));
+		pending_request->request_id = request->id;
+		pending_request->mgmt = mgmt;
+		timeout_add(request->timeout_seconds * 1000,
+				mgmt_pending_timeout, pending_request, NULL);
+	}
 
 	return true;
 }
@@ -267,6 +285,29 @@ static void request_complete(struct mgmt *mgmt, uint8_t status,
 	wakeup_writer(mgmt);
 }
 
+bool mgmt_pending_timeout(void *user_data)
+{
+	struct mgmt_pending_request *pending_request = user_data;
+	struct mgmt *mgmt = pending_request->mgmt;
+	const struct mgmt_request *request =
+			queue_find(mgmt->pending_list,
+					match_request_id,
+					UINT_TO_PTR(
+						pending_request->request_id));
+
+	free(pending_request);
+
+	if (!request)
+		return false;
+
+	/* Pretend that kernel has replied with TIMEOUT status. */
+	request_complete(mgmt, MGMT_STATUS_TIMEOUT, request->opcode,
+				request->index, /* length */ 0,
+				/* param */ NULL);
+
+	return false;
+}
+
 struct event_index {
 	uint16_t event;
 	uint16_t index;
@@ -571,10 +612,12 @@ static struct mgmt_request *create_request(uint16_t opcode, uint16_t index,
 	return request;
 }
 
-unsigned int mgmt_send(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
-				uint16_t length, const void *param,
-				mgmt_request_func_t callback,
-				void *user_data, mgmt_destroy_func_t destroy)
+unsigned int mgmt_send_with_timeout(
+			struct mgmt *mgmt, uint16_t opcode, uint16_t index,
+			uint16_t length, const void *param,
+			mgmt_request_func_t callback,
+			void *user_data, mgmt_destroy_func_t destroy,
+			int timeout_seconds)
 {
 	struct mgmt_request *request;
 
@@ -590,6 +633,7 @@ unsigned int mgmt_send(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 		mgmt->next_request_id = 1;
 
 	request->id = mgmt->next_request_id++;
+	request->timeout_seconds = timeout_seconds;
 
 	if (!queue_push_tail(mgmt->request_queue, request)) {
 		free(request->buf);
@@ -602,6 +646,16 @@ unsigned int mgmt_send(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 	return request->id;
 }
 
+unsigned int mgmt_send(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
+				uint16_t length, const void *param,
+				mgmt_request_func_t callback,
+				void *user_data, mgmt_destroy_func_t destroy)
+{
+	return mgmt_send_with_timeout(
+			mgmt, opcode, index, length, param, callback, user_data,
+			destroy, /* timeout_seconds */ 0);
+}
+
 unsigned int mgmt_send_nowait(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 				uint16_t length, const void *param,
 				mgmt_request_func_t callback,
diff --git a/src/shared/mgmt.h b/src/shared/mgmt.h
index 7caeb3844..9aea64010 100644
--- a/src/shared/mgmt.h
+++ b/src/shared/mgmt.h
@@ -46,6 +46,12 @@ bool mgmt_set_close_on_unref(struct mgmt *mgmt, bool do_close);
 typedef void (*mgmt_request_func_t)(uint8_t status, uint16_t length,
 					const void *param, void *user_data);
 
+unsigned int mgmt_send_with_timeout(
+			struct mgmt *mgmt, uint16_t opcode, uint16_t index,
+			uint16_t length, const void *param,
+			mgmt_request_func_t callback,
+			void *user_data, mgmt_destroy_func_t destroy,
+			int timeout_seconds);
 unsigned int mgmt_send(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 				uint16_t length, const void *param,
 				mgmt_request_func_t callback,
-- 
2.26.2