[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-bluetooth
Subject:    [PATCH BlueZ v1 2/3] shared/gatt: Cancel discovery requests in client
From:       Arman Uguray <armansito () chromium ! org>
Date:       2015-02-28 0:44:46
Message-ID: 1425084287-9974-2-git-send-email-armansito () chromium ! org
[Download RAW message or body]

This patch fixes potential cases of invalid access if discovery and
MTU exchange procedure callbacks are invoked after cleaning up a
bt_gatt_client, by cancelling all pending discovery requests in
bt_gatt_cancel_all.
---
 src/shared/gatt-client.c | 113 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 85 insertions(+), 28 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 008bd3e..88863bc 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -100,6 +100,9 @@ struct bt_gatt_client {
 	 */
 	struct queue *pending_requests;
 	unsigned int next_request_id;
+
+	struct bt_gatt_async_req *discovery_req;
+	unsigned int mtu_req_id;
 };
 
 struct request {
@@ -415,6 +418,15 @@ static void discovery_op_unref(void *data)
 	discovery_op_free(op);
 }
 
+static void discovery_req_clear(struct bt_gatt_client *client)
+{
+	if (!client->discovery_req)
+		return;
+
+	bt_gatt_async_req_unref(client->discovery_req);
+	client->discovery_req = NULL;
+}
+
 static void discover_chrcs_cb(bool success, uint8_t att_ecode,
 						struct bt_gatt_result *result,
 						void *user_data);
@@ -432,6 +444,8 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
 	char uuid_str[MAX_LEN_UUID_STR];
 	unsigned int includes_count, i;
 
+	discovery_req_clear(client);
+
 	if (!success) {
 		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND)
 			goto next;
@@ -508,11 +522,14 @@ next:
 			goto failed;
 
 		op->cur_svc = attr;
-		if (bt_gatt_discover_characteristics(client->att,
+
+		client->discovery_req = bt_gatt_discover_characteristics(
+							client->att,
 							start, end,
 							discover_chrcs_cb,
 							discovery_op_ref(op),
-							discovery_op_unref))
+							discovery_op_unref);
+		if (client->discovery_req)
 			return;
 
 		util_debug(client->debug_callback, client->debug_data,
@@ -529,10 +546,12 @@ next:
 	if (start == end)
 		goto next;
 
-	if (bt_gatt_discover_included_services(client->att, start, end,
+	client->discovery_req = bt_gatt_discover_included_services(client->att,
+							start, end,
 							discover_incl_cb,
 							discovery_op_ref(op),
-							discovery_op_unref))
+							discovery_op_unref);
+	if (client->discovery_req)
 		return;
 
 	util_debug(client->debug_callback, client->debug_data,
@@ -585,11 +604,13 @@ static bool discover_descs(struct discovery_op *op, bool *discovering)
 			continue;
 		}
 
-		if (bt_gatt_discover_descriptors(client->att, desc_start,
+		client->discovery_req = bt_gatt_discover_descriptors(
+							client->att, desc_start,
 							chrc_data->end_handle,
 							discover_descs_cb,
 							discovery_op_ref(op),
-							discovery_op_unref)) {
+							discovery_op_unref);
+		if (client->discovery_req) {
 			*discovering = true;
 			goto done;
 		}
@@ -625,6 +646,8 @@ static void discover_descs_cb(bool success, uint8_t att_ecode,
 	unsigned int desc_count;
 	bool discovering;
 
+	discovery_req_clear(client);
+
 	if (!success) {
 		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND) {
 			success = true;
@@ -684,10 +707,13 @@ next:
 
 	/* Move on to the next service */
 	op->cur_svc = attr;
-	if (bt_gatt_discover_characteristics(client->att, start, end,
+
+	client->discovery_req = bt_gatt_discover_characteristics(client->att,
+							start, end,
 							discover_chrcs_cb,
 							discovery_op_ref(op),
-							discovery_op_unref))
+							discovery_op_unref);
+	if (client->discovery_req)
 		return;
 
 	util_debug(client->debug_callback, client->debug_data,
@@ -719,6 +745,8 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
 	unsigned int chrc_count;
 	bool discovering;
 
+	discovery_req_clear(client);
+
 	if (!success) {
 		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND) {
 			success = true;
@@ -788,10 +816,13 @@ next:
 
 	/* Move on to the next service */
 	op->cur_svc = attr;
-	if (bt_gatt_discover_characteristics(client->att, start, end,
+
+	client->discovery_req = bt_gatt_discover_characteristics(client->att,
+							start, end,
 							discover_chrcs_cb,
 							discovery_op_ref(op),
-							discovery_op_unref))
+							discovery_op_unref);
+	if (client->discovery_req)
 		return;
 
 	util_debug(client->debug_callback, client->debug_data,
@@ -819,6 +850,8 @@ static void discover_secondary_cb(bool success, uint8_t att_ecode,
 	bt_uuid_t uuid;
 	char uuid_str[MAX_LEN_UUID_STR];
 
+	discovery_req_clear(client);
+
 	if (!success) {
 		util_debug(client->debug_callback, client->debug_data,
 					"Secondary service discovery failed."
@@ -883,10 +916,12 @@ next:
 		goto done;
 	}
 
-	if (bt_gatt_discover_included_services(client->att, start, end,
+	client->discovery_req = bt_gatt_discover_included_services(client->att,
+							start, end,
 							discover_incl_cb,
 							discovery_op_ref(op),
-							discovery_op_unref))
+							discovery_op_unref);
+	if (client->discovery_req)
 		return;
 
 	util_debug(client->debug_callback, client->debug_data,
@@ -911,6 +946,8 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 	bt_uuid_t uuid;
 	char uuid_str[MAX_LEN_UUID_STR];
 
+	discovery_req_clear(client);
+
 	if (!success) {
 		util_debug(client->debug_callback, client->debug_data,
 					"Primary service discovery failed."
@@ -950,11 +987,12 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 
 secondary:
 	/* Discover secondary services */
-	if (bt_gatt_discover_secondary_services(client->att, NULL,
-							op->start, op->end,
-							discover_secondary_cb,
-							discovery_op_ref(op),
-							discovery_op_unref))
+	client->discovery_req = bt_gatt_discover_secondary_services(client->att,
+						NULL, op->start, op->end,
+						discover_secondary_cb,
+						discovery_op_ref(op),
+						discovery_op_unref);
+	if (client->discovery_req)
 		return;
 
 	util_debug(client->debug_callback, client->debug_data,
@@ -984,6 +1022,7 @@ static void exchange_mtu_cb(bool success, uint8_t att_ecode, void *user_data)
 	struct bt_gatt_client *client = op->client;
 
 	op->success = success;
+	client->mtu_req_id = 0;
 
 	if (!success) {
 		util_debug(client->debug_callback, client->debug_data,
@@ -1006,10 +1045,12 @@ static void exchange_mtu_cb(bool success, uint8_t att_ecode, void *user_data)
 		return;
 	}
 
-	if (bt_gatt_discover_all_primary_services(client->att, NULL,
+	client->discovery_req = bt_gatt_discover_all_primary_services(
+							client->att, NULL,
 							discover_primary_cb,
 							discovery_op_ref(op),
-							discovery_op_unref))
+							discovery_op_unref);
+	if (client->discovery_req)
 		return;
 
 	util_debug(client->debug_callback, client->debug_data,
@@ -1120,7 +1161,9 @@ static void service_changed_complete(struct discovery_op *op, bool success,
 
 static void service_changed_failure(struct discovery_op *op)
 {
-	gatt_db_clear_range(op->client->db, op->start, op->end);
+	struct bt_gatt_client *client = op->client;
+
+	gatt_db_clear_range(client->db, op->start, op->end);
 }
 
 static void process_service_changed(struct bt_gatt_client *client,
@@ -1146,11 +1189,12 @@ static void process_service_changed(struct bt_gatt_client *client,
 	if (!op)
 		goto fail;
 
-	if (bt_gatt_discover_primary_services(client->att, NULL,
-						start_handle, end_handle,
+	client->discovery_req = bt_gatt_discover_primary_services(client->att,
+						NULL, start_handle, end_handle,
 						discover_primary_cb,
 						discovery_op_ref(op),
-						discovery_op_unref)) {
+						discovery_op_unref);
+	if (client->discovery_req) {
 		client->in_svc_chngd = true;
 		return;
 	}
@@ -1280,7 +1324,9 @@ done:
 
 static void init_fail(struct discovery_op *op)
 {
-	gatt_db_clear(op->client->db);
+	struct bt_gatt_client *client = op->client;
+
+	gatt_db_clear(client->db);
 }
 
 static bool gatt_client_init(struct bt_gatt_client *client, uint16_t mtu)
@@ -1296,10 +1342,12 @@ static bool gatt_client_init(struct bt_gatt_client *client, uint16_t mtu)
 		return false;
 
 	/* Configure the MTU */
-	if (!bt_gatt_exchange_mtu(client->att, MAX(BT_ATT_DEFAULT_LE_MTU, mtu),
-							exchange_mtu_cb,
-							discovery_op_ref(op),
-							discovery_op_unref)) {
+	client->mtu_req_id = bt_gatt_exchange_mtu(client->att,
+						MAX(BT_ATT_DEFAULT_LE_MTU, mtu),
+						exchange_mtu_cb,
+						discovery_op_ref(op),
+						discovery_op_unref);
+	if (!client->mtu_req_id) {
 		discovery_op_free(op);
 		return false;
 	}
@@ -1790,6 +1838,15 @@ bool bt_gatt_client_cancel_all(struct bt_gatt_client *client)
 
 	queue_remove_all(client->pending_requests, NULL, NULL, cancel_request);
 
+	if (client->discovery_req) {
+		bt_gatt_async_req_cancel(client->discovery_req);
+		bt_gatt_async_req_unref(client->discovery_req);
+		client->discovery_req = NULL;
+	}
+
+	if (client->mtu_req_id)
+		bt_att_cancel(client->att, client->mtu_req_id);
+
 	return true;
 }
 
-- 
2.2.0.rc0.207.ga3a616c

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic