| Message ID | 20230323103835.571037-2-simon.mikuda@streamunlimited.com |
|---|---|
| State | New |
| Headers | show |
| Series | gatt-db fix + btgatt-client features | expand |
Hi Simon, On Thu, Mar 23, 2023 at 3:44 AM Simon Mikuda <simon.mikuda@streamunlimited.com> wrote: > > It happens when next_handle is lower that discovered number of handles. > Found by PTS test case: GATT/CL/GAD/BC-01-C Can you add the backtrace of the crash? > --- > src/shared/gatt-db.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c > index b696fe33d..c9ffbfeed 100644 > --- a/src/shared/gatt-db.c > +++ b/src/shared/gatt-db.c > @@ -297,6 +297,7 @@ static void handle_notify(void *data, void *user_data) > struct hash_data { > struct iovec *iov; > uint16_t i; > + size_t size; > }; > > static void gen_hash_m(struct gatt_db_attribute *attr, void *user_data) > @@ -327,7 +328,7 @@ static void gen_hash_m(struct gatt_db_attribute *attr, void *user_data) > case GATT_CHARAC_AGREG_FMT_UUID: > /* Allocate space for handle + type */ > len = 2 + 2; > - data = malloc(2 + 2 + attr->value_len); > + data = malloc(2 + 2); This seems to be a different issue, looks like we are allocating more than necessary. > put_le16(attr->handle, data); > bt_uuid_to_le(&attr->uuid, data + 2); > break; > @@ -335,6 +336,13 @@ static void gen_hash_m(struct gatt_db_attribute *attr, void *user_data) > return; > } > > + if (hash->i >= hash->size) { > + /* double the size of iov if we've run out of space */ > + hash->iov = realloc(hash->iov, 2 * hash->size * sizeof(struct iovec)); > + memset(hash->iov + hash->size, 0, hash->size * sizeof(struct iovec)); > + hash->size *= 2; Not sure if we should double the size? I'd probably check why we are not able to allocate the size properly, perhaps we have an off by one of the next_handle happens to loop around? A better way would be to just calculate the actual number of attributes instead of using its handles since there could be spaces in between handles, we could just iterate over the services since they should each contain the number of attributes. > + } > + > hash->iov[hash->i].iov_base = data; > hash->iov[hash->i].iov_len = len; > > @@ -361,9 +369,10 @@ static bool db_hash_update(void *user_data) > > hash.iov = new0(struct iovec, db->next_handle); > hash.i = 0; > + hash.size = db->next_handle; > > 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, hash.i, db->hash); > > for (i = 0; i < hash.i; i++) > free(hash.iov[i].iov_base); > -- > 2.34.1 >
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c index b696fe33d..c9ffbfeed 100644 --- a/src/shared/gatt-db.c +++ b/src/shared/gatt-db.c @@ -297,6 +297,7 @@ static void handle_notify(void *data, void *user_data) struct hash_data { struct iovec *iov; uint16_t i; + size_t size; }; static void gen_hash_m(struct gatt_db_attribute *attr, void *user_data) @@ -327,7 +328,7 @@ static void gen_hash_m(struct gatt_db_attribute *attr, void *user_data) case GATT_CHARAC_AGREG_FMT_UUID: /* Allocate space for handle + type */ len = 2 + 2; - data = malloc(2 + 2 + attr->value_len); + data = malloc(2 + 2); put_le16(attr->handle, data); bt_uuid_to_le(&attr->uuid, data + 2); break; @@ -335,6 +336,13 @@ static void gen_hash_m(struct gatt_db_attribute *attr, void *user_data) return; } + if (hash->i >= hash->size) { + /* double the size of iov if we've run out of space */ + hash->iov = realloc(hash->iov, 2 * hash->size * sizeof(struct iovec)); + memset(hash->iov + hash->size, 0, hash->size * sizeof(struct iovec)); + hash->size *= 2; + } + hash->iov[hash->i].iov_base = data; hash->iov[hash->i].iov_len = len; @@ -361,9 +369,10 @@ static bool db_hash_update(void *user_data) hash.iov = new0(struct iovec, db->next_handle); hash.i = 0; + hash.size = db->next_handle; 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, hash.i, db->hash); for (i = 0; i < hash.i; i++) free(hash.iov[i].iov_base);