| Message ID | 20201001155738.Bluez.v4.5.Ic4a3667da774f5f34477d5168a68a9280657e2da@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bluetooth: Add new MGMT interface for advertising add | expand |
Hi Daniel, On Thu, Oct 1, 2020 at 4:05 PM Daniel Winkler <danielwinkler@google.com> wrote: > > This change adds parsers for the advertising intervals and tx power > properties of the LEAdvertisement1 object. It validates that each field > adheres to the 5.2 spec, and that min and max intervals are compatible > with each other, i.e. that min interval is less than max interval. > > A note here for maintainers: The tx power that is sent in the hci > parameter command is an int8_t, but as far as I can tell, there is no > clean way to use a signed 8-bit integer in dbus. The dbus byte type > seems incompatible with negative values in high-level languages (python) > without awkward usage manipulation on the client side. For this reason, > I chose to use an int16_t type for the tx power dbus field, which is > then downcasted to the int8_t in bluetoothd, which at least makes the > signed-ness of the type crystal clear to the dbus client that uses it. > > This change is manually verified by ensuring the intervals and tx power > parameters are correctly parsed from the LEAdvertisement1 object, and > that the parse fails if the parameters are incorrect or not compatible > with each other. > > Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org> > Reviewed-by: Alain Michaud <alainm@chromium.org> > --- > > Changes in v4: None > Changes in v3: None > Changes in v2: None > > src/advertising.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 89 insertions(+) > > diff --git a/src/advertising.c b/src/advertising.c > index 7c7599552..3690a2aac 100644 > --- a/src/advertising.c > +++ b/src/advertising.c > @@ -54,6 +54,11 @@ struct btd_adv_manager { > #define AD_TYPE_BROADCAST 0 > #define AD_TYPE_PERIPHERAL 1 > > +/* BLUETOOTH SPECIFICATION Version 5.2 | Vol 4, Part E, page 2585 > + * defines tx power value indicating no preference > + */ > +#define ADV_TX_POWER_NO_PREFERENCE 0x7F > + > struct btd_adv_client { > struct btd_adv_manager *manager; > char *owner; > @@ -74,6 +79,9 @@ struct btd_adv_client { > struct bt_ad *data; > struct bt_ad *scan; > uint8_t instance; > + uint32_t min_interval; > + uint32_t max_interval; > + int8_t tx_power; > }; > > struct dbus_obj_match { > @@ -937,6 +945,74 @@ static bool parse_secondary(DBusMessageIter *iter, > return false; > } > > +static bool parse_min_interval(DBusMessageIter *iter, > + struct btd_adv_client *client) > +{ > + if (!iter) > + return false; > + > + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT32) > + return false; > + > + dbus_message_iter_get_basic(iter, &client->min_interval); > + > + /* BLUETOOTH SPECIFICATION Version 5.2 | Vol 4, Part E, page 2584 > + * defines acceptable interval range > + */ > + if (client->min_interval < 0x20 || client->min_interval > 0xFFFFFF) { > + client->min_interval = 0; > + return false; > + } > + > + return true; > +} > + > +static bool parse_max_interval(DBusMessageIter *iter, > + struct btd_adv_client *client) > +{ > + if (!iter) > + return false; > + > + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT32) > + return false; > + > + dbus_message_iter_get_basic(iter, &client->max_interval); > + > + /* BLUETOOTH SPECIFICATION Version 5.2 | Vol 4, Part E, page 2584 > + * defines acceptable interval range > + */ > + if (client->max_interval < 0x20 || client->max_interval > 0xFFFFFF) { > + client->max_interval = 0; > + return false; > + } > + > + return true; > +} > + > +static bool parse_tx_power(DBusMessageIter *iter, > + struct btd_adv_client *client) > +{ > + int16_t val; > + > + if (!iter) > + return false; > + > + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_INT16) > + return false; > + > + dbus_message_iter_get_basic(iter, &val); > + > + /* BLUETOOTH SPECIFICATION Version 5.2 | Vol 4, Part E, page 2585 > + * defines acceptable tx power range > + */ > + if (val < -127 || val > 20) > + return false; > + > + client->tx_power = val; > + > + return true; > +} > + > static struct adv_parser { > const char *name; > bool (*func)(DBusMessageIter *iter, struct btd_adv_client *client); > @@ -955,6 +1031,9 @@ static struct adv_parser { > { "Discoverable", parse_discoverable }, > { "DiscoverableTimeout", parse_discoverable_timeout }, > { "SecondaryChannel", parse_secondary }, > + { "MinInterval", parse_min_interval }, > + { "MaxInterval", parse_max_interval }, > + { "TxPower", parse_tx_power }, > { }, > }; > > @@ -1083,6 +1162,13 @@ static DBusMessage *parse_advertisement(struct btd_adv_client *client) > goto fail; > } > > + if (client->min_interval > client->max_interval) { > + /* Min interval must not be bigger than max interval */ > + error("MinInterval must be less than MaxInterval (%lu > %lu)", > + client->min_interval, client->max_interval); > + goto fail; > + } > + > err = refresh_adv(client, add_adv_callback, &client->add_adv_id); > if (!err) > return NULL; > @@ -1158,6 +1244,9 @@ static struct btd_adv_client *client_create(struct btd_adv_manager *manager, > > client->manager = manager; > client->appearance = UINT16_MAX; > + client->tx_power = ADV_TX_POWER_NO_PREFERENCE; > + client->min_interval = 0; > + client->max_interval = 0; Lets only parse these if experimental is enabled. > return client; > > -- > 2.28.0.709.gb0816b6eb0-goog >
diff --git a/src/advertising.c b/src/advertising.c index 7c7599552..3690a2aac 100644 --- a/src/advertising.c +++ b/src/advertising.c @@ -54,6 +54,11 @@ struct btd_adv_manager { #define AD_TYPE_BROADCAST 0 #define AD_TYPE_PERIPHERAL 1 +/* BLUETOOTH SPECIFICATION Version 5.2 | Vol 4, Part E, page 2585 + * defines tx power value indicating no preference + */ +#define ADV_TX_POWER_NO_PREFERENCE 0x7F + struct btd_adv_client { struct btd_adv_manager *manager; char *owner; @@ -74,6 +79,9 @@ struct btd_adv_client { struct bt_ad *data; struct bt_ad *scan; uint8_t instance; + uint32_t min_interval; + uint32_t max_interval; + int8_t tx_power; }; struct dbus_obj_match { @@ -937,6 +945,74 @@ static bool parse_secondary(DBusMessageIter *iter, return false; } +static bool parse_min_interval(DBusMessageIter *iter, + struct btd_adv_client *client) +{ + if (!iter) + return false; + + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT32) + return false; + + dbus_message_iter_get_basic(iter, &client->min_interval); + + /* BLUETOOTH SPECIFICATION Version 5.2 | Vol 4, Part E, page 2584 + * defines acceptable interval range + */ + if (client->min_interval < 0x20 || client->min_interval > 0xFFFFFF) { + client->min_interval = 0; + return false; + } + + return true; +} + +static bool parse_max_interval(DBusMessageIter *iter, + struct btd_adv_client *client) +{ + if (!iter) + return false; + + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT32) + return false; + + dbus_message_iter_get_basic(iter, &client->max_interval); + + /* BLUETOOTH SPECIFICATION Version 5.2 | Vol 4, Part E, page 2584 + * defines acceptable interval range + */ + if (client->max_interval < 0x20 || client->max_interval > 0xFFFFFF) { + client->max_interval = 0; + return false; + } + + return true; +} + +static bool parse_tx_power(DBusMessageIter *iter, + struct btd_adv_client *client) +{ + int16_t val; + + if (!iter) + return false; + + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_INT16) + return false; + + dbus_message_iter_get_basic(iter, &val); + + /* BLUETOOTH SPECIFICATION Version 5.2 | Vol 4, Part E, page 2585 + * defines acceptable tx power range + */ + if (val < -127 || val > 20) + return false; + + client->tx_power = val; + + return true; +} + static struct adv_parser { const char *name; bool (*func)(DBusMessageIter *iter, struct btd_adv_client *client); @@ -955,6 +1031,9 @@ static struct adv_parser { { "Discoverable", parse_discoverable }, { "DiscoverableTimeout", parse_discoverable_timeout }, { "SecondaryChannel", parse_secondary }, + { "MinInterval", parse_min_interval }, + { "MaxInterval", parse_max_interval }, + { "TxPower", parse_tx_power }, { }, }; @@ -1083,6 +1162,13 @@ static DBusMessage *parse_advertisement(struct btd_adv_client *client) goto fail; } + if (client->min_interval > client->max_interval) { + /* Min interval must not be bigger than max interval */ + error("MinInterval must be less than MaxInterval (%lu > %lu)", + client->min_interval, client->max_interval); + goto fail; + } + err = refresh_adv(client, add_adv_callback, &client->add_adv_id); if (!err) return NULL; @@ -1158,6 +1244,9 @@ static struct btd_adv_client *client_create(struct btd_adv_manager *manager, client->manager = manager; client->appearance = UINT16_MAX; + client->tx_power = ADV_TX_POWER_NO_PREFERENCE; + client->min_interval = 0; + client->max_interval = 0; return client;