Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bluetooth: gatt: Fix ATT Read By Type by DB change unaware client #75720

Merged
merged 1 commit into from
Aug 27, 2024
Merged

bluetooth: gatt: Fix ATT Read By Type by DB change unaware client #75720

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions subsys/bluetooth/host/att.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1549,6 +1549,23 @@ static uint8_t att_read_type_req(struct bt_att_chan *chan, struct net_buf *buf)
return 0;
}

/* Reading Database Hash is special as it may be used to make client change aware
* (Core Specification 5.4 Vol 3. Part G. 2.5.2.1 Robust Caching).
*
* GATT client shall always use GATT Read Using Characteristic UUID sub-procedure for
* reading Database Hash
* (Core Specification 5.4 Vol 3. Part G. 7.3 Databse Hash)
*/
if (bt_uuid_cmp(&u.uuid, BT_UUID_GATT_DB_HASH) != 0) {
Comment on lines +1555 to +1559
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this made me think of whether we can/should reject reads to BT_UUID_GATT_DB_HASH from the other read requests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other reads (and also wrtite reqs) are already handling it (that if bt_gatt_change_aware() blocks are doing that)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are just blocking reading when the database is out of sync though, right?

The core spec states that the client shall read the database sync with the read by type request (read by UUID). That means that even if the client is not out of sync, it shall not read the characteristic with e.g. a regular read request. Arguably that is a requirement on the client side, but we could (not sure if we should) block reads of that characteristic if not with read by type

if (!bt_gatt_change_aware(chan->att->conn, true)) {
if (!atomic_test_and_set_bit(chan->flags, ATT_OUT_OF_SYNC_SENT)) {
return BT_ATT_ERR_DB_OUT_OF_SYNC;
} else {
return 0;
}
}
}

return att_read_type_rsp(chan, &u.uuid, start_handle, end_handle);
}

Expand Down
Loading