2018-03-23 12:06:18

by Szymon Janc

[permalink] [raw]
Subject: [PATCH] gatt: Add support for storing Service Changed CCC value

This adds support for storing CCC value of Service Changed
characteristic. Once bluetoothd is restart stored values are read
and any device subscribed to indications will receive Service Changed
indication with 0x00010-0xffff value. This is to invalidate any
non-core services since there is no way to verify if applications
will register their services in same order (or at all).

This fix accessing invalid handles by stacks that rely only on Service
Changed indication for rediscovery ie. Apple iOS.
---
src/adapter.c | 3 ++
src/device.c | 49 +++++++++++++++++++++++++
src/device.h | 4 +++
src/gatt-database.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++---
src/gatt-database.h | 2 ++
5 files changed, 154 insertions(+), 4 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 6d7d61504..fba01df0c 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -7998,6 +7998,9 @@ load:
clear_blocked(adapter);
load_devices(adapter);

+ /* restore Service Changed CCC value for bonded devices */
+ btd_gatt_database_restore_svc_chng_ccc(adapter->database);
+
/* retrieve the active connections: address the scenario where
* the are active connections before the daemon've started */
if (adapter->current_settings & MGMT_SETTING_POWERED)
diff --git a/src/device.c b/src/device.c
index bf5441543..f624649ff 100644
--- a/src/device.c
+++ b/src/device.c
@@ -225,6 +225,9 @@ struct btd_device {
uint16_t att_mtu; /* The ATT MTU */
unsigned int att_disconn_id;

+ uint16_t le_svc_cng_ccc;
+ uint16_t br_svc_cng_ccc;
+
/*
* TODO: For now, device creates and owns the client-role gatt_db, but
* this needs to be persisted in a more central place so that proper
@@ -364,6 +367,17 @@ static void store_csrk(struct csrk_info *csrk, GKeyFile *key_file,
g_key_file_set_integer(key_file, group, "Counter", csrk->counter);
}

+static void update_svc_chng_ccc(GKeyFile *file, struct btd_device *dev)
+{
+ if (dev->bredr && dev->bredr_state.bonded)
+ g_key_file_set_integer(file, "ServiceChanged", "CCC_BR/EDR",
+ dev->br_svc_cng_ccc);
+
+ if (dev->le && dev->le_state.bonded)
+ g_key_file_set_integer(file, "ServiceChanged", "CCC_LE",
+ dev->le_svc_cng_ccc);
+}
+
static gboolean store_device_info_cb(gpointer user_data)
{
struct btd_device *device = user_data;
@@ -408,6 +422,7 @@ static gboolean store_device_info_cb(gpointer user_data)
}

update_technologies(key_file, device);
+ update_svc_chng_ccc(key_file, device);

g_key_file_set_boolean(key_file, "General", "Trusted",
device->trusted);
@@ -2991,6 +3006,16 @@ static void load_info(struct btd_device *device, const char *local,
device->remote_csrk = load_csrk(key_file, "RemoteSignatureKey");
}

+ if (device->bredr)
+ device->br_svc_cng_ccc = g_key_file_get_integer(key_file,
+ "ServiceChanged", "CCC_BR/EDR",
+ NULL);
+
+ if (device->le)
+ device->le_svc_cng_ccc = g_key_file_get_integer(key_file,
+ "ServiceChanged", "CCC_LE",
+ NULL);
+
g_strfreev(techno);

next:
@@ -5238,6 +5263,10 @@ const bdaddr_t *device_get_address(struct btd_device *device)
{
return &device->bdaddr;
}
+uint8_t device_get_le_address_type(struct btd_device *device)
+{
+ return device->bdaddr_type;
+}

const char *device_get_path(const struct btd_device *device)
{
@@ -5329,6 +5358,26 @@ void device_set_legacy(struct btd_device *device, bool legacy)
DEVICE_INTERFACE, "LegacyPairing");
}

+void device_set_svc_chng_ccc(struct btd_device *device, uint8_t addr_type,
+ uint16_t value)
+{
+ if (addr_type == BDADDR_BREDR)
+ device->br_svc_cng_ccc = value;
+ else
+ device->le_svc_cng_ccc = value;
+
+ if (device_is_bonded(device, addr_type))
+ store_device_info(device);
+}
+
+uint16_t device_get_svc_chng_ccc(struct btd_device *device, uint8_t addr_type)
+{
+ if (addr_type == BDADDR_BREDR)
+ return device->br_svc_cng_ccc;
+
+ return device->le_svc_cng_ccc;
+}
+
void device_set_rssi_with_delta(struct btd_device *device, int8_t rssi,
int8_t delta_threshold)
{
diff --git a/src/device.h b/src/device.h
index bc046f780..4afaa9cde 100644
--- a/src/device.h
+++ b/src/device.h
@@ -87,6 +87,7 @@ void device_probe_profile(gpointer a, gpointer b);
void device_remove_profile(gpointer a, gpointer b);
struct btd_adapter *device_get_adapter(struct btd_device *device);
const bdaddr_t *device_get_address(struct btd_device *device);
+uint8_t device_get_le_address_type(struct btd_device *device);
const char *device_get_path(const struct btd_device *device);
gboolean device_is_temporary(struct btd_device *device);
bool device_is_connectable(struct btd_device *device);
@@ -131,6 +132,9 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type);
void device_request_disconnect(struct btd_device *device, DBusMessage *msg);
bool device_is_disconnecting(struct btd_device *device);

+void device_set_svc_chng_ccc(struct btd_device *device, uint8_t badddr_type, uint16_t value);
+uint16_t device_get_svc_chng_ccc(struct btd_device *device, uint8_t badddr_type);
+
typedef void (*disconnect_watch) (struct btd_device *device, gboolean removal,
void *user_data);

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 19f03c544..f97f3caf7 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -265,7 +265,7 @@ static struct ccc_state *find_ccc_state(struct device_state *dev_state,
}

static struct device_state *device_state_create(struct btd_gatt_database *db,
- bdaddr_t *bdaddr,
+ const bdaddr_t *bdaddr,
uint8_t bdaddr_type)
{
struct device_state *dev_state;
@@ -950,6 +950,25 @@ service_add_ccc(struct gatt_db_attribute *service,
return ccc;
}

+static uint8_t svc_chngd_ccc_write_cb(struct bt_att *att, uint16_t value,
+ void *user_data)
+{
+ struct btd_gatt_database *database = user_data;
+ struct btd_device * device;
+ bdaddr_t bdaddr;
+ uint8_t bdaddr_type;
+
+ if (!get_dst_info(att, &bdaddr, &bdaddr_type))
+ return 0;
+
+ device = btd_adapter_find_device(database->adapter, &bdaddr,
+ bdaddr_type);
+ if (device)
+ device_set_svc_chng_ccc(device, bdaddr_type, value);
+
+ return 0;
+}
+
static void populate_gatt_service(struct btd_gatt_database *database)
{
bt_uuid_t uuid;
@@ -964,8 +983,9 @@ static void populate_gatt_service(struct btd_gatt_database *database)
BT_ATT_PERM_READ, BT_GATT_CHRC_PROP_INDICATE,
NULL, NULL, database);

- database->svc_chngd_ccc = service_add_ccc(service, database, NULL, NULL,
- NULL);
+ database->svc_chngd_ccc = service_add_ccc(service, database,
+ svc_chngd_ccc_write_cb,
+ database, NULL);

gatt_db_service_set_active(service, true);
}
@@ -996,6 +1016,7 @@ static void state_set_pending(struct device_state *state, struct notify *notify)
{
uint16_t start, end, old_start, old_end;

+ /* Cache this only for Service Changed */
if (notify->conf != service_changed_conf)
return;

@@ -3175,7 +3196,6 @@ struct btd_gatt_database *btd_gatt_database_new(struct btd_adapter *adapter)
if (!database->db_id)
goto fail;

-
return database;

fail:
@@ -3224,3 +3244,75 @@ void btd_gatt_database_att_connected(struct btd_gatt_database *database,
free(state->pending);
state->pending = NULL;
}
+
+static void restore_ccc(struct btd_gatt_database *database,
+ const bdaddr_t *addr, uint8_t addr_type, uint16_t value)
+{
+ struct device_state *dev_state;
+ struct ccc_state *ccc;
+
+ dev_state = device_state_create(database, addr, addr_type);
+ queue_push_tail(database->device_states, dev_state);
+
+ ccc = new0(struct ccc_state, 1);
+ ccc->handle = gatt_db_attribute_get_handle(database->svc_chngd_ccc);
+ memcpy(ccc->value, &value, sizeof(ccc->value));
+ queue_push_tail(dev_state->ccc_states, ccc);
+}
+
+static void restore_state(struct btd_device *device, void *data)
+{
+ struct btd_gatt_database *database = data;
+ uint16_t ccc_value;
+ uint8_t addr_type;
+
+ addr_type = device_get_le_address_type(device);
+
+ if (addr_type != BDADDR_BREDR) {
+ ccc_value = device_get_svc_chng_ccc(device, addr_type);
+ if (ccc_value) {
+ restore_ccc(database, device_get_address(device),
+ addr_type, ccc_value);
+ DBG("%s LE", device_get_path(device));
+ }
+ }
+
+ ccc_value = device_get_svc_chng_ccc(device, BDADDR_BREDR);
+ if (ccc_value) {
+ restore_ccc(database, device_get_address(device), BDADDR_BREDR,
+ ccc_value);
+ DBG("%s BR/EDR", device_get_path(device));
+ }
+}
+
+void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database *database)
+{
+ uint8_t value[4];
+ uint16_t handle, ccc_handle;
+
+ handle = gatt_db_attribute_get_handle(database->svc_chngd);
+ ccc_handle = gatt_db_attribute_get_handle(database->svc_chngd_ccc);
+
+ if (!handle || !ccc_handle) {
+ error("Failed to obtain handles for \"Service Changed\""
+ " characteristic");
+ return;
+ }
+
+ /* restore states for bonded device that registered for Service Changed
+ * indication
+ */
+ btd_adapter_for_each_device(database->adapter, restore_state, database);
+
+ /* This needs to be updated (probably to 0x0001) if we ever change
+ * core services
+ *
+ * TODO we could also store this info (along with CCC value) and be able
+ * to send 0x0001-0xffff only once per device.
+ */
+ put_le16(0x000a, value);
+ put_le16(0xffff, value + 2);
+
+ send_notification_to_devices(database, handle, value, sizeof(value),
+ ccc_handle, service_changed_conf, NULL);
+}
diff --git a/src/gatt-database.h b/src/gatt-database.h
index 0da33f604..a6c3139c4 100644
--- a/src/gatt-database.h
+++ b/src/gatt-database.h
@@ -25,3 +25,5 @@ void btd_gatt_database_destroy(struct btd_gatt_database *database);
struct gatt_db *btd_gatt_database_get_db(struct btd_gatt_database *database);
void btd_gatt_database_att_connected(struct btd_gatt_database *database,
struct bt_att *att);
+
+void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database *database);
--
2.14.3



2018-03-26 14:51:55

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] gatt: Add support for storing Service Changed CCC value

Hi Szymon,

On Mon, Mar 26, 2018 at 5:25 PM, Szymon Janc <[email protected]> wrote:
> Hi Luiz,
>
> On Monday, 26 March 2018 11:33:38 CEST Luiz Augusto von Dentz wrote:
>> Hi Szymon,
>>
>> On Fri, Mar 23, 2018 at 2:06 PM, Szymon Janc <[email protected]>
> wrote:
>> > This adds support for storing CCC value of Service Changed
>> > characteristic. Once bluetoothd is restart stored values are read
>> > and any device subscribed to indications will receive Service Changed
>> > indication with 0x00010-0xffff value. This is to invalidate any
>> > non-core services since there is no way to verify if applications
>> > will register their services in same order (or at all).
>> >
>> > This fix accessing invalid handles by stacks that rely only on Service
>> > Changed indication for rediscovery ie. Apple iOS.
>> > ---
>> >
>> > src/adapter.c | 3 ++
>> > src/device.c | 49 +++++++++++++++++++++++++
>> > src/device.h | 4 +++
>> > src/gatt-database.c | 100
>> > +++++++++++++++++++++++++++++++++++++++++++++++++--- src/gatt-database.h
>> > | 2 ++
>> > 5 files changed, 154 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/src/adapter.c b/src/adapter.c
>> > index 6d7d61504..fba01df0c 100644
>> > --- a/src/adapter.c
>> > +++ b/src/adapter.c
>> >
>> > @@ -7998,6 +7998,9 @@ load:
>> > clear_blocked(adapter);
>> > load_devices(adapter);
>> >
>> > + /* restore Service Changed CCC value for bonded devices */
>> > + btd_gatt_database_restore_svc_chng_ccc(adapter->database);
>> > +
>> >
>> > /* retrieve the active connections: address the scenario where
>> >
>> > * the are active connections before the daemon've started */
>> >
>> > if (adapter->current_settings & MGMT_SETTING_POWERED)
>> >
>> > diff --git a/src/device.c b/src/device.c
>> > index bf5441543..f624649ff 100644
>> > --- a/src/device.c
>> > +++ b/src/device.c
>> > @@ -225,6 +225,9 @@ struct btd_device {
>> >
>> > uint16_t att_mtu; /* The ATT MTU */
>> > unsigned int att_disconn_id;
>> >
>> > + uint16_t le_svc_cng_ccc;
>> > + uint16_t br_svc_cng_ccc;
>>
>> I wonder why can't we pass these directly to gatt-database.c? I mean
>> once you load it you could call restore_state directly instead of
>> iterating to each device
>> and calling device_get_svc_chng_ccc.
>
> It was initially like that, but the thing is that CCC value might be set
> before device is bonded but must be stored only after it bonded. So to handle
> this only in gatt-database it would have to track if/when device is bonded.

Fair enough, it just looked a little odd the dependency is
gatt-database.c -> device.c for storing but for loading is the
opposite.

>>
>> > /*
>> >
>> > * TODO: For now, device creates and owns the client-role gatt_db,
>> > but
>> > * this needs to be persisted in a more central place so that
>> > proper
>> >
>> > @@ -364,6 +367,17 @@ static void store_csrk(struct csrk_info *csrk,
>> > GKeyFile *key_file,>
>> > g_key_file_set_integer(key_file, group, "Counter", csrk->counter);
>> >
>> > }
>> >
>> > +static void update_svc_chng_ccc(GKeyFile *file, struct btd_device *dev)
>> > +{
>> > + if (dev->bredr && dev->bredr_state.bonded)
>> > + g_key_file_set_integer(file, "ServiceChanged",
>> > "CCC_BR/EDR", +
>> > dev->br_svc_cng_ccc); +
>> > + if (dev->le && dev->le_state.bonded)
>> > + g_key_file_set_integer(file, "ServiceChanged", "CCC_LE",
>> > +
>> > dev->le_svc_cng_ccc); +}
>> > +
>> >
>> > static gboolean store_device_info_cb(gpointer user_data)
>> > {
>> >
>> > struct btd_device *device = user_data;
>> >
>> > @@ -408,6 +422,7 @@ static gboolean store_device_info_cb(gpointer
>> > user_data)>
>> > }
>> >
>> > update_technologies(key_file, device);
>> >
>> > + update_svc_chng_ccc(key_file, device);
>> >
>> > g_key_file_set_boolean(key_file, "General", "Trusted",
>> >
>> > device->trusted);
>> >
>> > @@ -2991,6 +3006,16 @@ static void load_info(struct btd_device *device,
>> > const char *local,>
>> > device->remote_csrk = load_csrk(key_file,
>> > "RemoteSignatureKey");
>> >
>> > }
>> >
>> > + if (device->bredr)
>> > + device->br_svc_cng_ccc = g_key_file_get_integer(key_file,
>> > + "ServiceChanged",
>> > "CCC_BR/EDR", + NULL);
>> > +
>> > + if (device->le)
>> > + device->le_svc_cng_ccc = g_key_file_get_integer(key_file,
>> > + "ServiceChanged",
>> > "CCC_LE",
>> > + NULL);
>>
>> We should probably document these in the storage documentation. Also
>
> Right, I'll update documentation.
>
>> Id like to store the core service handle range along with it so we can
>> compare and in case it changes a full reset shall be sent, it is not
>> fail proof but it should make it easier to detect if plugin register a
>> service which shall probably be considered a fixed service as well.
>
> We can do that for core (ie GAP and GATT) which are handled in gatt-database.c
> anyway (there is even command on this in the patch). I'm not sure about other
> plugins though. With core it is simple, we just need to track end handle, for
> plugins... should ranges be stored and compared?

Yep, we would probably need to separate the handle range of
application, with that it should be able to detect if a plugin is
adding a service or it is an application, but perhaps we should leave
this for later as the
as there are no plugins adding service currently.

> Also, do we have plugins that act as GATT server? Weren't those deprecated and
> removed?

The old ones were deprecated because none of them require tight
integration with the daemon, but there could be services that would
still require being a plugin, e.g. BAS or something like that.

>>
>> > g_strfreev(techno);
>> >
>> > next:
>> > @@ -5238,6 +5263,10 @@ const bdaddr_t *device_get_address(struct
>> > btd_device *device)>
>> > {
>> >
>> > return &device->bdaddr;
>> >
>> > }
>> >
>> > +uint8_t device_get_le_address_type(struct btd_device *device)
>> > +{
>> > + return device->bdaddr_type;
>> > +}
>> >
>> > const char *device_get_path(const struct btd_device *device)
>> > {
>> >
>> > @@ -5329,6 +5358,26 @@ void device_set_legacy(struct btd_device *device,
>> > bool legacy)>
>> > DEVICE_INTERFACE,
>> > "LegacyPairing");
>> >
>> > }
>> >
>> > +void device_set_svc_chng_ccc(struct btd_device *device, uint8_t
>> > addr_type,
>> > + uint16_t
>> > value) +{
>> > + if (addr_type == BDADDR_BREDR)
>> > + device->br_svc_cng_ccc = value;
>> > + else
>> > + device->le_svc_cng_ccc = value;
>> > +
>> > + if (device_is_bonded(device, addr_type))
>> > + store_device_info(device);
>> > +}
>> > +
>> > +uint16_t device_get_svc_chng_ccc(struct btd_device *device, uint8_t
>> > addr_type) +{
>> > + if (addr_type == BDADDR_BREDR)
>> > + return device->br_svc_cng_ccc;
>> > +
>> > + return device->le_svc_cng_ccc;
>> > +}
>> > +
>> >
>> > void device_set_rssi_with_delta(struct btd_device *device, int8_t rssi,
>> >
>> > int8_t
>> > delta_threshold)
>> >
>> > {
>> >
>> > diff --git a/src/device.h b/src/device.h
>> > index bc046f780..4afaa9cde 100644
>> > --- a/src/device.h
>> > +++ b/src/device.h
>> > @@ -87,6 +87,7 @@ void device_probe_profile(gpointer a, gpointer b);
>> >
>> > void device_remove_profile(gpointer a, gpointer b);
>> > struct btd_adapter *device_get_adapter(struct btd_device *device);
>> > const bdaddr_t *device_get_address(struct btd_device *device);
>> >
>> > +uint8_t device_get_le_address_type(struct btd_device *device);
>> >
>> > const char *device_get_path(const struct btd_device *device);
>> > gboolean device_is_temporary(struct btd_device *device);
>> > bool device_is_connectable(struct btd_device *device);
>> >
>> > @@ -131,6 +132,9 @@ void device_remove_connection(struct btd_device
>> > *device, uint8_t bdaddr_type);>
>> > void device_request_disconnect(struct btd_device *device, DBusMessage
>> > *msg); bool device_is_disconnecting(struct btd_device *device);
>> >
>> > +void device_set_svc_chng_ccc(struct btd_device *device, uint8_t
>> > badddr_type, uint16_t value); +uint16_t device_get_svc_chng_ccc(struct
>> > btd_device *device, uint8_t badddr_type); +
>> >
>> > typedef void (*disconnect_watch) (struct btd_device *device, gboolean
>> > removal,>
>> > void *user_data);
>> >
>> > diff --git a/src/gatt-database.c b/src/gatt-database.c
>> > index 19f03c544..f97f3caf7 100644
>> > --- a/src/gatt-database.c
>> > +++ b/src/gatt-database.c
>> > @@ -265,7 +265,7 @@ static struct ccc_state *find_ccc_state(struct
>> > device_state *dev_state,>
>> > }
>> >
>> > static struct device_state *device_state_create(struct btd_gatt_database
>> > *db,>
>> > - bdaddr_t *bdaddr,
>> > + const bdaddr_t
>> > *bdaddr,>
>> > uint8_t
>> > bdaddr_type)
>> >
>> > {
>> >
>> > struct device_state *dev_state;
>> >
>> > @@ -950,6 +950,25 @@ service_add_ccc(struct gatt_db_attribute *service,
>> >
>> > return ccc;
>> >
>> > }
>> >
>> > +static uint8_t svc_chngd_ccc_write_cb(struct bt_att *att, uint16_t value,
>> > + void
>> > *user_data) +{
>> > + struct btd_gatt_database *database = user_data;
>> > + struct btd_device * device;
>> > + bdaddr_t bdaddr;
>> > + uint8_t bdaddr_type;
>> > +
>> > + if (!get_dst_info(att, &bdaddr, &bdaddr_type))
>> > + return 0;
>> > +
>> > + device = btd_adapter_find_device(database->adapter, &bdaddr,
>> > +
>> > bdaddr_type); + if (device)
>> > + device_set_svc_chng_ccc(device, bdaddr_type, value);
>> > +
>> > + return 0;
>> > +}
>> > +
>> >
>> > static void populate_gatt_service(struct btd_gatt_database *database)
>> > {
>> >
>> > bt_uuid_t uuid;
>> >
>> > @@ -964,8 +983,9 @@ static void populate_gatt_service(struct
>> > btd_gatt_database *database)>
>> > BT_ATT_PERM_READ,
>> > BT_GATT_CHRC_PROP_INDICATE,
>> > NULL, NULL, database);
>> >
>> > - database->svc_chngd_ccc = service_add_ccc(service, database, NULL,
>> > NULL, -
>> > NULL); + database->svc_chngd_ccc = service_add_ccc(service,
>> > database, +
>> > svc_chngd_ccc_write_cb, +
>> > database, NULL);>
>> > gatt_db_service_set_active(service, true);
>> >
>> > }
>> >
>> > @@ -996,6 +1016,7 @@ static void state_set_pending(struct device_state
>> > *state, struct notify *notify)>
>> > {
>> >
>> > uint16_t start, end, old_start, old_end;
>> >
>> > + /* Cache this only for Service Changed */
>> >
>> > if (notify->conf != service_changed_conf)
>> >
>> > return;
>> >
>> > @@ -3175,7 +3196,6 @@ struct btd_gatt_database
>> > *btd_gatt_database_new(struct btd_adapter *adapter)>
>> > if (!database->db_id)
>> >
>> > goto fail;
>> >
>> > -
>> >
>> > return database;
>> >
>> > fail:
>> > @@ -3224,3 +3244,75 @@ void btd_gatt_database_att_connected(struct
>> > btd_gatt_database *database,>
>> > free(state->pending);
>> > state->pending = NULL;
>> >
>> > }
>> >
>> > +
>> > +static void restore_ccc(struct btd_gatt_database *database,
>> > + const bdaddr_t *addr, uint8_t addr_type, uint16_t
>> > value) +{
>> > + struct device_state *dev_state;
>> > + struct ccc_state *ccc;
>> > +
>> > + dev_state = device_state_create(database, addr, addr_type);
>> > + queue_push_tail(database->device_states, dev_state);
>> > +
>> > + ccc = new0(struct ccc_state, 1);
>> > + ccc->handle =
>> > gatt_db_attribute_get_handle(database->svc_chngd_ccc); +
>> > memcpy(ccc->value, &value, sizeof(ccc->value));
>> > + queue_push_tail(dev_state->ccc_states, ccc);
>> > +}
>> > +
>> > +static void restore_state(struct btd_device *device, void *data)
>> > +{
>> > + struct btd_gatt_database *database = data;
>> > + uint16_t ccc_value;
>> > + uint8_t addr_type;
>> > +
>> > + addr_type = device_get_le_address_type(device);
>> > +
>> > + if (addr_type != BDADDR_BREDR) {
>> > + ccc_value = device_get_svc_chng_ccc(device, addr_type);
>> > + if (ccc_value) {
>> > + restore_ccc(database, device_get_address(device),
>> > + addr_type,
>> > ccc_value); + DBG("%s LE",
>> > device_get_path(device));
>> > + }
>> > + }
>> > +
>> > + ccc_value = device_get_svc_chng_ccc(device, BDADDR_BREDR);
>> > + if (ccc_value) {
>> > + restore_ccc(database, device_get_address(device),
>> > BDADDR_BREDR, +
>> > ccc_value); + DBG("%s BR/EDR",
>> > device_get_path(device));
>> > + }
>> > +}
>> > +
>> > +void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database
>> > *database) +{
>> > + uint8_t value[4];
>> > + uint16_t handle, ccc_handle;
>> > +
>> > + handle = gatt_db_attribute_get_handle(database->svc_chngd);
>> > + ccc_handle =
>> > gatt_db_attribute_get_handle(database->svc_chngd_ccc);
>> > +
>> > + if (!handle || !ccc_handle) {
>> > + error("Failed to obtain handles for \"Service Changed\""
>> > + "
>> > characteristic");
>> > + return;
>> > + }
>> > +
>> > + /* restore states for bonded device that registered for Service
>> > Changed + * indication
>> > + */
>> > + btd_adapter_for_each_device(database->adapter, restore_state,
>> > database); +
>> > + /* This needs to be updated (probably to 0x0001) if we ever change
>> > + * core services
>> > + *
>> > + * TODO we could also store this info (along with CCC value) and
>> > be able + * to send 0x0001-0xffff only once per device.
>> > + */
>> > + put_le16(0x000a, value);
>> > + put_le16(0xffff, value + 2);
>> > +
>> > + send_notification_to_devices(database, handle, value,
>> > sizeof(value), + ccc_handle,
>> > service_changed_conf, NULL); +}
>> > diff --git a/src/gatt-database.h b/src/gatt-database.h
>> > index 0da33f604..a6c3139c4 100644
>> > --- a/src/gatt-database.h
>> > +++ b/src/gatt-database.h
>> > @@ -25,3 +25,5 @@ void btd_gatt_database_destroy(struct btd_gatt_database
>> > *database);>
>> > struct gatt_db *btd_gatt_database_get_db(struct btd_gatt_database
>> > *database); void btd_gatt_database_att_connected(struct
>> > btd_gatt_database *database,>
>> > struct bt_att *att);
>> >
>> > +
>> > +void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database
>> > *database); --
>> > 2.14.3
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
>> > in the body of a message to [email protected]
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> --
> pozdrawiam
> Szymon Janc
>
>



--
Luiz Augusto von Dentz

2018-03-26 14:25:27

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH] gatt: Add support for storing Service Changed CCC value

Hi Luiz,

On Monday, 26 March 2018 11:33:38 CEST Luiz Augusto von Dentz wrote:
> Hi Szymon,
>
> On Fri, Mar 23, 2018 at 2:06 PM, Szymon Janc <[email protected]>
wrote:
> > This adds support for storing CCC value of Service Changed
> > characteristic. Once bluetoothd is restart stored values are read
> > and any device subscribed to indications will receive Service Changed
> > indication with 0x00010-0xffff value. This is to invalidate any
> > non-core services since there is no way to verify if applications
> > will register their services in same order (or at all).
> >
> > This fix accessing invalid handles by stacks that rely only on Service
> > Changed indication for rediscovery ie. Apple iOS.
> > ---
> >
> > src/adapter.c | 3 ++
> > src/device.c | 49 +++++++++++++++++++++++++
> > src/device.h | 4 +++
> > src/gatt-database.c | 100
> > +++++++++++++++++++++++++++++++++++++++++++++++++--- src/gatt-database.h
> > | 2 ++
> > 5 files changed, 154 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/adapter.c b/src/adapter.c
> > index 6d7d61504..fba01df0c 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> >
> > @@ -7998,6 +7998,9 @@ load:
> > clear_blocked(adapter);
> > load_devices(adapter);
> >
> > + /* restore Service Changed CCC value for bonded devices */
> > + btd_gatt_database_restore_svc_chng_ccc(adapter->database);
> > +
> >
> > /* retrieve the active connections: address the scenario where
> >
> > * the are active connections before the daemon've started */
> >
> > if (adapter->current_settings & MGMT_SETTING_POWERED)
> >
> > diff --git a/src/device.c b/src/device.c
> > index bf5441543..f624649ff 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -225,6 +225,9 @@ struct btd_device {
> >
> > uint16_t att_mtu; /* The ATT MTU */
> > unsigned int att_disconn_id;
> >
> > + uint16_t le_svc_cng_ccc;
> > + uint16_t br_svc_cng_ccc;
>
> I wonder why can't we pass these directly to gatt-database.c? I mean
> once you load it you could call restore_state directly instead of
> iterating to each device
> and calling device_get_svc_chng_ccc.

It was initially like that, but the thing is that CCC value might be set
before device is bonded but must be stored only after it bonded. So to handle
this only in gatt-database it would have to track if/when device is bonded.

>
> > /*
> >
> > * TODO: For now, device creates and owns the client-role gatt_db,
> > but
> > * this needs to be persisted in a more central place so that
> > proper
> >
> > @@ -364,6 +367,17 @@ static void store_csrk(struct csrk_info *csrk,
> > GKeyFile *key_file,>
> > g_key_file_set_integer(key_file, group, "Counter", csrk->counter);
> >
> > }
> >
> > +static void update_svc_chng_ccc(GKeyFile *file, struct btd_device *dev)
> > +{
> > + if (dev->bredr && dev->bredr_state.bonded)
> > + g_key_file_set_integer(file, "ServiceChanged",
> > "CCC_BR/EDR", +
> > dev->br_svc_cng_ccc); +
> > + if (dev->le && dev->le_state.bonded)
> > + g_key_file_set_integer(file, "ServiceChanged", "CCC_LE",
> > +
> > dev->le_svc_cng_ccc); +}
> > +
> >
> > static gboolean store_device_info_cb(gpointer user_data)
> > {
> >
> > struct btd_device *device = user_data;
> >
> > @@ -408,6 +422,7 @@ static gboolean store_device_info_cb(gpointer
> > user_data)>
> > }
> >
> > update_technologies(key_file, device);
> >
> > + update_svc_chng_ccc(key_file, device);
> >
> > g_key_file_set_boolean(key_file, "General", "Trusted",
> >
> > device->trusted);
> >
> > @@ -2991,6 +3006,16 @@ static void load_info(struct btd_device *device,
> > const char *local,>
> > device->remote_csrk = load_csrk(key_file,
> > "RemoteSignatureKey");
> >
> > }
> >
> > + if (device->bredr)
> > + device->br_svc_cng_ccc = g_key_file_get_integer(key_file,
> > + "ServiceChanged",
> > "CCC_BR/EDR", + NULL);
> > +
> > + if (device->le)
> > + device->le_svc_cng_ccc = g_key_file_get_integer(key_file,
> > + "ServiceChanged",
> > "CCC_LE",
> > + NULL);
>
> We should probably document these in the storage documentation. Also

Right, I'll update documentation.

> Id like to store the core service handle range along with it so we can
> compare and in case it changes a full reset shall be sent, it is not
> fail proof but it should make it easier to detect if plugin register a
> service which shall probably be considered a fixed service as well.

We can do that for core (ie GAP and GATT) which are handled in gatt-database.c
anyway (there is even command on this in the patch). I'm not sure about other
plugins though. With core it is simple, we just need to track end handle, for
plugins... should ranges be stored and compared?

Also, do we have plugins that act as GATT server? Weren't those deprecated and
removed?

>
> > g_strfreev(techno);
> >
> > next:
> > @@ -5238,6 +5263,10 @@ const bdaddr_t *device_get_address(struct
> > btd_device *device)>
> > {
> >
> > return &device->bdaddr;
> >
> > }
> >
> > +uint8_t device_get_le_address_type(struct btd_device *device)
> > +{
> > + return device->bdaddr_type;
> > +}
> >
> > const char *device_get_path(const struct btd_device *device)
> > {
> >
> > @@ -5329,6 +5358,26 @@ void device_set_legacy(struct btd_device *device,
> > bool legacy)>
> > DEVICE_INTERFACE,
> > "LegacyPairing");
> >
> > }
> >
> > +void device_set_svc_chng_ccc(struct btd_device *device, uint8_t
> > addr_type,
> > + uint16_t
> > value) +{
> > + if (addr_type == BDADDR_BREDR)
> > + device->br_svc_cng_ccc = value;
> > + else
> > + device->le_svc_cng_ccc = value;
> > +
> > + if (device_is_bonded(device, addr_type))
> > + store_device_info(device);
> > +}
> > +
> > +uint16_t device_get_svc_chng_ccc(struct btd_device *device, uint8_t
> > addr_type) +{
> > + if (addr_type == BDADDR_BREDR)
> > + return device->br_svc_cng_ccc;
> > +
> > + return device->le_svc_cng_ccc;
> > +}
> > +
> >
> > void device_set_rssi_with_delta(struct btd_device *device, int8_t rssi,
> >
> > int8_t
> > delta_threshold)
> >
> > {
> >
> > diff --git a/src/device.h b/src/device.h
> > index bc046f780..4afaa9cde 100644
> > --- a/src/device.h
> > +++ b/src/device.h
> > @@ -87,6 +87,7 @@ void device_probe_profile(gpointer a, gpointer b);
> >
> > void device_remove_profile(gpointer a, gpointer b);
> > struct btd_adapter *device_get_adapter(struct btd_device *device);
> > const bdaddr_t *device_get_address(struct btd_device *device);
> >
> > +uint8_t device_get_le_address_type(struct btd_device *device);
> >
> > const char *device_get_path(const struct btd_device *device);
> > gboolean device_is_temporary(struct btd_device *device);
> > bool device_is_connectable(struct btd_device *device);
> >
> > @@ -131,6 +132,9 @@ void device_remove_connection(struct btd_device
> > *device, uint8_t bdaddr_type);>
> > void device_request_disconnect(struct btd_device *device, DBusMessage
> > *msg); bool device_is_disconnecting(struct btd_device *device);
> >
> > +void device_set_svc_chng_ccc(struct btd_device *device, uint8_t
> > badddr_type, uint16_t value); +uint16_t device_get_svc_chng_ccc(struct
> > btd_device *device, uint8_t badddr_type); +
> >
> > typedef void (*disconnect_watch) (struct btd_device *device, gboolean
> > removal,>
> > void *user_data);
> >
> > diff --git a/src/gatt-database.c b/src/gatt-database.c
> > index 19f03c544..f97f3caf7 100644
> > --- a/src/gatt-database.c
> > +++ b/src/gatt-database.c
> > @@ -265,7 +265,7 @@ static struct ccc_state *find_ccc_state(struct
> > device_state *dev_state,>
> > }
> >
> > static struct device_state *device_state_create(struct btd_gatt_database
> > *db,>
> > - bdaddr_t *bdaddr,
> > + const bdaddr_t
> > *bdaddr,>
> > uint8_t
> > bdaddr_type)
> >
> > {
> >
> > struct device_state *dev_state;
> >
> > @@ -950,6 +950,25 @@ service_add_ccc(struct gatt_db_attribute *service,
> >
> > return ccc;
> >
> > }
> >
> > +static uint8_t svc_chngd_ccc_write_cb(struct bt_att *att, uint16_t value,
> > + void
> > *user_data) +{
> > + struct btd_gatt_database *database = user_data;
> > + struct btd_device * device;
> > + bdaddr_t bdaddr;
> > + uint8_t bdaddr_type;
> > +
> > + if (!get_dst_info(att, &bdaddr, &bdaddr_type))
> > + return 0;
> > +
> > + device = btd_adapter_find_device(database->adapter, &bdaddr,
> > +
> > bdaddr_type); + if (device)
> > + device_set_svc_chng_ccc(device, bdaddr_type, value);
> > +
> > + return 0;
> > +}
> > +
> >
> > static void populate_gatt_service(struct btd_gatt_database *database)
> > {
> >
> > bt_uuid_t uuid;
> >
> > @@ -964,8 +983,9 @@ static void populate_gatt_service(struct
> > btd_gatt_database *database)>
> > BT_ATT_PERM_READ,
> > BT_GATT_CHRC_PROP_INDICATE,
> > NULL, NULL, database);
> >
> > - database->svc_chngd_ccc = service_add_ccc(service, database, NULL,
> > NULL, -
> > NULL); + database->svc_chngd_ccc = service_add_ccc(service,
> > database, +
> > svc_chngd_ccc_write_cb, +
> > database, NULL);>
> > gatt_db_service_set_active(service, true);
> >
> > }
> >
> > @@ -996,6 +1016,7 @@ static void state_set_pending(struct device_state
> > *state, struct notify *notify)>
> > {
> >
> > uint16_t start, end, old_start, old_end;
> >
> > + /* Cache this only for Service Changed */
> >
> > if (notify->conf != service_changed_conf)
> >
> > return;
> >
> > @@ -3175,7 +3196,6 @@ struct btd_gatt_database
> > *btd_gatt_database_new(struct btd_adapter *adapter)>
> > if (!database->db_id)
> >
> > goto fail;
> >
> > -
> >
> > return database;
> >
> > fail:
> > @@ -3224,3 +3244,75 @@ void btd_gatt_database_att_connected(struct
> > btd_gatt_database *database,>
> > free(state->pending);
> > state->pending = NULL;
> >
> > }
> >
> > +
> > +static void restore_ccc(struct btd_gatt_database *database,
> > + const bdaddr_t *addr, uint8_t addr_type, uint16_t
> > value) +{
> > + struct device_state *dev_state;
> > + struct ccc_state *ccc;
> > +
> > + dev_state = device_state_create(database, addr, addr_type);
> > + queue_push_tail(database->device_states, dev_state);
> > +
> > + ccc = new0(struct ccc_state, 1);
> > + ccc->handle =
> > gatt_db_attribute_get_handle(database->svc_chngd_ccc); +
> > memcpy(ccc->value, &value, sizeof(ccc->value));
> > + queue_push_tail(dev_state->ccc_states, ccc);
> > +}
> > +
> > +static void restore_state(struct btd_device *device, void *data)
> > +{
> > + struct btd_gatt_database *database = data;
> > + uint16_t ccc_value;
> > + uint8_t addr_type;
> > +
> > + addr_type = device_get_le_address_type(device);
> > +
> > + if (addr_type != BDADDR_BREDR) {
> > + ccc_value = device_get_svc_chng_ccc(device, addr_type);
> > + if (ccc_value) {
> > + restore_ccc(database, device_get_address(device),
> > + addr_type,
> > ccc_value); + DBG("%s LE",
> > device_get_path(device));
> > + }
> > + }
> > +
> > + ccc_value = device_get_svc_chng_ccc(device, BDADDR_BREDR);
> > + if (ccc_value) {
> > + restore_ccc(database, device_get_address(device),
> > BDADDR_BREDR, +
> > ccc_value); + DBG("%s BR/EDR",
> > device_get_path(device));
> > + }
> > +}
> > +
> > +void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database
> > *database) +{
> > + uint8_t value[4];
> > + uint16_t handle, ccc_handle;
> > +
> > + handle = gatt_db_attribute_get_handle(database->svc_chngd);
> > + ccc_handle =
> > gatt_db_attribute_get_handle(database->svc_chngd_ccc);
> > +
> > + if (!handle || !ccc_handle) {
> > + error("Failed to obtain handles for \"Service Changed\""
> > + "
> > characteristic");
> > + return;
> > + }
> > +
> > + /* restore states for bonded device that registered for Service
> > Changed + * indication
> > + */
> > + btd_adapter_for_each_device(database->adapter, restore_state,
> > database); +
> > + /* This needs to be updated (probably to 0x0001) if we ever change
> > + * core services
> > + *
> > + * TODO we could also store this info (along with CCC value) and
> > be able + * to send 0x0001-0xffff only once per device.
> > + */
> > + put_le16(0x000a, value);
> > + put_le16(0xffff, value + 2);
> > +
> > + send_notification_to_devices(database, handle, value,
> > sizeof(value), + ccc_handle,
> > service_changed_conf, NULL); +}
> > diff --git a/src/gatt-database.h b/src/gatt-database.h
> > index 0da33f604..a6c3139c4 100644
> > --- a/src/gatt-database.h
> > +++ b/src/gatt-database.h
> > @@ -25,3 +25,5 @@ void btd_gatt_database_destroy(struct btd_gatt_database
> > *database);>
> > struct gatt_db *btd_gatt_database_get_db(struct btd_gatt_database
> > *database); void btd_gatt_database_att_connected(struct
> > btd_gatt_database *database,>
> > struct bt_att *att);
> >
> > +
> > +void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database
> > *database); --
> > 2.14.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> > in the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html


--
pozdrawiam
Szymon Janc



2018-03-26 09:33:38

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] gatt: Add support for storing Service Changed CCC value

Hi Szymon,

On Fri, Mar 23, 2018 at 2:06 PM, Szymon Janc <[email protected]> wrote:
> This adds support for storing CCC value of Service Changed
> characteristic. Once bluetoothd is restart stored values are read
> and any device subscribed to indications will receive Service Changed
> indication with 0x00010-0xffff value. This is to invalidate any
> non-core services since there is no way to verify if applications
> will register their services in same order (or at all).
>
> This fix accessing invalid handles by stacks that rely only on Service
> Changed indication for rediscovery ie. Apple iOS.
> ---
> src/adapter.c | 3 ++
> src/device.c | 49 +++++++++++++++++++++++++
> src/device.h | 4 +++
> src/gatt-database.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++---
> src/gatt-database.h | 2 ++
> 5 files changed, 154 insertions(+), 4 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 6d7d61504..fba01df0c 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -7998,6 +7998,9 @@ load:
> clear_blocked(adapter);
> load_devices(adapter);
>
> + /* restore Service Changed CCC value for bonded devices */
> + btd_gatt_database_restore_svc_chng_ccc(adapter->database);
> +
> /* retrieve the active connections: address the scenario where
> * the are active connections before the daemon've started */
> if (adapter->current_settings & MGMT_SETTING_POWERED)
> diff --git a/src/device.c b/src/device.c
> index bf5441543..f624649ff 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -225,6 +225,9 @@ struct btd_device {
> uint16_t att_mtu; /* The ATT MTU */
> unsigned int att_disconn_id;
>
> + uint16_t le_svc_cng_ccc;
> + uint16_t br_svc_cng_ccc;

I wonder why can't we pass these directly to gatt-database.c? I mean
once you load it you could call restore_state directly instead of
iterating to each device
and calling device_get_svc_chng_ccc.

> /*
> * TODO: For now, device creates and owns the client-role gatt_db, but
> * this needs to be persisted in a more central place so that proper
> @@ -364,6 +367,17 @@ static void store_csrk(struct csrk_info *csrk, GKeyFile *key_file,
> g_key_file_set_integer(key_file, group, "Counter", csrk->counter);
> }
>
> +static void update_svc_chng_ccc(GKeyFile *file, struct btd_device *dev)
> +{
> + if (dev->bredr && dev->bredr_state.bonded)
> + g_key_file_set_integer(file, "ServiceChanged", "CCC_BR/EDR",
> + dev->br_svc_cng_ccc);
> +
> + if (dev->le && dev->le_state.bonded)
> + g_key_file_set_integer(file, "ServiceChanged", "CCC_LE",
> + dev->le_svc_cng_ccc);
> +}
> +
> static gboolean store_device_info_cb(gpointer user_data)
> {
> struct btd_device *device = user_data;
> @@ -408,6 +422,7 @@ static gboolean store_device_info_cb(gpointer user_data)
> }
>
> update_technologies(key_file, device);
> + update_svc_chng_ccc(key_file, device);
>
> g_key_file_set_boolean(key_file, "General", "Trusted",
> device->trusted);
> @@ -2991,6 +3006,16 @@ static void load_info(struct btd_device *device, const char *local,
> device->remote_csrk = load_csrk(key_file, "RemoteSignatureKey");
> }
>
> + if (device->bredr)
> + device->br_svc_cng_ccc = g_key_file_get_integer(key_file,
> + "ServiceChanged", "CCC_BR/EDR",
> + NULL);
> +
> + if (device->le)
> + device->le_svc_cng_ccc = g_key_file_get_integer(key_file,
> + "ServiceChanged", "CCC_LE",
> + NULL);

We should probably document these in the storage documentation. Also
Id like to store the core service handle range along with it so we can
compare and in case it changes a full reset shall be sent, it is not
fail proof but it should make it easier to detect if plugin register a
service which shall probably be considered a fixed service as well.

> g_strfreev(techno);
>
> next:
> @@ -5238,6 +5263,10 @@ const bdaddr_t *device_get_address(struct btd_device *device)
> {
> return &device->bdaddr;
> }
> +uint8_t device_get_le_address_type(struct btd_device *device)
> +{
> + return device->bdaddr_type;
> +}
>
> const char *device_get_path(const struct btd_device *device)
> {
> @@ -5329,6 +5358,26 @@ void device_set_legacy(struct btd_device *device, bool legacy)
> DEVICE_INTERFACE, "LegacyPairing");
> }
>
> +void device_set_svc_chng_ccc(struct btd_device *device, uint8_t addr_type,
> + uint16_t value)
> +{
> + if (addr_type == BDADDR_BREDR)
> + device->br_svc_cng_ccc = value;
> + else
> + device->le_svc_cng_ccc = value;
> +
> + if (device_is_bonded(device, addr_type))
> + store_device_info(device);
> +}
> +
> +uint16_t device_get_svc_chng_ccc(struct btd_device *device, uint8_t addr_type)
> +{
> + if (addr_type == BDADDR_BREDR)
> + return device->br_svc_cng_ccc;
> +
> + return device->le_svc_cng_ccc;
> +}
> +
> void device_set_rssi_with_delta(struct btd_device *device, int8_t rssi,
> int8_t delta_threshold)
> {
> diff --git a/src/device.h b/src/device.h
> index bc046f780..4afaa9cde 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -87,6 +87,7 @@ void device_probe_profile(gpointer a, gpointer b);
> void device_remove_profile(gpointer a, gpointer b);
> struct btd_adapter *device_get_adapter(struct btd_device *device);
> const bdaddr_t *device_get_address(struct btd_device *device);
> +uint8_t device_get_le_address_type(struct btd_device *device);
> const char *device_get_path(const struct btd_device *device);
> gboolean device_is_temporary(struct btd_device *device);
> bool device_is_connectable(struct btd_device *device);
> @@ -131,6 +132,9 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type);
> void device_request_disconnect(struct btd_device *device, DBusMessage *msg);
> bool device_is_disconnecting(struct btd_device *device);
>
> +void device_set_svc_chng_ccc(struct btd_device *device, uint8_t badddr_type, uint16_t value);
> +uint16_t device_get_svc_chng_ccc(struct btd_device *device, uint8_t badddr_type);
> +
> typedef void (*disconnect_watch) (struct btd_device *device, gboolean removal,
> void *user_data);
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index 19f03c544..f97f3caf7 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -265,7 +265,7 @@ static struct ccc_state *find_ccc_state(struct device_state *dev_state,
> }
>
> static struct device_state *device_state_create(struct btd_gatt_database *db,
> - bdaddr_t *bdaddr,
> + const bdaddr_t *bdaddr,
> uint8_t bdaddr_type)
> {
> struct device_state *dev_state;
> @@ -950,6 +950,25 @@ service_add_ccc(struct gatt_db_attribute *service,
> return ccc;
> }
>
> +static uint8_t svc_chngd_ccc_write_cb(struct bt_att *att, uint16_t value,
> + void *user_data)
> +{
> + struct btd_gatt_database *database = user_data;
> + struct btd_device * device;
> + bdaddr_t bdaddr;
> + uint8_t bdaddr_type;
> +
> + if (!get_dst_info(att, &bdaddr, &bdaddr_type))
> + return 0;
> +
> + device = btd_adapter_find_device(database->adapter, &bdaddr,
> + bdaddr_type);
> + if (device)
> + device_set_svc_chng_ccc(device, bdaddr_type, value);
> +
> + return 0;
> +}
> +
> static void populate_gatt_service(struct btd_gatt_database *database)
> {
> bt_uuid_t uuid;
> @@ -964,8 +983,9 @@ static void populate_gatt_service(struct btd_gatt_database *database)
> BT_ATT_PERM_READ, BT_GATT_CHRC_PROP_INDICATE,
> NULL, NULL, database);
>
> - database->svc_chngd_ccc = service_add_ccc(service, database, NULL, NULL,
> - NULL);
> + database->svc_chngd_ccc = service_add_ccc(service, database,
> + svc_chngd_ccc_write_cb,
> + database, NULL);
>
> gatt_db_service_set_active(service, true);
> }
> @@ -996,6 +1016,7 @@ static void state_set_pending(struct device_state *state, struct notify *notify)
> {
> uint16_t start, end, old_start, old_end;
>
> + /* Cache this only for Service Changed */
> if (notify->conf != service_changed_conf)
> return;
>
> @@ -3175,7 +3196,6 @@ struct btd_gatt_database *btd_gatt_database_new(struct btd_adapter *adapter)
> if (!database->db_id)
> goto fail;
>
> -
> return database;
>
> fail:
> @@ -3224,3 +3244,75 @@ void btd_gatt_database_att_connected(struct btd_gatt_database *database,
> free(state->pending);
> state->pending = NULL;
> }
> +
> +static void restore_ccc(struct btd_gatt_database *database,
> + const bdaddr_t *addr, uint8_t addr_type, uint16_t value)
> +{
> + struct device_state *dev_state;
> + struct ccc_state *ccc;
> +
> + dev_state = device_state_create(database, addr, addr_type);
> + queue_push_tail(database->device_states, dev_state);
> +
> + ccc = new0(struct ccc_state, 1);
> + ccc->handle = gatt_db_attribute_get_handle(database->svc_chngd_ccc);
> + memcpy(ccc->value, &value, sizeof(ccc->value));
> + queue_push_tail(dev_state->ccc_states, ccc);
> +}
> +
> +static void restore_state(struct btd_device *device, void *data)
> +{
> + struct btd_gatt_database *database = data;
> + uint16_t ccc_value;
> + uint8_t addr_type;
> +
> + addr_type = device_get_le_address_type(device);
> +
> + if (addr_type != BDADDR_BREDR) {
> + ccc_value = device_get_svc_chng_ccc(device, addr_type);
> + if (ccc_value) {
> + restore_ccc(database, device_get_address(device),
> + addr_type, ccc_value);
> + DBG("%s LE", device_get_path(device));
> + }
> + }
> +
> + ccc_value = device_get_svc_chng_ccc(device, BDADDR_BREDR);
> + if (ccc_value) {
> + restore_ccc(database, device_get_address(device), BDADDR_BREDR,
> + ccc_value);
> + DBG("%s BR/EDR", device_get_path(device));
> + }
> +}
> +
> +void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database *database)
> +{
> + uint8_t value[4];
> + uint16_t handle, ccc_handle;
> +
> + handle = gatt_db_attribute_get_handle(database->svc_chngd);
> + ccc_handle = gatt_db_attribute_get_handle(database->svc_chngd_ccc);
> +
> + if (!handle || !ccc_handle) {
> + error("Failed to obtain handles for \"Service Changed\""
> + " characteristic");
> + return;
> + }
> +
> + /* restore states for bonded device that registered for Service Changed
> + * indication
> + */
> + btd_adapter_for_each_device(database->adapter, restore_state, database);
> +
> + /* This needs to be updated (probably to 0x0001) if we ever change
> + * core services
> + *
> + * TODO we could also store this info (along with CCC value) and be able
> + * to send 0x0001-0xffff only once per device.
> + */
> + put_le16(0x000a, value);
> + put_le16(0xffff, value + 2);
> +
> + send_notification_to_devices(database, handle, value, sizeof(value),
> + ccc_handle, service_changed_conf, NULL);
> +}
> diff --git a/src/gatt-database.h b/src/gatt-database.h
> index 0da33f604..a6c3139c4 100644
> --- a/src/gatt-database.h
> +++ b/src/gatt-database.h
> @@ -25,3 +25,5 @@ void btd_gatt_database_destroy(struct btd_gatt_database *database);
> struct gatt_db *btd_gatt_database_get_db(struct btd_gatt_database *database);
> void btd_gatt_database_att_connected(struct btd_gatt_database *database,
> struct bt_att *att);
> +
> +void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database *database);
> --
> 2.14.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Luiz Augusto von Dentz