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.

Signed-off-by: Anuj Jain <anuj01.jain@samsung.com>
Signed-off-by: Ayush Garg <ayush.garg@samsung.com>

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 87474ff..fb66bf7 100644 (file)
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -141,8 +141,9 @@ struct external_desc {
 };
 
 struct pending_op {
-       struct btd_device *device;
+       struct bt_att *att;
        unsigned int id;
+       unsigned int disconn_id;
 #ifdef TIZEN_FEATURE_BLUEZ_MODIFY
        uint8_t opcode;
 #endif
@@ -1081,6 +1082,26 @@ static struct btd_device *att_get_device(struct bt_att *att)
        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 struct pending_op *pending_ccc_new(struct bt_att *att,
                                        struct gatt_db_attribute *attrib,
                                        uint16_t value,
@@ -1100,21 +1121,16 @@ static struct pending_op *pending_ccc_new(struct bt_att *att,
        op->data.iov_base = UINT_TO_PTR(value);
        op->data.iov_len = sizeof(value);
 
-       op->device = device;
+       op->att = bt_att_ref(att);
        op->attrib = attrib;
        op->link_type = link_type;
 
-       return op;
-}
+       bt_att_register_disconnect(att,
+                                  pending_disconnect_cb,
+                                  op,
+                                  NULL);
 
-static void pending_op_free(void *data)
-{
-       struct pending_op *op = data;
-
-       if (op->owner_queue)
-               queue_remove(op->owner_queue, op);
-
-       free(op);
+       return op;
 }
 
 static void gatt_ccc_write_cb(struct gatt_db_attribute *attrib,
@@ -2598,31 +2614,35 @@ done:
        gatt_db_attribute_read_result(op->attrib, op->id, ecode, value, len);
 }
 
-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);
        struct bt_gatt_server *server;
        const char *link;
        uint16_t mtu;
@@ -2649,7 +2669,7 @@ static void append_options(DBusMessageIter *iter, void *user_data)
                dict_append_entry(iter, "prepare-authorize", DBUS_TYPE_BOOLEAN,
                                                        &op->prep_authorize);
 
-       server = btd_device_get_gatt_server(op->device);
+       server = btd_device_get_gatt_server(device);
        mtu = bt_gatt_server_get_mtu(server);
 
        dict_append_entry(iter, "mtu", DBUS_TYPE_UINT16, &mtu);
@@ -2662,10 +2682,10 @@ static void read_setup_cb(DBusMessageIter *iter, void *user_data)
        char dst_addr[18] = { 0 };
        char *addr_value = dst_addr;
 
-       if (device_get_rpa_exist(op->device) == true) {
-               ba2str(device_get_rpa(op->device), dst_addr);
+       if (device_get_rpa_exist(att_get_device(op->att)) == true) {
+               ba2str(device_get_rpa(att_get_device(op->att)), dst_addr);
        } else {
-               ba2str(device_get_address(op->device), dst_addr);
+               ba2str(device_get_address(att_get_device(op->att)), dst_addr);
        }
 
        dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &addr_value);
@@ -2687,18 +2707,16 @@ static void read_setup_cb(DBusMessageIter *iter, void *user_data)
 #endif
 }
 
-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)
@@ -2720,10 +2738,10 @@ static void write_setup_cb(DBusMessageIter *iter, void *user_data)
        gboolean response_needed = FALSE;
        gboolean prep_auth = FALSE;
 
-       if (device_get_rpa_exist(op->device) == true) {
-               ba2str(device_get_rpa(op->device), dst_addr);
+       if (device_get_rpa_exist(att_get_device(op->att)) == true) {
+               ba2str(device_get_rpa(att_get_device(op->att)), dst_addr);
        } else {
-               ba2str(device_get_address(op->device), dst_addr);
+               ba2str(device_get_address(att_get_device(op->att)), dst_addr);
        }
 
        dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &addr_value);
@@ -2814,10 +2832,12 @@ static void write_reply_cb(DBusMessage *message, void *user_data)
        }
 
 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,
@@ -2825,7 +2845,7 @@ static struct pending_op *pending_write_new(struct btd_device *device,
 #ifdef TIZEN_FEATURE_BLUEZ_MODIFY
                                        uint8_t opcode,
 #endif
-                                       uint16_t offset, uint8_t link_type,
+                                       uint16_t offset,
                                        bool is_characteristic,
                                        bool prep_authorize)
 {
@@ -2836,7 +2856,7 @@ static struct pending_op *pending_write_new(struct btd_device *device,
        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;
@@ -2844,15 +2864,18 @@ static struct pending_op *pending_write_new(struct btd_device *device,
        op->opcode = opcode;
 #endif
        op->offset = offset;
-       op->link_type = link_type;
+       op->link_type = bt_att_get_link_type(att);
        op->is_characteristic = is_characteristic;
        op->prep_authorize = prep_authorize;
        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,
@@ -2861,17 +2884,17 @@ static struct pending_op *send_write(struct btd_device *device,
 #ifdef TIZEN_FEATURE_BLUEZ_MODIFY
                                        uint8_t opcode,
 #endif
-                                       uint16_t offset, uint8_t link_type,
+                                       uint16_t offset,
                                        bool is_characteristic,
                                        bool prep_authorize)
 {
        struct pending_op *op;
 
-       op = pending_write_new(device, owner_queue, attrib, id, value, len,
+       op = pending_write_new(att, owner_queue, attrib, id, value, len,
 #ifdef TIZEN_FEATURE_BLUEZ_MODIFY
                                        opcode,
 #endif
-                                       offset, link_type, is_characteristic,
+                                       offset, is_characteristic,
                                        prep_authorize);
 
        if (g_dbus_proxy_method_call(proxy, "WriteValue", write_setup_cb,
@@ -3052,21 +3075,20 @@ static void acquire_write_setup(DBusMessageIter *iter, void *user_data)
 }
 
 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;
        bool acquiring = !queue_isempty(chrc->pending_writes);
 
 #ifdef TIZEN_FEATURE_BLUEZ_MODIFY
-       op = pending_write_new(device, chrc->pending_writes, attrib, id, value, len, 0, 0,
+       op = pending_write_new(att, chrc->pending_writes, attrib, id, value, len, 0, 0,
 #else
-       op = pending_write_new(device, chrc->pending_writes, attrib, id, value, len, 0,
+       op = pending_write_new(att, chrc->pending_writes, attrib, id, value, len, 0,
 #endif
-                                               link_type, false, false);
+                                               false, false);
 
        if (acquiring)
                return op;
@@ -3086,7 +3108,7 @@ static struct pending_op *acquire_write(struct external_chrc *chrc,
 static void start_notify_setup(DBusMessageIter *iter, void *user_data)
 {
        struct pending_op *op = user_data;
-       struct btd_device *device = op->device;
+       struct btd_device *device = att_get_device(op->att);
        char dst_addr[18] = { 0 };
        char *addr_value = dst_addr;
 
@@ -3175,7 +3197,7 @@ static void acquire_notify_setup(DBusMessageIter *iter, void *user_data)
 static void stop_notify_setup(DBusMessageIter *iter, void *user_data)
 {
        struct pending_op *op = user_data;
-       struct btd_device *device = op->device;
+       struct btd_device *device = att_get_device(op->att);
        char dst_addr[18] = { 0 };
        char *addr_value = dst_addr;
 
@@ -3513,8 +3535,8 @@ static void desc_read_cb(struct gatt_db_attribute *attrib,
                error("Unable to find device object");
                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:
@@ -3545,13 +3567,12 @@ static void desc_write_cb(struct gatt_db_attribute *attrib,
        if (opcode == BT_ATT_OP_PREP_WRITE_REQ) {
                if (!device_is_trusted(device) && !desc->prep_authorized &&
                                                desc->req_prep_authorization)
-                       send_write(device, attrib, desc->proxy,
+                       send_write(att, attrib, desc->proxy,
                                        desc->pending_writes, id, value, len,
 #ifdef TIZEN_FEATURE_BLUEZ_MODIFY
                                        opcode,
 #endif
-                                       offset, bt_att_get_link_type(att),
-                                       false, true);
+                                       offset, false, true);
                else
                        gatt_db_attribute_write_result(attrib, id, 0);
 
@@ -3561,11 +3582,11 @@ static void desc_write_cb(struct gatt_db_attribute *attrib,
        if (opcode == BT_ATT_OP_EXEC_WRITE_REQ)
                desc->prep_authorized = false;
 
-       if (send_write(device, attrib, desc->proxy, desc->pending_writes, id,
+       if (send_write(att, attrib, desc->proxy, desc->pending_writes, id,
 #ifdef TIZEN_FEATURE_BLUEZ_MODIFY
-                       value, len, opcode, offset, bt_att_get_link_type(att), false,
+                       value, len, opcode, offset, false,
 #else
-                       value, len, offset, bt_att_get_link_type(att), false,
+                       value, len, offset, false,
 #endif
                        false))
                return;
@@ -3645,8 +3666,8 @@ static void chrc_read_cb(struct gatt_db_attribute *attrib,
                error("Unable to find device object");
                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:
@@ -3684,13 +3705,13 @@ static void chrc_write_cb(struct gatt_db_attribute *attrib,
        if (opcode == BT_ATT_OP_PREP_WRITE_REQ) {
                if (!device_is_trusted(device) && !chrc->prep_authorized &&
                                                chrc->req_prep_authorization)
-                       send_write(device, attrib, chrc->proxy, queue,
+                       send_write(att, attrib, chrc->proxy, queue,
 #ifdef TIZEN_FEATURE_BLUEZ_MODIFY
                                        id, value, len, opcode, offset,
 #else
                                        id, value, len, offset,
 #endif
-                                       bt_att_get_link_type(att), true, true);
+                                       true, true);
                else
                        gatt_db_attribute_write_result(attrib, id, 0);
 
@@ -3711,16 +3732,15 @@ static void chrc_write_cb(struct gatt_db_attribute *attrib,
        }
 
        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;
        }
 
-       if (send_write(device, attrib, chrc->proxy, queue, id, value, len,
+       if (send_write(att, attrib, chrc->proxy, queue, id, value, len,
 #ifdef TIZEN_FEATURE_BLUEZ_MODIFY
                        opcode,
 #endif
-                       offset, bt_att_get_link_type(att), false, false))
+                       offset, false, false))
                return;
 
 fail: