File gatt-Fix-not-cleaning-up-when-disconnected.patch of Package bluez.29390
From 838c0dc7641e1c991c0f3027bf94bee4606012f8 Mon Sep 17 00:00:00 2001
From: Bernie Conrad <bernie@allthenticate.net>
Date: Tue, 28 Sep 2021 16:00:15 -0700
Subject: [PATCH] gatt: Fix not cleaning up when disconnected
There is a current use after free possible on a gatt server if a client
disconnects while a WriteValue call is being processed with dbus.
This patch includes the addition of a pending disconnect callback to handle
cleanup better if a disconnect occurs during a write, an acquire write
or read operation using bt_att_register_disconnect with the cb.
Joey Lee:
Modified original 838c0dc7641e patch, I only applied the parts show in
bluez-5.48 and removed other parts.
---
src/gatt-database.c | 128 +++++++++++++++++++++++++-------------------
1 file changed, 74 insertions(+), 54 deletions(-)
Index: bluez-5.48/src/gatt-database.c
===================================================================
--- bluez-5.48.orig/src/gatt-database.c
+++ bluez-5.48/src/gatt-database.c
@@ -146,8 +146,9 @@ struct external_desc {
};
struct pending_op {
- struct btd_device *device;
+ struct bt_att *att;
unsigned int id;
+ unsigned int disconn_id;
uint16_t offset;
uint8_t link_type;
struct gatt_db_attribute *attrib;
@@ -836,6 +837,60 @@ done:
gatt_db_attribute_read_result(attrib, id, ecode, value, len);
}
+static struct btd_device *att_get_device(struct bt_att *att)
+{
+ GIOChannel *io = NULL;
+ GError *gerr = NULL;
+ bdaddr_t src, dst;
+ uint8_t dst_type;
+ struct btd_adapter *adapter;
+
+ io = g_io_channel_unix_new(bt_att_get_fd(att));
+ if (!io)
+ return NULL;
+
+ bt_io_get(io, &gerr, BT_IO_OPT_SOURCE_BDADDR, &src,
+ BT_IO_OPT_DEST_BDADDR, &dst,
+ BT_IO_OPT_DEST_TYPE, &dst_type,
+ BT_IO_OPT_INVALID);
+ if (gerr) {
+ error("bt_io_get: %s", gerr->message);
+ g_error_free(gerr);
+ g_io_channel_unref(io);
+ return NULL;
+ }
+
+ g_io_channel_unref(io);
+
+ adapter = adapter_find(&src);
+ if (!adapter) {
+ error("Unable to find adapter object");
+ return NULL;
+ }
+
+ return btd_adapter_find_device(adapter, &dst, dst_type);
+}
+
+
+static void pending_op_free(void *data)
+{
+ struct pending_op *op = data;
+
+ if (op->owner_queue)
+ queue_remove(op->owner_queue, op);
+
+ bt_att_unregister_disconnect(op->att, op->disconn_id);
+ bt_att_unref(op->att);
+ free(op);
+}
+
+static void pending_disconnect_cb(int err, void *user_data)
+{
+ struct pending_op *op = user_data;
+
+ op->owner_queue = NULL;
+}
+
static void gatt_ccc_write_cb(struct gatt_db_attribute *attrib,
unsigned int id, uint16_t offset,
const uint8_t *value, size_t len,
@@ -1709,41 +1764,35 @@ done:
gatt_db_attribute_read_result(op->attrib, op->id, ecode, value, len);
}
-static void pending_op_free(void *data)
-{
- struct pending_op *op = data;
- if (op->owner_queue)
- queue_remove(op->owner_queue, op);
-
- free(op);
-}
-
-static struct pending_op *pending_read_new(struct btd_device *device,
+static struct pending_op *pending_read_new(struct bt_att *att,
struct queue *owner_queue,
struct gatt_db_attribute *attrib,
- unsigned int id, uint16_t offset,
- uint8_t link_type)
+ unsigned int id, uint16_t offset)
{
struct pending_op *op;
op = new0(struct pending_op, 1);
op->owner_queue = owner_queue;
- op->device = device;
+ op->att = bt_att_ref(att);
op->attrib = attrib;
op->id = id;
op->offset = offset;
- op->link_type = link_type;
+ op->link_type = bt_att_get_link_type(att);
queue_push_tail(owner_queue, op);
+ op->disconn_id = bt_att_register_disconnect(att, pending_disconnect_cb,
+ op, NULL);
+
return op;
}
static void append_options(DBusMessageIter *iter, void *user_data)
{
struct pending_op *op = user_data;
- const char *path = device_get_path(op->device);
+ struct btd_device *device = att_get_device(op->att);
+ const char *path = device_get_path(device);
const char *link;
switch (op->link_type) {
@@ -1783,18 +1832,16 @@ static void read_setup_cb(DBusMessageIte
dbus_message_iter_close_container(iter, &dict);
}
-static struct pending_op *send_read(struct btd_device *device,
+static struct pending_op *send_read(struct bt_att *att,
struct gatt_db_attribute *attrib,
GDBusProxy *proxy,
struct queue *owner_queue,
unsigned int id,
- uint16_t offset,
- uint8_t link_type)
+ uint16_t offset)
{
struct pending_op *op;
- op = pending_read_new(device, owner_queue, attrib, id, offset,
- link_type);
+ op = pending_read_new(att, owner_queue, attrib, id, offset);
if (g_dbus_proxy_method_call(proxy, "ReadValue", read_setup_cb,
read_reply_cb, op, pending_op_free) == TRUE)
@@ -1865,16 +1912,18 @@ static void write_reply_cb(DBusMessage *
}
done:
- gatt_db_attribute_write_result(op->attrib, op->id, ecode);
+ /* Make sure that only reply if the device is connected */
+ if (!bt_att_get_fd(op->att))
+ gatt_db_attribute_write_result(op->attrib, op->id, ecode);
}
-static struct pending_op *pending_write_new(struct btd_device *device,
+static struct pending_op *pending_write_new(struct bt_att *att,
struct queue *owner_queue,
struct gatt_db_attribute *attrib,
unsigned int id,
const uint8_t *value,
size_t len,
- uint16_t offset, uint8_t link_type)
+ uint16_t offset)
{
struct pending_op *op;
@@ -1883,29 +1932,33 @@ static struct pending_op *pending_write_
op->data.iov_base = (uint8_t *) value;
op->data.iov_len = len;
- op->device = device;
+ op->att = bt_att_ref(att);
op->owner_queue = owner_queue;
op->attrib = attrib;
op->id = id;
op->offset = offset;
- op->link_type = link_type;
+ op->link_type = bt_att_get_link_type(att);
queue_push_tail(owner_queue, op);
+ bt_att_register_disconnect(att,
+ pending_disconnect_cb,
+ op, NULL);
+
return op;
}
-static struct pending_op *send_write(struct btd_device *device,
+static struct pending_op *send_write(struct bt_att *att,
struct gatt_db_attribute *attrib,
GDBusProxy *proxy,
struct queue *owner_queue,
unsigned int id,
const uint8_t *value, size_t len,
- uint16_t offset, uint8_t link_type)
+ uint16_t offset)
{
struct pending_op *op;
- op = pending_write_new(device, owner_queue, attrib, id, value, len,
- offset, link_type);
+ op = pending_write_new(att, owner_queue, attrib, id, value, len,
+ offset);
if (g_dbus_proxy_method_call(proxy, "WriteValue", write_setup_cb,
owner_queue ? write_reply_cb : NULL,
@@ -2016,9 +2069,8 @@ static void acquire_write_reply(DBusMess
return;
retry:
- send_write(op->device, op->attrib, chrc->proxy, NULL, op->id,
- op->data.iov_base, op->data.iov_len, 0,
- op->link_type);
+ send_write(op->att, op->attrib, chrc->proxy, NULL, op->id,
+ op->data.iov_base, op->data.iov_len, 0);
}
static void acquire_write_setup(DBusMessageIter *iter, void *user_data)
@@ -2027,6 +2079,7 @@ static void acquire_write_setup(DBusMess
DBusMessageIter dict;
struct bt_gatt_server *server;
uint16_t mtu;
+ struct btd_device *device = att_get_device(op->att);
dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
@@ -2037,7 +2090,7 @@ static void acquire_write_setup(DBusMess
append_options(&dict, op);
- server = btd_device_get_gatt_server(op->device);
+ server = btd_device_get_gatt_server(device);
mtu = bt_gatt_server_get_mtu(server);
@@ -2047,16 +2100,15 @@ static void acquire_write_setup(DBusMess
}
static struct pending_op *acquire_write(struct external_chrc *chrc,
- struct btd_device *device,
+ struct bt_att *att,
struct gatt_db_attribute *attrib,
unsigned int id,
- const uint8_t *value, size_t len,
- uint8_t link_type)
+ const uint8_t *value, size_t len)
{
struct pending_op *op;
- op = pending_write_new(device, NULL, attrib, id, value, len, 0,
- link_type);
+ op = pending_write_new(att, chrc->pending_writes, attrib, id, value,
+ len, 0);
if (g_dbus_proxy_method_call(chrc->proxy, "AcquireWrite",
acquire_write_setup,
@@ -2295,40 +2347,6 @@ static bool database_add_cep(struct exte
return true;
}
-static struct btd_device *att_get_device(struct bt_att *att)
-{
- GIOChannel *io = NULL;
- GError *gerr = NULL;
- bdaddr_t src, dst;
- uint8_t dst_type;
- struct btd_adapter *adapter;
-
- io = g_io_channel_unix_new(bt_att_get_fd(att));
- if (!io)
- return NULL;
-
- bt_io_get(io, &gerr, BT_IO_OPT_SOURCE_BDADDR, &src,
- BT_IO_OPT_DEST_BDADDR, &dst,
- BT_IO_OPT_DEST_TYPE, &dst_type,
- BT_IO_OPT_INVALID);
- if (gerr) {
- error("bt_io_get: %s", gerr->message);
- g_error_free(gerr);
- g_io_channel_unref(io);
- return NULL;
- }
-
- g_io_channel_unref(io);
-
- adapter = adapter_find(&src);
- if (!adapter) {
- error("Unable to find adapter object");
- return NULL;
- }
-
- return btd_adapter_find_device(adapter, &dst, dst_type);
-}
-
static void desc_read_cb(struct gatt_db_attribute *attrib,
unsigned int id, uint16_t offset,
uint8_t opcode, struct bt_att *att,
@@ -2348,8 +2366,8 @@ static void desc_read_cb(struct gatt_db_
goto fail;
}
- if (send_read(device, attrib, desc->proxy, desc->pending_reads, id,
- offset, bt_att_get_link_type(att)))
+ if (send_read(att, attrib, desc->proxy, desc->pending_reads, id,
+ offset))
return;
fail:
@@ -2377,8 +2395,8 @@ static void desc_write_cb(struct gatt_db
goto fail;
}
- if (send_write(device, attrib, desc->proxy, desc->pending_writes, id,
- value, len, offset, bt_att_get_link_type(att)))
+ if (send_write(att, attrib, desc->proxy, desc->pending_writes, id,
+ value, len, offset))
return;
fail:
@@ -2428,8 +2446,8 @@ static void chrc_read_cb(struct gatt_db_
goto fail;
}
- if (send_read(device, attrib, chrc->proxy, chrc->pending_reads, id,
- offset, bt_att_get_link_type(att)))
+ if (send_read(att, attrib, chrc->proxy, chrc->pending_reads, id,
+ offset))
return;
fail:
@@ -2470,8 +2488,7 @@ static void chrc_write_cb(struct gatt_db
}
if (g_dbus_proxy_get_property(chrc->proxy, "WriteAcquired", &iter)) {
- if (acquire_write(chrc, device, attrib, id, value, len,
- bt_att_get_link_type(att)))
+ if (acquire_write(chrc, att, attrib, id, value, len))
return;
}
@@ -2480,8 +2497,8 @@ static void chrc_write_cb(struct gatt_db
else
queue = NULL;
- if (send_write(device, attrib, chrc->proxy, queue, id, value, len,
- offset, bt_att_get_link_type(att)))
+ if (send_write(att, attrib, chrc->proxy, queue, id, value, len,
+ offset))
return;
fail:
