* [PATCH 2/4] android/gatt: Fix sign counter comparison
2014-11-06 9:30 [PATCH 1/4] android/gatt: Fix using wrong variable type Jakub Tyszkowski
@ 2014-11-06 9:30 ` Jakub Tyszkowski
2014-11-06 9:30 ` [PATCH 3/4] android/gatt: Fix pending request data leakage Jakub Tyszkowski
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Jakub Tyszkowski @ 2014-11-06 9:30 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jakub Tyszkowski
This fixes invalid sign counter comparison and fixes one issue with
TC_SEC_CSIGN_BI_03_C PTS test case.
---
android/gatt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index d601cda..828f788 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -5935,8 +5935,8 @@ static void write_signed_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
uint8_t t[ATT_SIGNATURE_LEN];
uint32_t r_sign_cnt = get_le32(s);
- if (r_sign_cnt <= sign_cnt) {
- error("gatt: Invalid sign counter (%d<=%d)",
+ if (r_sign_cnt < sign_cnt) {
+ error("gatt: Invalid sign counter (%d<%d)",
r_sign_cnt, sign_cnt);
return;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/4] android/gatt: Fix pending request data leakage
2014-11-06 9:30 [PATCH 1/4] android/gatt: Fix using wrong variable type Jakub Tyszkowski
2014-11-06 9:30 ` [PATCH 2/4] android/gatt: Fix sign counter comparison Jakub Tyszkowski
@ 2014-11-06 9:30 ` Jakub Tyszkowski
2014-11-06 9:30 ` [PATCH 4/4] android/gatt: Use proper identity address for auto connect Jakub Tyszkowski
2014-11-06 15:28 ` [PATCH 1/4] android/gatt: Fix using wrong variable type Szymon Janc
3 siblings, 0 replies; 7+ messages in thread
From: Jakub Tyszkowski @ 2014-11-06 9:30 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jakub Tyszkowski
Fix potential memory leaks and one reported by Valgrind:
==28453== 201 (144 direct, 57 indirect) bytes in 3 blocks are definitely
lost in loss record 156 of 166
==28453== at 0x4C2CC70: calloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28453== by 0x4362AD: att_handler (gatt.c:5655)
==28453== by 0x44496D: received_data.part.2 (gattrib.c:432)
==28453== by 0x4E7FCE4: g_main_context_dispatch (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
==28453== by 0x4E80047: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
==28453== by 0x4E80309: g_main_loop_run (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
==28453== by 0x4045B6: main (main.c:772)
---
android/gatt.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/android/gatt.c b/android/gatt.c
index 828f788..47dadc2 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4350,6 +4350,7 @@ static void send_dev_complete_response(struct gatt_device *device,
if (val->error) {
queue_destroy(temp, NULL);
error = val->error;
+ destroy_pending_request(val);
goto done;
}
@@ -4363,6 +4364,9 @@ static void send_dev_complete_response(struct gatt_device *device,
adl = att_data_list_alloc(queue_length(temp), sizeof(uint16_t) +
length);
+ if (val)
+ destroy_pending_request(val);
+
val = queue_pop_head(temp);
while (val) {
uint8_t *value = adl->data[iterator++];
@@ -5637,7 +5641,8 @@ static uint8_t read_by_type(const uint8_t *cmd, uint16_t cmd_len,
data->state = REQUEST_INIT;
data->handle = handle;
- queue_push_tail(device->pending_requests, data);
+ if (!queue_push_tail(device->pending_requests, data))
+ free(data);
}
queue_destroy(q, NULL);
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/4] android/gatt: Use proper identity address for auto connect
2014-11-06 9:30 [PATCH 1/4] android/gatt: Fix using wrong variable type Jakub Tyszkowski
2014-11-06 9:30 ` [PATCH 2/4] android/gatt: Fix sign counter comparison Jakub Tyszkowski
2014-11-06 9:30 ` [PATCH 3/4] android/gatt: Fix pending request data leakage Jakub Tyszkowski
@ 2014-11-06 9:30 ` Jakub Tyszkowski
2014-11-06 13:43 ` Luiz Augusto von Dentz
2014-11-06 15:28 ` [PATCH 1/4] android/gatt: Fix using wrong variable type Szymon Janc
3 siblings, 1 reply; 7+ messages in thread
From: Jakub Tyszkowski @ 2014-11-06 9:30 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jakub Tyszkowski
We should behave the same as whe nwe connect using active scan.
---
android/gatt.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index 47dadc2..8cc7536 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -583,8 +583,24 @@ static void device_set_state(struct gatt_device *dev, uint32_t state)
static bool auto_connect_le(struct gatt_device *dev)
{
/* For LE devices use auto connect feature if possible */
- if (bt_kernel_conn_control())
- return bt_auto_connect_add(&dev->bdaddr);
+ if (bt_kernel_conn_control()) {
+ const bdaddr_t *bdaddr;
+
+ /*
+ * If address type is random it might be that IRK was received
+ * and random is just for faking Android Framework. ID address
+ * should be used for connection if present.
+ */
+ if (dev->bdaddr_type == BDADDR_LE_RANDOM) {
+ bdaddr = bt_get_id_addr(&dev->bdaddr, NULL);
+ if (!bdaddr)
+ return -EINVAL;
+ } else {
+ bdaddr = &dev->bdaddr;
+ }
+
+ return bt_auto_connect_add(bdaddr);
+ }
/* Trigger discovery if not already started */
if (!scanning) {
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 4/4] android/gatt: Use proper identity address for auto connect
2014-11-06 9:30 ` [PATCH 4/4] android/gatt: Use proper identity address for auto connect Jakub Tyszkowski
@ 2014-11-06 13:43 ` Luiz Augusto von Dentz
2014-11-09 19:47 ` Szymon Janc
0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2014-11-06 13:43 UTC (permalink / raw)
To: Jakub Tyszkowski; +Cc: linux-bluetooth@vger.kernel.org
Hi Jakub,
On Thu, Nov 6, 2014 at 11:30 AM, Jakub Tyszkowski
<jakub.tyszkowski@tieto.com> wrote:
> We should behave the same as whe nwe connect using active scan.
> ---
> android/gatt.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 47dadc2..8cc7536 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -583,8 +583,24 @@ static void device_set_state(struct gatt_device *dev, uint32_t state)
> static bool auto_connect_le(struct gatt_device *dev)
> {
> /* For LE devices use auto connect feature if possible */
> - if (bt_kernel_conn_control())
> - return bt_auto_connect_add(&dev->bdaddr);
> + if (bt_kernel_conn_control()) {
> + const bdaddr_t *bdaddr;
> +
> + /*
> + * If address type is random it might be that IRK was received
> + * and random is just for faking Android Framework. ID address
> + * should be used for connection if present.
> + */
> + if (dev->bdaddr_type == BDADDR_LE_RANDOM) {
> + bdaddr = bt_get_id_addr(&dev->bdaddr, NULL);
> + if (!bdaddr)
> + return -EINVAL;
> + } else {
> + bdaddr = &dev->bdaddr;
> + }
> +
> + return bt_auto_connect_add(bdaddr);
> + }
>
> /* Trigger discovery if not already started */
> if (!scanning) {
> --
> 1.9.1
Perhaps this would be better done inside bt_auto_connect_add since
anyway bt_get_id_addr is in bluetooth.c, actually perhaps
auto_connect_le is not really necessary since bt_auto_connect_add
should be able to do the checks done here.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 4/4] android/gatt: Use proper identity address for auto connect
2014-11-06 13:43 ` Luiz Augusto von Dentz
@ 2014-11-09 19:47 ` Szymon Janc
0 siblings, 0 replies; 7+ messages in thread
From: Szymon Janc @ 2014-11-09 19:47 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Jakub Tyszkowski, linux-bluetooth@vger.kernel.org
Hi Jakub, Luiz,
On Thursday 06 of November 2014 15:43:14 Luiz Augusto von Dentz wrote:
> Hi Jakub,
>
> On Thu, Nov 6, 2014 at 11:30 AM, Jakub Tyszkowski
>
> <jakub.tyszkowski@tieto.com> wrote:
> > We should behave the same as whe nwe connect using active scan.
> > ---
> >
> > android/gatt.c | 20 ++++++++++++++++++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/android/gatt.c b/android/gatt.c
> > index 47dadc2..8cc7536 100644
> > --- a/android/gatt.c
> > +++ b/android/gatt.c
> > @@ -583,8 +583,24 @@ static void device_set_state(struct gatt_device *dev,
> > uint32_t state)>
> > static bool auto_connect_le(struct gatt_device *dev)
> > {
> >
> > /* For LE devices use auto connect feature if possible */
> >
> > - if (bt_kernel_conn_control())
> > - return bt_auto_connect_add(&dev->bdaddr);
> > + if (bt_kernel_conn_control()) {
> > + const bdaddr_t *bdaddr;
> > +
> > + /*
> > + * If address type is random it might be that IRK was
> > received + * and random is just for faking Android
> > Framework. ID address + * should be used for connection if
> > present.
> > + */
> > + if (dev->bdaddr_type == BDADDR_LE_RANDOM) {
> > + bdaddr = bt_get_id_addr(&dev->bdaddr, NULL);
> > + if (!bdaddr)
> > + return -EINVAL;
> > + } else {
> > + bdaddr = &dev->bdaddr;
> > + }
> > +
> > + return bt_auto_connect_add(bdaddr);
> > + }
> >
> > /* Trigger discovery if not already started */
> > if (!scanning) {
> >
> > --
> > 1.9.1
>
> Perhaps this would be better done inside bt_auto_connect_add since
> anyway bt_get_id_addr is in bluetooth.c, actually perhaps
> auto_connect_le is not really necessary since bt_auto_connect_add
> should be able to do the checks done here.
I've applied this patch so we use proper address when connecting.
Nevertheless, Jakub please send follow up patch that address Luiz comment.
Thanks.
--
BR
Szymon Janc
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] android/gatt: Fix using wrong variable type
2014-11-06 9:30 [PATCH 1/4] android/gatt: Fix using wrong variable type Jakub Tyszkowski
` (2 preceding siblings ...)
2014-11-06 9:30 ` [PATCH 4/4] android/gatt: Use proper identity address for auto connect Jakub Tyszkowski
@ 2014-11-06 15:28 ` Szymon Janc
3 siblings, 0 replies; 7+ messages in thread
From: Szymon Janc @ 2014-11-06 15:28 UTC (permalink / raw)
To: Jakub Tyszkowski; +Cc: linux-bluetooth
Hi Jakub,
On Thursday 06 of November 2014 10:30:35 Jakub Tyszkowski wrote:
> Not all bytes were set thus the following valgrind report:
> ==4748== Conditional jump or move depends on uninitialised value(s)
> ==4748== at 0x436493: att_handler (gatt.c:5922)
> ==4748== by 0x4448ED: received_data.part.2 (gattrib.c:432)
> ==4748== by 0x4E7FCE4: g_main_context_dispatch (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
> ==4748== by 0x4E80047: ??? (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
> ==4748== by 0x4E80309: g_main_loop_run (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
> ==4748== by 0x4045B6: main (main.c:772)
> ==4748== Uninitialised value was created by a stack allocation
> ==4748== at 0x432690: get_cid.isra.5 (gatt.c:2983)
> ==4748==
> ---
> android/gatt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 7cf612f..d601cda 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -2965,7 +2965,7 @@ static void read_char_cb(guint8 status, const guint8 *pdu, guint16 len,
> static int get_cid(struct gatt_device *dev)
> {
> GIOChannel *io;
> - int cid;
> + uint16_t cid;
>
> io = g_attrib_get_channel(dev->attrib);
>
Patch 1-3 are now applied, thanks.
--
Best regards,
Szymon Janc
^ permalink raw reply [flat|nested] 7+ messages in thread