2014-10-31 08:51:57

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCHv3 1/7] shared/io: Remove disconnect handler in io_destroy

Diconnect callback may be called after destroying IO in next
mainloop iteration (it leada to reading freed data). Removing
disconnect handler prevents that situation.
---
src/shared/io-glib.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/src/shared/io-glib.c b/src/shared/io-glib.c
index a2ada66..5ebde3d 100644
--- a/src/shared/io-glib.c
+++ b/src/shared/io-glib.c
@@ -115,6 +115,11 @@ void io_destroy(struct io *io)
io->write_watch = 0;
}

+ if (io->disconnect_watch > 0) {
+ g_source_remove(io->disconnect_watch);
+ io->disconnect_watch = 0;
+ }
+
g_io_channel_unref(io->channel);
io->channel = NULL;

--
1.9.3



2014-10-31 14:48:32

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCHv3 1/7] shared/io: Remove disconnect handler in io_destroy

Hi Marcin,

On Fri, Oct 31, 2014 at 10:51 AM, Marcin Kraglak
<[email protected]> wrote:
> Diconnect callback may be called after destroying IO in next
> mainloop iteration (it leada to reading freed data). Removing
> disconnect handler prevents that situation.
> ---
> src/shared/io-glib.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/src/shared/io-glib.c b/src/shared/io-glib.c
> index a2ada66..5ebde3d 100644
> --- a/src/shared/io-glib.c
> +++ b/src/shared/io-glib.c
> @@ -115,6 +115,11 @@ void io_destroy(struct io *io)
> io->write_watch = 0;
> }
>
> + if (io->disconnect_watch > 0) {
> + g_source_remove(io->disconnect_watch);
> + io->disconnect_watch = 0;
> + }
> +
> g_io_channel_unref(io->channel);
> io->channel = NULL;
>
> --
> 1.9.3

Applied, thanks.


--
Luiz Augusto von Dentz

2014-10-31 08:52:01

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCHv3 5/7] unit/test-gatt: Add TP/GAD/CL/BV-02-C-2 test

Verify that a Generic Attribute Profile client
can discover Primary Services selected by service
using 128-bit UUID.
---
unit/test-gatt.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 1167d4d..239f471 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -99,6 +99,12 @@ static bt_uuid_t uuid_16 = {
.value.u16 = 0x1800
};

+static bt_uuid_t uuid_128 = {
+ .type = BT_UUID128,
+ .value.u128.data = {0x00, 0x00, 0x18, 0x0d, 0x00, 0x00, 0x10, 0x00,
+ 0x80, 0x00, 0x00, 0x80, 0x5f, 0x9b, 0x34, 0xfb}
+};
+
static void test_debug(const char *str, void *user_data)
{
const char *prefix = user_data;
@@ -349,5 +355,19 @@ int main(int argc, char *argv[])
0x18),
raw_pdu(0x01, 0x06, 0x08, 0x00, 0x0a));

+ define_test("/TP/GAD/CL/BV-02-C-2", test_search_primary, ATT, &uuid_128,
+ raw_pdu(0x02, 0x00, 0x02),
+ raw_pdu(0x03, 0x00, 0x02),
+ raw_pdu(06, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28, 0xfb,
+ 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00,
+ 0x80, 0x00, 0x10, 0x00, 0x00, 0x0d,
+ 0x18, 0x00, 0x00),
+ raw_pdu(0x07, 0x10, 0x00, 0x17, 0x00),
+ raw_pdu(06, 0x18, 0x00, 0xff, 0xff, 0x00, 0x28, 0xfb,
+ 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00,
+ 0x80, 0x00, 0x10, 0x00, 0x00, 0x0d,
+ 0x18, 0x00, 0x00),
+ raw_pdu(0x01, 0x06, 0x08, 0x00, 0x0a));
+
return g_test_run();
}
--
1.9.3


2014-10-31 08:52:03

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCHv3 7/7] unit/test-gatt: Add TP/GAD/CL/BV-04-C test

Verify that a Generic Attribute Profile client
can discover characteristic declarations within
a specified service definition.
---
unit/test-gatt.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 3ee9460..27471ea 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -270,6 +270,17 @@ static void primary_cb(bool success, uint8_t att_ecode,
context_quit(context);
}

+static void characteristic_cb(bool success, uint8_t att_ecode,
+ struct bt_gatt_result *result,
+ void *user_data)
+{
+ struct context *context = user_data;
+
+ g_assert(success);
+
+ context_quit(context);
+}
+
static void included_cb(bool success, uint8_t att_ecode,
struct bt_gatt_result *result,
void *user_data)
@@ -332,6 +343,17 @@ static void test_search_included(gconstpointer data)
execute_context(context);
}

+static void test_search_chars(gconstpointer data)
+{
+ struct context *context = create_context(512, data);
+
+ g_assert(bt_gatt_discover_characteristics(context->att, 0x0010, 0x0020,
+ characteristic_cb, context,
+ NULL));
+
+ execute_context(context);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -413,5 +435,19 @@ int main(int argc, char *argv[])
raw_pdu(0x08, 0x06, 0x00, 0xff, 0xff, 0x02, 0x28),
raw_pdu(0x01, 0x08, 0x06, 0x00, 0x0a));

+ define_test("/TP/GAD/CL/BV-04-C", test_search_chars, ATT, NULL,
+ raw_pdu(0x02, 0x00, 0x02),
+ raw_pdu(0x03, 0x00, 0x02),
+ raw_pdu(0x08, 0x10, 0x00, 0x20, 0x00, 0x03, 0x28),
+ raw_pdu(0x09, 0x07, 0x11, 0x00, 02, 0x12, 0x00, 0x25,
+ 0x2a),
+ raw_pdu(0x08, 0x12, 0x00, 0x20, 0x00, 0x03, 0x28),
+ raw_pdu(0x09, 0x15, 0x13, 0x00, 0x02, 0x14, 0x00, 0x85,
+ 0x00, 0xef, 0xcd, 0xab, 0x89, 0x67,
+ 0x45, 0x23, 0x01, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00),
+ raw_pdu(0x08, 0x14, 0x00, 0x20, 0x00, 0x03, 0x28),
+ raw_pdu(0x01, 0x08, 0x12, 0x00, 0x0a));
+
return g_test_run();
}
--
1.9.3


2014-10-31 08:52:00

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCHv3 4/7] shared/gatt: Fix incorrect data read

Data set in Find By Type Value response contains 4 octets,
2 for start handle and two for end group handle. Reading data
with offset 6 from end of pdu can cause illegal access.
---
src/shared/gatt-helpers.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index d751d5a..6e19066 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -686,7 +686,13 @@ static void find_by_type_val_cb(uint8_t opcode, const void *pdu,
op->result_tail = cur_result;
}

- last_end = get_le16(pdu + length - 6);
+ /*
+ * Each data set contains:
+ * 2 octets with start handle
+ * 2 octets with end handle
+ * last_end is end handle of last data set
+ */
+ last_end = get_le16(pdu + length - 2);
if (last_end < op->end_handle) {
uint8_t pdu[6 + get_uuid_len(&op->uuid)];

--
1.9.3


2014-10-31 08:52:02

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCHv3 6/7] unit/test-gatt: Add TP/GAD/CL/BV-03-C test

Verify that a Generic Attribute Profile client
can find include service declarations within
a specified service definition on a server.
---
unit/test-gatt.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 239f471..3ee9460 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -270,6 +270,17 @@ static void primary_cb(bool success, uint8_t att_ecode,
context_quit(context);
}

+static void included_cb(bool success, uint8_t att_ecode,
+ struct bt_gatt_result *result,
+ void *user_data)
+{
+ struct context *context = user_data;
+
+ g_assert(success);
+
+ context_quit(context);
+}
+
static void destroy_context(struct context *context)
{
if (context->source > 0)
@@ -311,6 +322,16 @@ static void test_search_primary(gconstpointer data)
execute_context(context);
}

+static void test_search_included(gconstpointer data)
+{
+ struct context *context = create_context(512, data);
+
+ bt_gatt_discover_included_services(context->att, 0x0001, 0xffff,
+ included_cb, context, NULL);
+
+ execute_context(context);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -369,5 +390,28 @@ int main(int argc, char *argv[])
0x18, 0x00, 0x00),
raw_pdu(0x01, 0x06, 0x08, 0x00, 0x0a));

+ define_test("/TP/GAD/CL/BV-03-C", test_search_included, ATT, NULL,
+ raw_pdu(0x02, 0x00, 0x02),
+ raw_pdu(0x03, 0x00, 0x02),
+ raw_pdu(0x08, 0x01, 0x00, 0xff, 0xff, 0x02, 0x28),
+ raw_pdu(0x09, 0x08, 0x02, 0x00, 0x10, 0x00, 0x1f, 0x00,
+ 0x0f, 0x18),
+ raw_pdu(0x08, 0x03, 0x00, 0xff, 0xff, 0x02, 0x28),
+ raw_pdu(0x09, 0x06, 0x03, 0x00, 0x20, 0x00, 0x2f, 0x00,
+ 0x04, 0x00, 0x30, 0x00, 0x3f, 0x00),
+ raw_pdu(0x0a, 0x20, 0x00),
+ raw_pdu(0x0b, 0x00, 0x00, 0x3e, 0x39, 0x00, 0x00, 0x00,
+ 0x00, 0x01, 0x23, 0x45, 0x67, 0x89,
+ 0xab, 0xcd, 0xef),
+ raw_pdu(0x0a, 0x30, 0x00),
+ raw_pdu(0x0b, 0x00, 0x00, 0x3b, 0x39, 0x00, 0x00, 0x00,
+ 0x00, 0x01, 0x23, 0x45, 0x67, 0x89,
+ 0xab, 0xcd, 0xef),
+ raw_pdu(0x08, 0x05, 0x00, 0xff, 0xff, 0x02, 0x28),
+ raw_pdu(0x09, 0x08, 0x05, 0x00, 0x40, 0x00, 0x4f, 0x00,
+ 0x0a, 0x18),
+ raw_pdu(0x08, 0x06, 0x00, 0xff, 0xff, 0x02, 0x28),
+ raw_pdu(0x01, 0x08, 0x06, 0x00, 0x0a));
+
return g_test_run();
}
--
1.9.3


2014-10-31 08:51:59

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCHv3 3/7] unit/test-gatt: Add TP/GAD/CL/BV-02-C-1 test

Verify that a Generic Attribute Profile client
can discover Primary Services selected by service
using 16-bit UUID.
---
unit/test-gatt.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index f84b0fc..1167d4d 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -57,6 +57,7 @@ struct test_data {
char *test_name;
struct test_pdu *pdu_list;
enum context_type context_type;
+ bt_uuid_t *uuid;
};

struct context {
@@ -79,7 +80,7 @@ struct context {
.size = sizeof(data(args)), \
}

-#define define_test(name, function, type, args...) \
+#define define_test(name, function, type, bt_uuid, args...) \
do { \
const struct test_pdu pdus[] = { \
args, { } \
@@ -87,11 +88,17 @@ struct context {
static struct test_data data; \
data.test_name = g_strdup(name); \
data.context_type = type; \
+ data.uuid = bt_uuid; \
data.pdu_list = g_malloc(sizeof(pdus)); \
memcpy(data.pdu_list, pdus, sizeof(pdus)); \
g_test_add_data_func(name, &data, function); \
} while (0)

+static bt_uuid_t uuid_16 = {
+ .type = BT_UUID16,
+ .value.u16 = 0x1800
+};
+
static void test_debug(const char *str, void *user_data)
{
const char *prefix = user_data;
@@ -290,9 +297,10 @@ static void test_client(gconstpointer data)
static void test_search_primary(gconstpointer data)
{
struct context *context = create_context(512, data);
+ const struct test_data *test_data = data;

- bt_gatt_discover_all_primary_services(context->att, NULL, primary_cb,
- context, NULL);
+ bt_gatt_discover_all_primary_services(context->att, test_data->uuid,
+ primary_cb, context, NULL);

execute_context(context);
}
@@ -307,7 +315,7 @@ int main(int argc, char *argv[])
* The test group objective is to verify Generic Attribute Profile
* Server Configuration.
*/
- define_test("/TP/GAC/CL/BV-01-C", test_client, CLIENT,
+ define_test("/TP/GAC/CL/BV-01-C", test_client, CLIENT, NULL,
raw_pdu(0x02, 0x00, 0x02));

/*
@@ -316,7 +324,7 @@ int main(int argc, char *argv[])
* The test group objective is to verify Generic Attribute Profile
* Discovery of Services and Service Characteristics.
*/
- define_test("/TP/GAD/CL/BV-01-C", test_search_primary, ATT,
+ define_test("/TP/GAD/CL/BV-01-C", test_search_primary, ATT, NULL,
raw_pdu(0x02, 0x00, 0x02),
raw_pdu(0x03, 0x00, 0x02),
raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28),
@@ -331,5 +339,15 @@ int main(int argc, char *argv[])
raw_pdu(0x10, 0x97, 0x00, 0xff, 0xff, 0x00, 0x28),
raw_pdu(0x01, 0x10, 0x97, 0x00, 0x0a));

+ define_test("/TP/GAD/CL/BV-02-C-1", test_search_primary, ATT, &uuid_16,
+ raw_pdu(0x02, 0x00, 0x02),
+ raw_pdu(0x03, 0x00, 0x02),
+ raw_pdu(0x06, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28, 0x00,
+ 0x18),
+ raw_pdu(0x07, 0x01, 0x00, 0x07, 0x00),
+ raw_pdu(0x06, 0x08, 0x00, 0xff, 0xff, 0x00, 0x28, 0x00,
+ 0x18),
+ raw_pdu(0x01, 0x06, 0x08, 0x00, 0x0a));
+
return g_test_run();
}
--
1.9.3


2014-10-31 08:51:58

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCHv3 2/7] unit/test-gatt: Add TP/GAD/CL/BV-01-C test

Verify that a Generic Attribute Profile client discovers
Primary Services in a GATT server.
---
unit/test-gatt.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 77 insertions(+), 8 deletions(-)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index bbbf9a5..f84b0fc 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -35,8 +35,10 @@

#include <glib.h>

+#include "lib/uuid.h"
#include "src/shared/util.h"
#include "src/shared/att.h"
+#include "src/shared/gatt-helpers.h"
#include "src/shared/gatt-client.h"

struct test_pdu {
@@ -45,14 +47,22 @@ struct test_pdu {
size_t size;
};

+enum context_type {
+ ATT,
+ CLIENT,
+ SERVER
+};
+
struct test_data {
char *test_name;
struct test_pdu *pdu_list;
+ enum context_type context_type;
};

struct context {
GMainLoop *main_loop;
struct bt_gatt_client *client;
+ struct bt_att *att;
guint source;
guint process;
int fd;
@@ -69,13 +79,14 @@ struct context {
.size = sizeof(data(args)), \
}

-#define define_test(name, function, args...) \
+#define define_test(name, function, type, args...) \
do { \
const struct test_pdu pdus[] = { \
args, { } \
}; \
static struct test_data data; \
data.test_name = g_strdup(name); \
+ data.context_type = type; \
data.pdu_list = g_malloc(sizeof(pdus)); \
memcpy(data.pdu_list, pdus, sizeof(pdus)); \
g_test_add_data_func(name, &data, function); \
@@ -182,6 +193,7 @@ static void gatt_debug(const char *str, void *user_data)
static struct context *create_context(uint16_t mtu, gconstpointer data)
{
struct context *context = g_new0(struct context, 1);
+ const struct test_data *test_data = data;
GIOChannel *channel;
int err, sv[2];
struct bt_att *att;
@@ -195,12 +207,25 @@ static struct context *create_context(uint16_t mtu, gconstpointer data)
att = bt_att_new(sv[0]);
g_assert(att);

- context->client = bt_gatt_client_new(att, mtu);
- g_assert(context->client);
+ switch (test_data->context_type) {
+ case ATT:
+ context->att = att;

- if (g_test_verbose())
- bt_gatt_client_set_debug(context->client, gatt_debug, "gatt:",
- NULL);
+ bt_gatt_exchange_mtu(context->att, mtu, NULL, NULL, NULL);
+ break;
+ case CLIENT:
+ context->client = bt_gatt_client_new(att, mtu);
+ g_assert(context->client);
+
+ if (g_test_verbose())
+ bt_gatt_client_set_debug(context->client, gatt_debug,
+ "gatt:", NULL);
+
+ bt_att_unref(att);
+ break;
+ default:
+ break;
+ }

channel = g_io_channel_unix_new(sv[1]);

@@ -214,7 +239,6 @@ static struct context *create_context(uint16_t mtu, gconstpointer data)
g_assert(context->source > 0);

g_io_channel_unref(channel);
- bt_att_unref(att);

context->fd = sv[1];
context->data = data;
@@ -222,6 +246,17 @@ static struct context *create_context(uint16_t mtu, gconstpointer data)
return context;
}

+static void primary_cb(bool success, uint8_t att_ecode,
+ struct bt_gatt_result *result,
+ void *user_data)
+{
+ struct context *context = user_data;
+
+ g_assert(success);
+
+ context_quit(context);
+}
+
static void destroy_context(struct context *context)
{
if (context->source > 0)
@@ -229,6 +264,9 @@ static void destroy_context(struct context *context)

bt_gatt_client_unref(context->client);

+ if (context->att)
+ bt_att_unref(context->att);
+
g_main_loop_unref(context->main_loop);

test_free(context->data);
@@ -249,6 +287,16 @@ static void test_client(gconstpointer data)
execute_context(context);
}

+static void test_search_primary(gconstpointer data)
+{
+ struct context *context = create_context(512, data);
+
+ bt_gatt_discover_all_primary_services(context->att, NULL, primary_cb,
+ context, NULL);
+
+ execute_context(context);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -259,8 +307,29 @@ int main(int argc, char *argv[])
* The test group objective is to verify Generic Attribute Profile
* Server Configuration.
*/
- define_test("/TP/GAC/CL/BV-01-C", test_client,
+ define_test("/TP/GAC/CL/BV-01-C", test_client, CLIENT,
raw_pdu(0x02, 0x00, 0x02));

+ /*
+ * Discovery
+ *
+ * The test group objective is to verify Generic Attribute Profile
+ * Discovery of Services and Service Characteristics.
+ */
+ define_test("/TP/GAD/CL/BV-01-C", test_search_primary, ATT,
+ raw_pdu(0x02, 0x00, 0x02),
+ raw_pdu(0x03, 0x00, 0x02),
+ raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28),
+ raw_pdu(0x11, 0x06, 0x10, 0x00, 0x13, 0x00, 0x00, 0x18,
+ 0x20, 0x00, 0x29, 0x00, 0xb0, 0x68,
+ 0x30, 0x00, 0x32, 0x00, 0x19, 0x18),
+ raw_pdu(0x10, 0x33, 0x00, 0xff, 0xff, 0x00, 0x28),
+ raw_pdu(0x11, 0x14, 0x90, 0x00, 0x96, 0x00, 0xef, 0xcd,
+ 0xab, 0x89, 0x67, 0x45, 0x23, 0x01,
+ 0x00, 0x00, 0x00, 0x00, 0x85, 0x60,
+ 0x00, 0x00),
+ raw_pdu(0x10, 0x97, 0x00, 0xff, 0xff, 0x00, 0x28),
+ raw_pdu(0x01, 0x10, 0x97, 0x00, 0x0a));
+
return g_test_run();
}
--
1.9.3