2014-05-21 14:34:40

by Jakub Tyszkowski

Subject: [PATCH 1/3] android/gatt: Check for connection state on connection search

This search was used with the assumption that connection is in
"connected" state. This could result in attrib pointer being
dereferenced while it's still NULL (pending connection).
---
android/gatt.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index 89da60d..1b000c4 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -364,8 +364,14 @@ static bool match_connection_by_device_and_app(const void *data,

static struct app_connection *find_connection_by_id(int32_t conn_id)
{
- return queue_find(app_connections, match_connection_by_id,
+ struct app_connection *conn;
+
+ conn = queue_find(app_connections, match_connection_by_id,
INT_TO_PTR(conn_id));
+ if (conn && conn->device->state == DEVICE_CONNECTED)
+ return conn;
+
+ return NULL;
}

static bool match_connection_by_device(const void *data, const void *user_data)
--
1.9.3

2014-05-22 12:12:19

by Szymon Janc

Subject: Re: [PATCH 1/3] android/gatt: Check for connection state on connection search

Hi Jakub,

On Wednesday 21 of May 2014 16:34:40 Jakub Tyszkowski wrote:
> This search was used with the assumption that connection is in
> "connected" state. This could result in attrib pointer being
> dereferenced while it's still NULL (pending connection).
> ---
> android/gatt.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 89da60d..1b000c4 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -364,8 +364,14 @@ static bool match_connection_by_device_and_app(const void *data,
>
> static struct app_connection *find_connection_by_id(int32_t conn_id)
> {
> - return queue_find(app_connections, match_connection_by_id,
> + struct app_connection *conn;
> +
> + conn = queue_find(app_connections, match_connection_by_id,
> INT_TO_PTR(conn_id));
> + if (conn && conn->device->state == DEVICE_CONNECTED)
> + return conn;
> +
> + return NULL;
> }
>
> static bool match_connection_by_device(const void *data, const void *user_data)

Patch 1/3 is now applied, thanks.

--
Best regards,
Szymon Janc

2014-05-22 11:01:56

by Jakub Tyszkowski

Subject: [PATCHv2] gatt: Fix not freeing GError on failure

---
profiles/gatt/gas.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index d240450..b51b4a8 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -329,6 +329,11 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
DBG("MTU Exchange: Requesting %d", imtu);
}

+ if (gerr) {
+ error("Could not acquire att imtu and cid: %s", gerr->message);
+ g_error_free(gerr);
+ }
+
if (device_get_appearance(gas->device, &app) < 0) {
bt_uuid_t uuid;

--
1.9.3

2014-05-21 14:54:31

by Johan Hedberg

Subject: Re: [PATCH 3/3] gatt: Fix not freeing GError on failure

Hi Jakub,

On Wed, May 21, 2014, Jakub Tyszkowski wrote:
> + if (gerr) {
> + error("Could not acquire att imtu and cid: %s", gerr->message);
> + free(gerr);
> + }

Good catch, but GErrors are freed with g_error_free()

Johan

2014-05-21 14:34:42

by Jakub Tyszkowski

Subject: [PATCH 3/3] gatt: Fix not freeing GError on failure

---
profiles/gatt/gas.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index d240450..01defa0 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -329,6 +329,11 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
DBG("MTU Exchange: Requesting %d", imtu);
}

+ if (gerr) {
+ error("Could not acquire att imtu and cid: %s", gerr->message);
+ free(gerr);
+ }
+
if (device_get_appearance(gas->device, &app) < 0) {
bt_uuid_t uuid;

--
1.9.3

2014-05-21 14:34:41

by Jakub Tyszkowski

Subject: [PATCH 2/3] attrib: Fix minor whitespace issue

Replace spaces with tabs.
---
attrib/att.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index c612d80..927a044 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -83,8 +83,8 @@
#define ATT_PSM 31

/* Flags for Execute Write Request Operation */
-#define ATT_CANCEL_ALL_PREP_WRITES 0x00
-#define ATT_WRITE_ALL_PREP_WRITES 0x01
+#define ATT_CANCEL_ALL_PREP_WRITES 0x00
+#define ATT_WRITE_ALL_PREP_WRITES 0x01

/* Find Information Response Formats */
#define ATT_FIND_INFO_RESP_FMT_16BIT 0x01
--
1.9.3