* [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 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).