PTS test GATT/CL/GAD/BV-03-C published a service starting at handle 0xfffd
and ending at 0xffff.
This resets the next_handle to 0 in gatt_db_insert_service() instead of
setting it to 0x10000. Other services are added later.
This could end-up by a crash in db_hash_update() if not enough space has
been allocated for hash.iov and some entries are overwritten.
Next_handle can be replaced by a last_handle variable which will not loop
over. This can be replaced by queue_peek_tail() and computing the value,
but keeping last_handle will avoid this sort of lookup.
Add a unit test to check regression.
v1 -> v2: Replace next_handle by last_handle
Check empty db using gatt_db_isempty(db) instead of
next_handle == 0
Add robustness unit test to check that gatt_db_get_hash()
doesn't crash
v2 -> v3: Fix line length checkpatch errors
v3 -> v4: Update commit comment to explain reason for keeping last_handle
Split unit test to its own commit
Rephrase db setup comment in unit test
Frédéric Danis (2):
shared/gatt-db: Fix munmap_chunk invalid pointer
unit/test-gatt: Add unordered setup db test
src/shared/gatt-db.c | 19 ++++++------
unit/test-gatt.c | 73 +++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 82 insertions(+), 10 deletions(-)
--
2.34.1
PTS test GATT/CL/GAD/BV-03-C published a service starting at handle 0xfffd
and ending at 0xffff.
This resets the next_handle to 0 in gatt_db_insert_service() instead of
setting it to 0x10000. Other services are added later.
This could end-up by a crash in db_hash_update() if not enough space has
been allocated for hash.iov and some entries are overwritten.
Next_handle can be replaced by a last_handle variable which will not loop
over. This can be replaced by queue_peek_tail() and computing the value,
but keeping last_handle will avoid this sort of lookup.
---
src/shared/gatt-db.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 676f963ec..9559583d1 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -58,7 +58,7 @@ struct gatt_db {
struct bt_crypto *crypto;
uint8_t hash[16];
unsigned int hash_id;
- uint16_t next_handle;
+ uint16_t last_handle;
struct queue *services;
struct queue *notify_list;
@@ -255,7 +255,7 @@ struct gatt_db *gatt_db_new(void)
db->crypto = bt_crypto_new();
db->services = queue_new();
db->notify_list = queue_new();
- db->next_handle = 0x0001;
+ db->last_handle = 0x0000;
return gatt_db_ref(db);
}
@@ -356,14 +356,15 @@ static bool db_hash_update(void *user_data)
db->hash_id = 0;
- if (!db->next_handle)
+ if (gatt_db_isempty(db))
return false;
- hash.iov = new0(struct iovec, db->next_handle);
+ hash.iov = new0(struct iovec, db->last_handle + 1);
hash.i = 0;
gatt_db_foreach_service(db, NULL, service_gen_hash_m, &hash);
- bt_crypto_gatt_hash(db->crypto, hash.iov, db->next_handle, db->hash);
+ bt_crypto_gatt_hash(db->crypto, hash.iov, db->last_handle + 1,
+ db->hash);
for (i = 0; i < hash.i; i++)
free(hash.iov[i].iov_base);
@@ -624,7 +625,7 @@ bool gatt_db_clear_range(struct gatt_db *db, uint16_t start_handle,
done:
if (gatt_db_isempty(db))
- db->next_handle = 0;
+ db->last_handle = 0;
return true;
}
@@ -700,7 +701,7 @@ struct gatt_db_attribute *gatt_db_insert_service(struct gatt_db *db,
return NULL;
if (!handle)
- handle = db->next_handle;
+ handle = db->last_handle + 1;
if (num_handles < 1 || (handle + num_handles - 1) > UINT16_MAX)
return NULL;
@@ -747,8 +748,8 @@ struct gatt_db_attribute *gatt_db_insert_service(struct gatt_db *db,
service->attributes[0]->handle = handle;
service->num_handles = num_handles;
- /* Fast-forward next_handle if the new service was added to the end */
- db->next_handle = MAX(handle + num_handles, db->next_handle);
+ /* Fast-forward last_handle if the new service was added to the end */
+ db->last_handle = MAX(handle + num_handles - 1, db->last_handle);
return service->attributes[0];
--
2.34.1
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=816181
---Test result---
Test Summary:
CheckPatch PASS 1.10 seconds
GitLint PASS 0.83 seconds
BuildEll PASS 23.87 seconds
BluezMake PASS 727.83 seconds
MakeCheck PASS 11.38 seconds
MakeDistcheck PASS 159.36 seconds
CheckValgrind PASS 221.84 seconds
CheckSmatch PASS 327.53 seconds
bluezmakeextell PASS 105.96 seconds
IncrementalBuild PASS 1352.37 seconds
ScanBuild PASS 930.29 seconds
---
Regards,
Linux Bluetooth
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Thu, 11 Jan 2024 14:39:53 +0100 you wrote:
> PTS test GATT/CL/GAD/BV-03-C published a service starting at handle 0xfffd
> and ending at 0xffff.
> This resets the next_handle to 0 in gatt_db_insert_service() instead of
> setting it to 0x10000. Other services are added later.
> This could end-up by a crash in db_hash_update() if not enough space has
> been allocated for hash.iov and some entries are overwritten.
>
> [...]
Here is the summary with links:
- [v4,1/2] shared/gatt-db: Fix munmap_chunk invalid pointer
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=dacc69373263
- [v4,2/2] unit/test-gatt: Add unordered setup db test
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html