2014-12-16 12:02:36

by Jakub Tyszkowski

Subject: [PATCH 1/2] android/gatt: Fix embeded services not responsing to some requests

For application attributes this is triggered when application sends response but
for embeded ones we need to trigger this manually. As in prepare and execute
write requests handlers we don't know if its embeded or application attribute,
we should try sending processed request responses if there are any.
---
android/gatt.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index 2aa52b0..1e2627b 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4414,6 +4414,9 @@ static void send_dev_complete_response(struct gatt_device *device,
uint16_t len = 0;
uint8_t error = 0;

+ if (queue_isempty(device->pending_requests))
+ return;
+
if (queue_find(device->pending_requests, match_pending_dev_request,
NULL)) {
DBG("Still pending requests");
@@ -6513,6 +6516,8 @@ static uint8_t write_prep_request(const uint8_t *cmd, uint16_t cmd_len,
return ATT_ECODE_UNLIKELY;
}

+ send_dev_complete_response(dev, cmd[0]);
+
return 0;
}

@@ -6572,6 +6577,7 @@ static uint8_t write_execute_request(const uint8_t *cmd, uint16_t cmd_len,
}

queue_foreach(app_connections, send_server_write_execute_notify, &ev);
+ send_dev_complete_response(dev, cmd[0]);

return 0;
}
--
1.9.1

2014-12-18 10:47:43

by Szymon Janc

Subject: Re: [PATCH 2/2] android/gatt: Report error on invalid value length for CCC descriptor

Hi Jakub,

On Tuesday 16 of December 2014 13:02:37 Jakub Tyszkowski wrote:
> For application services, characteristics and descriptors it's user app's
> responsibility to verify value length but for embeded ones we need to do
> this in daemon. This is checked in TC_GAW_SR_BI_34_C and PTS tries to
> write to embeded CCC descritor if no other descriptors are added by the
> apps. It is expected by PTS for us to be more strict about value length
> controll.
> ---
> android/gatt.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 1e2627b..5f2f1c4 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -6958,6 +6958,13 @@ static void gatt_srvc_change_write_cb(struct gatt_db_attribute *attrib,
> return;
> }
>
> + /* No more than 2 octets are expected */
> + if (len > 2) {
> + gatt_db_attribute_write_result(attrib, id,
> + ATT_ECODE_INVAL_ATTR_VALUE_LEN);
> + return;
> + }
> +

I think we should check if len == 2 here and use get_le16()
and pass u16 to bt_store_gatt_ccc() (instead of u8 as will *value).

> /* Set services changed indication value */
> bt_store_gatt_ccc(bdaddr, *value);
>
>

--
Best regards,
Szymon Janc

2014-12-18 10:02:36

by Szymon Janc

Subject: Re: [PATCH 1/2] android/gatt: Fix embeded services not responsing to some requests

Hi Jakub,

On Tuesday 16 of December 2014 13:02:36 Jakub Tyszkowski wrote:
> For application attributes this is triggered when application sends response but
> for embeded ones we need to trigger this manually. As in prepare and execute
> write requests handlers we don't know if its embeded or application attribute,
> we should try sending processed request responses if there are any.
> ---
> android/gatt.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 2aa52b0..1e2627b 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -4414,6 +4414,9 @@ static void send_dev_complete_response(struct gatt_device *device,
> uint16_t len = 0;
> uint8_t error = 0;
>
> + if (queue_isempty(device->pending_requests))
> + return;
> +
> if (queue_find(device->pending_requests, match_pending_dev_request,
> NULL)) {
> DBG("Still pending requests");
> @@ -6513,6 +6516,8 @@ static uint8_t write_prep_request(const uint8_t *cmd, uint16_t cmd_len,
> return ATT_ECODE_UNLIKELY;
> }
>
> + send_dev_complete_response(dev, cmd[0]);
> +
> return 0;
> }
>
> @@ -6572,6 +6577,7 @@ static uint8_t write_execute_request(const uint8_t *cmd, uint16_t cmd_len,
> }
>
> queue_foreach(app_connections, send_server_write_execute_notify, &ev);
> + send_dev_complete_response(dev, cmd[0]);
>
> return 0;
> }
>

This patch is now applied, thanks.

--
Best regards,
Szymon Janc

2014-12-16 12:02:37

by Jakub Tyszkowski

Subject: [PATCH 2/2] android/gatt: Report error on invalid value length for CCC descriptor

For application services, characteristics and descriptors it's user app's
responsibility to verify value length but for embeded ones we need to do
this in daemon. This is checked in TC_GAW_SR_BI_34_C and PTS tries to
write to embeded CCC descritor if no other descriptors are added by the
apps. It is expected by PTS for us to be more strict about value length
controll.
---
android/gatt.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index 1e2627b..5f2f1c4 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -6958,6 +6958,13 @@ static void gatt_srvc_change_write_cb(struct gatt_db_attribute *attrib,
return;
}

+ /* No more than 2 octets are expected */
+ if (len > 2) {
+ gatt_db_attribute_write_result(attrib, id,
+ ATT_ECODE_INVAL_ATTR_VALUE_LEN);
+ return;
+ }
+
/* Set services changed indication value */
bt_store_gatt_ccc(bdaddr, *value);

--
1.9.1