* [PATCH BlueZ 1/2] shared/gatt-db: Introduce gatt_db_attribute_set_fixed_length @ 2021-01-06 0:55 Luiz Augusto von Dentz 2021-01-06 0:55 ` [PATCH BlueZ 2/2] gatt: Make use of gatt_db_attribute_set_fixed_length Luiz Augusto von Dentz 2021-01-06 1:36 ` [BlueZ,1/2] shared/gatt-db: Introduce gatt_db_attribute_set_fixed_length bluez.test.bot 0 siblings, 2 replies; 4+ messages in thread From: Luiz Augusto von Dentz @ 2021-01-06 0:55 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> This enables user to inform if an attribute has a fixed length so it can automatically perform bounds checking. --- src/shared/gatt-db.c | 68 ++++++++++++++++++++++++++++++++++++-------- src/shared/gatt-db.h | 3 ++ 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c index 54e64d0e0..8bff4d37a 100644 --- a/src/shared/gatt-db.c +++ b/src/shared/gatt-db.c @@ -1836,6 +1836,38 @@ static uint8_t attribute_authorize(struct gatt_db_attribute *attrib, return db->authorize(attrib, opcode, att, db->authorize_data); } +bool gatt_db_attribute_set_fixed_length(struct gatt_db_attribute *attrib, + uint16_t len) +{ + struct gatt_db_service *service; + + if (!attrib) + return false; + + service = attrib->service; + + /* Don't allow overwriting length of service attribute */ + if (attrib->service->attributes[0] == attrib) + return false; + + /* If attribute is a characteristic declaration ajust to its value */ + if (!bt_uuid_cmp(&characteristic_uuid, &attrib->uuid)) { + int i; + + /* Start from the attribute following the value handle */ + for (i = 0; i < service->num_handles; i++) { + if (service->attributes[i] == attrib) { + attrib = service->attributes[i + 1]; + break; + } + } + } + + attrib->value_len = len; + + return true; +} + bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset, uint8_t opcode, struct bt_att *att, gatt_db_attribute_read_t func, void *user_data) @@ -1845,6 +1877,12 @@ bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset, if (!attrib || !func) return false; + /* Check boundaries if value_len is set */ + if (attrib->value_len && offset > attrib->value_len) { + func(attrib, BT_ATT_ERROR_INVALID_OFFSET, NULL, 0, user_data); + return true; + } + if (attrib->read_func) { struct pending_read *p; uint8_t err; @@ -1870,12 +1908,6 @@ bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset, return true; } - /* Check boundary if value is stored in the db */ - if (offset > attrib->value_len) { - func(attrib, BT_ATT_ERROR_INVALID_OFFSET, NULL, 0, user_data); - return true; - } - /* Guard against invalid access if offset equals to value length */ value = offset == attrib->value_len ? NULL : &attrib->value[offset]; @@ -1930,19 +1962,31 @@ bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset, gatt_db_attribute_write_t func, void *user_data) { + uint8_t err = 0; + if (!attrib || !func) return false; if (attrib->write_func) { struct pending_write *p; - uint8_t err; - err = attribute_authorize(attrib, opcode, att); - if (err) { - func(attrib, err, user_data); - return true; + /* Check boundaries if value_len is set */ + if (attrib->value_len) { + if (offset > attrib->value_len) { + err = BT_ATT_ERROR_INVALID_OFFSET; + goto done; + } + + if (offset + len > attrib->value_len) { + err = BT_ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN; + goto done; + } } + err = attribute_authorize(attrib, opcode, att); + if (err) + goto done; + p = new0(struct pending_write, 1); p->attrib = attrib; p->id = ++attrib->write_id; @@ -1983,7 +2027,7 @@ bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset, memcpy(&attrib->value[offset], value, len); done: - func(attrib, 0, user_data); + func(attrib, err, user_data); return true; } diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h index a0da33065..321a2aba6 100644 --- a/src/shared/gatt-db.h +++ b/src/shared/gatt-db.h @@ -241,6 +241,9 @@ bool gatt_db_attribute_get_incl_data(const struct gatt_db_attribute *attrib, uint32_t gatt_db_attribute_get_permissions(const struct gatt_db_attribute *attrib); +bool gatt_db_attribute_set_fixed_length(struct gatt_db_attribute *attrib, + uint16_t len); + typedef void (*gatt_db_attribute_read_t) (struct gatt_db_attribute *attrib, int err, const uint8_t *value, size_t length, void *user_data); -- 2.26.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH BlueZ 2/2] gatt: Make use of gatt_db_attribute_set_fixed_length 2021-01-06 0:55 [PATCH BlueZ 1/2] shared/gatt-db: Introduce gatt_db_attribute_set_fixed_length Luiz Augusto von Dentz @ 2021-01-06 0:55 ` Luiz Augusto von Dentz 2021-01-06 1:36 ` [BlueZ,1/2] shared/gatt-db: Introduce gatt_db_attribute_set_fixed_length bluez.test.bot 1 sibling, 0 replies; 4+ messages in thread From: Luiz Augusto von Dentz @ 2021-01-06 0:55 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> This makes use of gatt_db_attribute_set_fixed_length so the database is aware of the length of the values and perform bounds checking. --- src/gatt-database.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/gatt-database.c b/src/gatt-database.c index f2d7b5821..b7d2bea1d 100644 --- a/src/gatt-database.c +++ b/src/gatt-database.c @@ -161,12 +161,14 @@ struct notify { void *user_data; }; +#define CLI_FEAT_SIZE 1 + struct device_state { struct btd_gatt_database *db; bdaddr_t bdaddr; uint8_t bdaddr_type; unsigned int disc_id; - uint8_t cli_feat[1]; + uint8_t cli_feat[CLI_FEAT_SIZE]; bool change_aware; bool out_of_sync; struct queue *ccc_states; @@ -688,18 +690,12 @@ static void gap_appearance_read_cb(struct gatt_db_attribute *attrib, dev_class = btd_adapter_get_class(database->adapter); - if (offset > 2) { - error = BT_ATT_ERROR_INVALID_OFFSET; - goto done; - } - appearance[0] = dev_class & 0x00ff; appearance[1] = (dev_class >> 8) & 0x001f; len -= offset; value = len ? &appearance[offset] : NULL; -done: gatt_db_attribute_read_result(attrib, id, error, value, len); } @@ -820,7 +816,7 @@ static void database_add_record(struct btd_gatt_database *database, static void populate_gap_service(struct btd_gatt_database *database) { bt_uuid_t uuid; - struct gatt_db_attribute *service; + struct gatt_db_attribute *service, *attrib; /* Add the GAP service */ bt_uuid16_create(&uuid, UUID_GAP); @@ -839,10 +835,12 @@ static void populate_gap_service(struct btd_gatt_database *database) * Device Appearance characteristic. */ bt_uuid16_create(&uuid, GATT_CHARAC_APPEARANCE); - gatt_db_service_add_characteristic(service, &uuid, BT_ATT_PERM_READ, + attrib = gatt_db_service_add_characteristic(service, &uuid, + BT_ATT_PERM_READ, BT_GATT_CHRC_PROP_READ, gap_appearance_read_cb, NULL, database); + gatt_db_attribute_set_fixed_length(attrib, 2); gatt_db_service_set_active(service, true); @@ -865,11 +863,6 @@ static void gatt_ccc_read_cb(struct gatt_db_attribute *attrib, DBG("CCC read called for handle: 0x%04x", handle); - if (offset) { - ecode = BT_ATT_ERROR_INVALID_OFFSET; - goto done; - } - ccc = get_ccc_state(database, att, handle); if (!ccc) { ecode = BT_ATT_ERROR_UNLIKELY; @@ -1046,6 +1039,8 @@ service_add_ccc(struct gatt_db_attribute *service, return NULL; } + gatt_db_attribute_set_fixed_length(ccc, 2); + ccc_cb->handle = gatt_db_attribute_get_handle(ccc); ccc_cb->callback = write_callback; ccc_cb->destroy = destroy; @@ -1075,11 +1070,6 @@ static void cli_feat_read_cb(struct gatt_db_attribute *attrib, goto done; } - if (offset >= sizeof(state->cli_feat)) { - ecode = BT_ATT_ERROR_INVALID_OFFSET; - goto done; - } - len = sizeof(state->cli_feat) - offset; value = len ? &state->cli_feat[offset] : NULL; @@ -1208,6 +1198,7 @@ static void populate_gatt_service(struct btd_gatt_database *database) cli_feat_read_cb, cli_feat_write_cb, database); + gatt_db_attribute_set_fixed_length(database->cli_feat, CLI_FEAT_SIZE); /* Only expose database hash chrc if supported */ if (gatt_db_hash_support(database->db)) { @@ -1215,6 +1206,7 @@ static void populate_gatt_service(struct btd_gatt_database *database) database->db_hash = gatt_db_service_add_characteristic(service, &uuid, BT_ATT_PERM_READ, BT_GATT_CHRC_PROP_READ, db_hash_read_cb, NULL, database); + gatt_db_attribute_set_fixed_length(database->db_hash, 16); } /* Only enable EATT if there is a socket listening */ @@ -1223,6 +1215,7 @@ static void populate_gatt_service(struct btd_gatt_database *database) database->eatt = gatt_db_service_add_characteristic(service, &uuid, BT_ATT_PERM_READ, BT_GATT_CHRC_PROP_READ, server_feat_read_cb, NULL, database); + gatt_db_attribute_set_fixed_length(database->eatt, 1); } gatt_db_service_set_active(service, true); @@ -1254,12 +1247,15 @@ static void populate_devinfo_service(struct btd_gatt_database *database) service = gatt_db_add_service(database->db, &uuid, true, 3); if (btd_opts.did_source > 0) { + struct gatt_db_attribute *attrib; + bt_uuid16_create(&uuid, GATT_CHARAC_PNP_ID); - gatt_db_service_add_characteristic(service, &uuid, + attrib = gatt_db_service_add_characteristic(service, &uuid, BT_ATT_PERM_READ, BT_GATT_CHRC_PROP_READ, device_info_read_pnp_id_cb, NULL, database); + gatt_db_attribute_set_fixed_length(attrib, 7); } gatt_db_service_set_active(service, true); -- 2.26.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [BlueZ,1/2] shared/gatt-db: Introduce gatt_db_attribute_set_fixed_length 2021-01-06 0:55 [PATCH BlueZ 1/2] shared/gatt-db: Introduce gatt_db_attribute_set_fixed_length Luiz Augusto von Dentz 2021-01-06 0:55 ` [PATCH BlueZ 2/2] gatt: Make use of gatt_db_attribute_set_fixed_length Luiz Augusto von Dentz @ 2021-01-06 1:36 ` bluez.test.bot 2021-01-06 19:06 ` Luiz Augusto von Dentz 1 sibling, 1 reply; 4+ messages in thread From: bluez.test.bot @ 2021-01-06 1:36 UTC (permalink / raw) To: linux-bluetooth, luiz.dentz [-- Attachment #1: Type: text/plain, Size: 557 bytes --] 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=409691 ---Test result--- ############################## Test: CheckPatch - PASS ############################## Test: CheckGitLint - PASS ############################## Test: CheckBuild - PASS ############################## Test: MakeCheck - PASS --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [BlueZ,1/2] shared/gatt-db: Introduce gatt_db_attribute_set_fixed_length 2021-01-06 1:36 ` [BlueZ,1/2] shared/gatt-db: Introduce gatt_db_attribute_set_fixed_length bluez.test.bot @ 2021-01-06 19:06 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 4+ messages in thread From: Luiz Augusto von Dentz @ 2021-01-06 19:06 UTC (permalink / raw) To: linux-bluetooth@vger.kernel.org Hi, On Tue, Jan 5, 2021 at 5:36 PM <bluez.test.bot@gmail.com> wrote: > > 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=409691 > > ---Test result--- > > ############################## > Test: CheckPatch - PASS > > ############################## > Test: CheckGitLint - PASS > > ############################## > Test: CheckBuild - PASS > > ############################## > Test: MakeCheck - PASS > > > > --- > Regards, > Linux Bluetooth Pushed. -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-06 19:06 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-01-06 0:55 [PATCH BlueZ 1/2] shared/gatt-db: Introduce gatt_db_attribute_set_fixed_length Luiz Augusto von Dentz 2021-01-06 0:55 ` [PATCH BlueZ 2/2] gatt: Make use of gatt_db_attribute_set_fixed_length Luiz Augusto von Dentz 2021-01-06 1:36 ` [BlueZ,1/2] shared/gatt-db: Introduce gatt_db_attribute_set_fixed_length bluez.test.bot 2021-01-06 19:06 ` Luiz Augusto von Dentz
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.