[PATCH 1/4] unit/eir: Fix G-tag test data

[PATCH 1/4] unit/eir: Fix G-tag test data

[Mariusz Skamra]

This patch adds SCAN_RSP data to gigaset_gtag_data.
Since MGMT 1.6 advertising and scan response data are merged.
So eir parser shall be able to parse this data.
---
 unit/test-eir.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/unit/test-eir.c b/unit/test-eir.c
index 421d0db..53abfa4 100644
--- a/unit/test-eir.c
+++ b/unit/test-eir.c
@@ -621,7 +621,9 @@ static const unsigned char gigaset_gtag_data[] = {
 		0x02, 0x01, 0x06, 0x0d, 0xff, 0x80, 0x01, 0x02,
 		0x15, 0x12, 0x34, 0x80, 0x91, 0xd0, 0xf2, 0xbb,
 		0xc5, 0x03, 0x02, 0x0f, 0x18, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0e, 0x09,
+		0x47, 0x69, 0x67, 0x61, 0x73, 0x65, 0x74, 0x20,
+		0x47, 0x2d, 0x74, 0x61, 0x67
 };
 
 static const char *gigaset_gtag_uuid[] = {
@@ -635,6 +637,8 @@ static const struct test_data gigaset_gtag_test = {
 	.flags = 0x06,
 	.tx_power = 127,
 	.uuid = gigaset_gtag_uuid,
+	.name = "Gigaset G-tag",
+	.name_complete = true,
 };
 
 static const char *uri_beacon_uuid[] = {
-- 
1.9.1

[PATCH 2/4] src/eir: Fix eir parser

[Mariusz Skamra]

Some devices (like Gigaset's G-Tag) sends Advertising Report with significant part
set to 0. According specification (Core 4.2, Vol 3, Part C, Sec 11) it is proper
behaviour. So if ADV and SCAN_RSP data are merged, there is a possibility to lose
SCAN_RSP data if the parser found early terminator.

This patch fixes this issue by checking the address type. For LE, early termination
won't be not used.
---
 android/bluetooth.c |  2 +-
 src/adapter.c       |  4 ++--
 src/eir.c           | 19 +++++++++++++------
 src/eir.h           |  3 ++-
 unit/test-eir.c     |  6 ++++--
 5 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 4d0cd48..09a31f8 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1920,7 +1920,7 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 
 	memset(&eir, 0, sizeof(eir));
 
-	eir_parse(&eir, data, data_len);
+	eir_parse(&eir, data, data_len, bdaddr_type);
 
 	dev = get_device(bdaddr, bdaddr_type);
 
diff --git a/src/adapter.c b/src/adapter.c
index 8ee5b5b..762fb19 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -5384,7 +5384,7 @@ static void update_found_devices(struct btd_adapter *adapter,
 	char addr[18];
 
 	memset(&eir_data, 0, sizeof(eir_data));
-	eir_parse(&eir_data, data, data_len);
+	eir_parse(&eir_data, data, data_len, bdaddr_type);
 
 	if (bdaddr_type == BDADDR_BREDR)
 		discoverable = true;
@@ -7467,7 +7467,7 @@ static void connected_callback(uint16_t index, uint16_t length,
 
 	memset(&eir_data, 0, sizeof(eir_data));
 	if (eir_len > 0)
-		eir_parse(&eir_data, ev->eir, eir_len);
+		eir_parse(&eir_data, ev->eir, eir_len, ev->addr.type);
 
 	if (eir_data.class != 0)
 		device_set_class(device, eir_data.class);
diff --git a/src/eir.c b/src/eir.c
index c984fa5..423a37f 100644
--- a/src/eir.c
+++ b/src/eir.c
@@ -226,7 +226,8 @@ static void eir_parse_uuid128_data(struct eir_data *eir, const uint8_t *data,
 	eir_parse_sd(eir, &service, data + 16, len - 16);
 }
 
-void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
+void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len,
+							uint8_t bdaddr_type)
 {
 	uint16_t len = 0;
 
@@ -242,15 +243,21 @@ void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
 		const uint8_t *data;
 		uint8_t data_len;
 
-		/* Check for the end of EIR */
-		if (field_len == 0)
+		/* Do not continue EIR Data parsing if got incorrect length */
+		if (len > eir_len)
 			break;
 
 		len += field_len + 1;
 
-		/* Do not continue EIR Data parsing if got incorrect length */
-		if (len > eir_len)
+		if (field_len == 0) {
+			/* Ignore redundant advertising data over LE*/
+			if (bdaddr_type != BDADDR_BREDR) {
+				eir_data += field_len + 1;
+				continue;
+			}
+			/* field_len == 0 for BREDR means the end of EIR */
 			break;
+		}
 
 		data = &eir_data[2];
 		data_len = field_len - 1;
@@ -370,7 +377,7 @@ int eir_parse_oob(struct eir_data *eir, uint8_t *eir_data, uint16_t eir_len)
 
 	/* optional OOB EIR data */
 	if (eir_len > 0)
-		eir_parse(eir, eir_data, eir_len);
+		eir_parse(eir, eir_data, eir_len, 0);
 
 	return 0;
 }
diff --git a/src/eir.h b/src/eir.h
index 219ee79..944413d 100644
--- a/src/eir.h
+++ b/src/eir.h
@@ -95,7 +95,8 @@ struct eir_data {
 };
 
 void eir_data_free(struct eir_data *eir);
-void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len);
+void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len,
+							uint8_t bdaddr_type);
 int eir_parse_oob(struct eir_data *eir, uint8_t *eir_data, uint16_t eir_len);
 int eir_create_oob(const bdaddr_t *addr, const char *name, uint32_t cod,
 			const uint8_t *hash, const uint8_t *randomizer,
diff --git a/unit/test-eir.c b/unit/test-eir.c
index 53abfa4..09d9865 100644
--- a/unit/test-eir.c
+++ b/unit/test-eir.c
@@ -44,6 +44,7 @@ struct test_data {
 	bool name_complete;
 	int8_t tx_power;
 	const char **uuid;
+	uint8_t bdaddr_type;
 };
 
 static const unsigned char macbookair_data[] = {
@@ -536,7 +537,7 @@ static void test_basic(const void *data)
 	memset(buf, 0, sizeof(buf));
 	memset(&eir, 0, sizeof(eir));
 
-	eir_parse(&eir, buf, HCI_MAX_EIR_LENGTH);
+	eir_parse(&eir, buf, HCI_MAX_EIR_LENGTH, BDADDR_BREDR);
 	g_assert(eir.services == NULL);
 	g_assert(eir.name == NULL);
 
@@ -560,7 +561,7 @@ static void test_parsing(gconstpointer data)
 
 	memset(&eir, 0, sizeof(eir));
 
-	eir_parse(&eir, test->eir_data, test->eir_size);
+	eir_parse(&eir, test->eir_data, test->eir_size, test->bdaddr_type);
 
 	tester_debug("Flags: %d", eir.flags);
 	tester_debug("Name: %s", eir.name);
@@ -639,6 +640,7 @@ static const struct test_data gigaset_gtag_test = {
 	.uuid = gigaset_gtag_uuid,
 	.name = "Gigaset G-tag",
 	.name_complete = true,
+	.bdaddr_type = BDADDR_LE_PUBLIC,
 };
 
 static const char *uri_beacon_uuid[] = {
-- 
1.9.1

Re: [PATCH 2/4] src/eir: Fix eir parser

[Luiz Augusto von Dentz]

Hi Mariusz,

On Thu, Apr 30, 2015 at 2:26 PM, Mariusz Skamra
<mariusz.skamra@gmail.com> wrote:
> Some devices (like Gigaset's G-Tag) sends Advertising Report with significant part
> set to 0. According specification (Core 4.2, Vol 3, Part C, Sec 11) it is proper
> behaviour. So if ADV and SCAN_RSP data are merged, there is a possibility to lose
> SCAN_RSP data if the parser found early terminator.

This sounds like a bug in the code merging ADV and SCAN_RSP together,
it should have taken care of eliminating the early terminator
otherwise we are changing the format which breaks userspace.

> This patch fixes this issue by checking the address type. For LE, early termination
> won't be not used.
> ---
>  android/bluetooth.c |  2 +-
>  src/adapter.c       |  4 ++--
>  src/eir.c           | 19 +++++++++++++------
>  src/eir.h           |  3 ++-
>  unit/test-eir.c     |  6 ++++--
>  5 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index 4d0cd48..09a31f8 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -1920,7 +1920,7 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
>
>         memset(&eir, 0, sizeof(eir));
>
> -       eir_parse(&eir, data, data_len);
> +       eir_parse(&eir, data, data_len, bdaddr_type);
>
>         dev = get_device(bdaddr, bdaddr_type);
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 8ee5b5b..762fb19 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -5384,7 +5384,7 @@ static void update_found_devices(struct btd_adapter *adapter,
>         char addr[18];
>
>         memset(&eir_data, 0, sizeof(eir_data));
> -       eir_parse(&eir_data, data, data_len);
> +       eir_parse(&eir_data, data, data_len, bdaddr_type);
>
>         if (bdaddr_type == BDADDR_BREDR)
>                 discoverable = true;
> @@ -7467,7 +7467,7 @@ static void connected_callback(uint16_t index, uint16_t length,
>
>         memset(&eir_data, 0, sizeof(eir_data));
>         if (eir_len > 0)
> -               eir_parse(&eir_data, ev->eir, eir_len);
> +               eir_parse(&eir_data, ev->eir, eir_len, ev->addr.type);
>
>         if (eir_data.class != 0)
>                 device_set_class(device, eir_data.class);
> diff --git a/src/eir.c b/src/eir.c
> index c984fa5..423a37f 100644
> --- a/src/eir.c
> +++ b/src/eir.c
> @@ -226,7 +226,8 @@ static void eir_parse_uuid128_data(struct eir_data *eir, const uint8_t *data,
>         eir_parse_sd(eir, &service, data + 16, len - 16);
>  }
>
> -void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
> +void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len,
> +                                                       uint8_t bdaddr_type)
>  {
>         uint16_t len = 0;
>
> @@ -242,15 +243,21 @@ void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
>                 const uint8_t *data;
>                 uint8_t data_len;
>
> -               /* Check for the end of EIR */
> -               if (field_len == 0)
> +               /* Do not continue EIR Data parsing if got incorrect length */
> +               if (len > eir_len)
>                         break;
>
>                 len += field_len + 1;
>
> -               /* Do not continue EIR Data parsing if got incorrect length */
> -               if (len > eir_len)
> +               if (field_len == 0) {
> +                       /* Ignore redundant advertising data over LE*/
> +                       if (bdaddr_type != BDADDR_BREDR) {
> +                               eir_data += field_len + 1;
> +                               continue;
> +                       }
> +                       /* field_len == 0 for BREDR means the end of EIR */
>                         break;
> +               }
>
>                 data = &eir_data[2];
>                 data_len = field_len - 1;
> @@ -370,7 +377,7 @@ int eir_parse_oob(struct eir_data *eir, uint8_t *eir_data, uint16_t eir_len)
>
>         /* optional OOB EIR data */
>         if (eir_len > 0)
> -               eir_parse(eir, eir_data, eir_len);
> +               eir_parse(eir, eir_data, eir_len, 0);
>
>         return 0;
>  }
> diff --git a/src/eir.h b/src/eir.h
> index 219ee79..944413d 100644
> --- a/src/eir.h
> +++ b/src/eir.h
> @@ -95,7 +95,8 @@ struct eir_data {
>  };
>
>  void eir_data_free(struct eir_data *eir);
> -void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len);
> +void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len,
> +                                                       uint8_t bdaddr_type);
>  int eir_parse_oob(struct eir_data *eir, uint8_t *eir_data, uint16_t eir_len);
>  int eir_create_oob(const bdaddr_t *addr, const char *name, uint32_t cod,
>                         const uint8_t *hash, const uint8_t *randomizer,
> diff --git a/unit/test-eir.c b/unit/test-eir.c
> index 53abfa4..09d9865 100644
> --- a/unit/test-eir.c
> +++ b/unit/test-eir.c
> @@ -44,6 +44,7 @@ struct test_data {
>         bool name_complete;
>         int8_t tx_power;
>         const char **uuid;
> +       uint8_t bdaddr_type;
>  };
>
>  static const unsigned char macbookair_data[] = {
> @@ -536,7 +537,7 @@ static void test_basic(const void *data)
>         memset(buf, 0, sizeof(buf));
>         memset(&eir, 0, sizeof(eir));
>
> -       eir_parse(&eir, buf, HCI_MAX_EIR_LENGTH);
> +       eir_parse(&eir, buf, HCI_MAX_EIR_LENGTH, BDADDR_BREDR);
>         g_assert(eir.services == NULL);
>         g_assert(eir.name == NULL);
>
> @@ -560,7 +561,7 @@ static void test_parsing(gconstpointer data)
>
>         memset(&eir, 0, sizeof(eir));
>
> -       eir_parse(&eir, test->eir_data, test->eir_size);
> +       eir_parse(&eir, test->eir_data, test->eir_size, test->bdaddr_type);
>
>         tester_debug("Flags: %d", eir.flags);
>         tester_debug("Name: %s", eir.name);
> @@ -639,6 +640,7 @@ static const struct test_data gigaset_gtag_test = {
>         .uuid = gigaset_gtag_uuid,
>         .name = "Gigaset G-tag",
>         .name_complete = true,
> +       .bdaddr_type = BDADDR_LE_PUBLIC,
>  };
>
>  static const char *uri_beacon_uuid[] = {
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

[PATCH 3/4] unit/eir: Add WahooHRM test

[Mariusz Skamra]

---
 unit/test-eir.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/unit/test-eir.c b/unit/test-eir.c
index 09d9865..0a8d0ce 100644
--- a/unit/test-eir.c
+++ b/unit/test-eir.c
@@ -660,6 +660,28 @@ static const struct test_data uri_beacon_test = {
 	.uuid = uri_beacon_uuid,
 };
 
+static const char *wahoo_hrm_uuid[] = {
+		"0000180d-0000-1000-8000-00805f9b34fb",
+		NULL
+};
+
+static const unsigned char wahoo_hrm_data[] = {
+		0x02, 0x01, 0x06, 0x03, 0x02, 0x0d, 0x18, 0x0f, 0x09,
+		0x57, 0x61, 0x68, 0x6f, 0x6f, 0x20, 0x48, 0x52, 0x4d,
+		0x20, 0x76, 0x32, 0x2e, 0x31
+};
+
+static const struct test_data wahoo_hrm_test = {
+	.eir_data = wahoo_hrm_data,
+	.eir_size = sizeof(wahoo_hrm_data),
+	.flags = 0x06,
+	.tx_power = 127,
+	.uuid = wahoo_hrm_uuid,
+	.name = "Wahoo HRM v2.1",
+	.name_complete = true,
+	.bdaddr_type = BDADDR_LE_RANDOM,
+};
+
 int main(int argc, char *argv[])
 {
 	tester_init(&argc, &argv);
@@ -685,6 +707,7 @@ int main(int argc, char *argv[])
 									NULL);
 	tester_add("ad/g-tag", &gigaset_gtag_test, NULL, test_parsing, NULL);
 	tester_add("ad/uri-beacon", &uri_beacon_test, NULL, test_parsing, NULL);
+	tester_add("ad/wahoo-hrm", &wahoo_hrm_test, NULL, test_parsing, NULL);
 
 	return tester_run();
 }
-- 
1.9.1

[PATCH 4/4] android/gatt: Fix autoconnect

[Mariusz Skamra]

This patch fixes autoconnect issue, however there is also race hazard in
Android Gatt framework. If app uses autoconnect while connecting,
direct flag is set to false. Then bt_gatt_add_autoconnect should be called
to add this app's id to autoconnect_apps queue.
---
 android/gatt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/android/gatt.c b/android/gatt.c
index 4da959f..72a27ab 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1972,6 +1972,7 @@ static bool trigger_connection(struct app_connection *conn, bool direct)
 		if (direct)
 			return connect_le(conn->device) == 0;
 
+		bt_gatt_add_autoconnect(conn->app->id, &conn->device->bdaddr);
 		return auto_connect_le(conn->device);
 	case DEVICE_CONNECTED:
 		notify_app_connect_status(conn, GATT_SUCCESS);
-- 
1.9.1

Re: [PATCH 4/4] android/gatt: Fix autoconnect

[Szymon Janc]

Hi Mariusz,

On Thursday 30 of April 2015 13:26:18 Mariusz Skamra wrote:
> This patch fixes autoconnect issue, however there is also race hazard in
> Android Gatt framework. If app uses autoconnect while connecting,
> direct flag is set to false. Then bt_gatt_add_autoconnect should be called
> to add this app's id to autoconnect_apps queue.
> ---
>  android/gatt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/android/gatt.c b/android/gatt.c
> index 4da959f..72a27ab 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -1972,6 +1972,7 @@ static bool trigger_connection(struct app_connection
> *conn, bool direct) if (direct)
>  			return connect_le(conn->device) == 0;
> 
> +		bt_gatt_add_autoconnect(conn->app->id, &conn->device->bdaddr);
>  		return auto_connect_le(conn->device);
>  	case DEVICE_CONNECTED:
>  		notify_app_connect_status(conn, GATT_SUCCESS);

This patch is now applied. Thanks.

-- 
BR
Szymon Janc