Hi Luiz,
Thanks for the response.
This issue was found in ChromeOS edition BlueZ. I'm not sure if any
official BlueZ would do the same discovery procedure or not.
I extracted relevant part of the trace as below
==========================
< ACL Data TX: Handle 2048 flags 0x00 dlen 11
#43961 [hci0]
2024-03-05 07:33:55.936283
ATT: Read By Group Type Request (0x10) len 6
Handle range: 0x001f-0xffff
Attribute group type: Primary Service (0x2800)
...
> ACL Data RX: Handle 2048 flags 0x02 dlen 27 #43963 [hci0] 2024-03-05 07:33:55.967020
> ACL Data RX: Handle 2048 flags 0x01 dlen 3 #43964 [hci0] 2024-03-05 07:33:55.967022
ATT: Read By Group Type Response (0x11) len 25
Attribute data length: 6
Attribute group list: 4 entries
Handle range: 0x001f-0x0035
UUID: Human Interface Device (0x1812)
Handle range: 0x0036-0x0044
UUID: Human Interface Device (0x1812)
Handle range: 0x0045-0x0055
UUID: Human Interface Device (0x1812)
Handle range: 0x0056-0x0062
UUID: Logitech International SA (0xfd72)
…
< ACL Data TX: Handle 2048 flags 0x00 dlen 11
#44018 [hci0]
2024-03-05 07:33:56.281436
ATT: Read By Type Request (0x08) len 6
Handle range: 0x0021-0xffff
Attribute type: Characteristic (0x2803)
...
> ACL Data RX: Handle 2048 flags 0x02 dlen 27 #44023 [hci0] 2024-03-05 07:33:56.311026
ATT: Read By Type Response (0x09) len 22
Attribute data length: 7
Attribute data list: 3 entries
Handle: 0x0022
Value: 122300332a
Properties: 0x12
Read (0x02)
Notify (0x10)
Value Handle: 0x0023
Value UUID: Boot Mouse Input Report (0x2a33)
Handle: 0x0025
Value: 0226004b2a
Properties: 0x02
Read (0x02)
Value Handle: 0x0026
Value UUID: Report Map (0x2a4b)
Handle: 0x0027
Value: 1228004d2a
Properties: 0x12
Read (0x02)
Notify (0x10)
Value Handle: 0x0028
Value UUID: Report (0x2a4d)
...
< ACL Data TX: Handle 2048 flags 0x00 dlen 11
#44024 [hci0]
2024-03-05 07:33:56.311458
ATT: Read By Type Request (0x08) len 6
Handle range: 0x0028-0xffff
Attribute type: Characteristic (0x2803)
…
> ACL Data RX: Handle 2048 flags 0x02 dlen 27 #44029 [hci0] 2024-03-05 07:33:56.341025
ATT: Read By Type Response (0x09) len 22
Attribute data length: 7
Attribute data list: 3 entries
Handle: 0x002b
Value: 122c004d2a
Properties: 0x12
Read (0x02)
Notify (0x10)
Value Handle: 0x002c
Value UUID: Report (0x2a4d)
Handle: 0x002f
Value: 0e30004d2a
Properties: 0x0e
Read (0x02)
Write Without Response (0x04)
Write (0x08)
Value Handle: 0x0030
Value UUID: Report (0x2a4d)
Handle: 0x0032
Value: 0433004c2a
Properties: 0x04
Write Without Response (0x04)
Value Handle: 0x0033
Value UUID: HID Control Point (0x2a4c)
…
< ACL Data TX: Handle 2048 flags 0x00 dlen 9
#44071 [hci0]
2024-03-05 07:33:56.702334
ATT: Find Information Request (0x04) len 4
Handle range: 0x0029-0x002a
…
> ACL Data RX: Handle 2048 flags 0x02 dlen 14 #44073 [hci0] 2024-03-05 07:33:56.731069
ATT: Find Information Response (0x05) len 9
Format: UUID-16 (0x01)
Handle: 0x0029
UUID: Client Characteristic Configuration (0x2902)
Handle: 0x002a
UUID: Report Reference (0x2908)
=============================
When decoding packet #44073, we can't append descriptors 0x0029 and
0x002a at the tail of the |service->attributes| since the
characteristics 0x002b,0x002f and 0x0032 are already in the array.
On Wed, Apr 3, 2024 at 10:37 PM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Howard,
>
> On Wed, Apr 3, 2024 at 7:06 AM Howard Chung <howardchung@xxxxxxxxxx> wrote:
> >
> > service->attributes has an assumption that it should look like:
> > |char_uuid|char_1|desc_1_1|desc_1_2|char_uuid|char_2|desc_2_1|char_uuid|...
> > where desc_x_y means a descriptor of char_x.
>
> Will need to check the trace but I believe BlueZ always discover all
> characteristics before moving to descriptors, if that is not happening
> they there is probably a bug somewhere, that said perhaps you are
> doing the discovery procedure with another stack which doesn't employ
> the same logic, although inefficient it is possible to discover out of
> order but that would require reassigning the descriptors to
> characteristics once they are found, which is also inefficient but I
> would understand if you after supporting other stacks.
>
> > In monitor, an attribute is inserted as soon as it is found. It is not
> > guranteed that all the descriptors of a characteristic would be found
> > before another characteristic is found.
>
> You might want to include such a trace, don't recall seeing any stack
> that does out-order discover.
>
> > This adds a function to insert the descriptor with the given handle to
> > an appropriate place in its service attribute list and make monitor to
> > use this function when a descripter is found.
> >
> > Reviewed-by: Archie Pusaka <apusaka@xxxxxxxxxxxx>
> > ---
> >
> > monitor/att.c | 2 +-
> > src/shared/gatt-db.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
> > src/shared/gatt-db.h | 9 ++++++
> > 3 files changed, 76 insertions(+), 1 deletion(-)
> >
> > diff --git a/monitor/att.c b/monitor/att.c
> > index ddeb54d9e..503099745 100644
> > --- a/monitor/att.c
> > +++ b/monitor/att.c
> > @@ -4153,7 +4153,7 @@ static struct gatt_db_attribute *insert_desc(const struct l2cap_frame *frame,
> > if (!db)
> > return NULL;
> >
> > - return gatt_db_append_descriptor(db, handle, uuid, 0, NULL, NULL, NULL);
> > + return gatt_db_insert_descriptor(db, handle, uuid, 0, NULL, NULL, NULL);
> > }
> >
> > static void att_find_info_rsp_16(const struct l2cap_frame *frame)
> > diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> > index 39bba9b54..f08c5afaa 100644
> > --- a/src/shared/gatt-db.c
> > +++ b/src/shared/gatt-db.c
> > @@ -1002,6 +1002,72 @@ service_append_descriptor(struct gatt_db_service *service,
> > return service->attributes[i];
> > }
> >
> > +struct gatt_db_attribute *
> > +gatt_db_insert_descriptor(struct gatt_db *db,
> > + uint16_t handle,
> > + const bt_uuid_t *uuid,
> > + uint32_t permissions,
> > + gatt_db_read_t read_func,
> > + gatt_db_write_t write_func,
> > + void *user_data)
> > +{
> > + struct gatt_db_attribute *attrib, *iter_attr, *char_attr = NULL;
> > + struct gatt_db_service *service;
> > + int i, j, num_index, char_index;
> > +
> > + attrib = gatt_db_get_service(db, handle);
> > + if (!attrib)
> > + return NULL;
> > +
> > + service = attrib->service;
> > + num_index = get_attribute_index(service, 0);
> > + if (!num_index)
> > + return NULL;
> > +
> > + // Find the characteristic the descriptor belongs to.
> > + for (i = 0; i < num_index; i++) {
> > + iter_attr = service->attributes[i];
> > + if (bt_uuid_cmp(&characteristic_uuid, &iter_attr->uuid))
> > + continue;
> > +
> > + if (iter_attr->handle > handle)
> > + continue;
> > +
> > + // Find the characteristic with the largest handle among those
> > + // with handles less than the descriptor's handle.
> > + if (!char_attr || iter_attr->handle > char_attr->handle) {
> > + char_attr = iter_attr;
> > + char_index = i;
> > + }
> > + }
> > +
> > + // There is no characteristic contain this descriptor. Something went
> > + // wrong
> > + if (!char_attr)
> > + return NULL;
> > +
> > + // Find the end of this characteristic
> > + for (i = char_index + 1; i < num_index; i++) {
> > + iter_attr = service->attributes[i];
> > +
> > + if (!bt_uuid_cmp(&characteristic_uuid, &iter_attr->uuid) ||
> > + !bt_uuid_cmp(&included_service_uuid, &iter_attr->uuid))
> > + break;
> > + }
> > +
> > + // Move all of the attributes after the end of this characteristic to
> > + // its next index.
> > + for (j = num_index; j > i; j--)
> > + service->attributes[j] = service->attributes[j - 1];
> > +
> > + service->attributes[i] = new_attribute(service, handle, uuid, NULL, 0);
> > +
> > + set_attribute_data(service->attributes[i], read_func, write_func,
> > + permissions, user_data);
> > +
> > + return service->attributes[i];
> > +}
> > +
> > struct gatt_db_attribute *
> > gatt_db_append_descriptor(struct gatt_db *db,
> > uint16_t handle,
> > diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
> > index ec0eccdfc..4c4e88d87 100644
> > --- a/src/shared/gatt-db.h
> > +++ b/src/shared/gatt-db.h
> > @@ -80,6 +80,15 @@ gatt_db_append_characteristic(struct gatt_db *db,
> > gatt_db_write_t write_func,
> > void *user_data);
> >
> > +struct gatt_db_attribute *
> > +gatt_db_insert_descriptor(struct gatt_db *db,
> > + uint16_t handle,
> > + const bt_uuid_t *uuid,
> > + uint32_t permissions,
> > + gatt_db_read_t read_func,
> > + gatt_db_write_t write_func,
> > + void *user_data);
> > +
> > struct gatt_db_attribute *
> > gatt_db_append_descriptor(struct gatt_db *db,
> > uint16_t handle,
> > --
> > 2.44.0.478.gd926399ef9-goog
> >
>
>
> --
> Luiz Augusto von Dentz
