[PATCH BlueZ 0/6] plugins/policy: Fixes

[PATCH BlueZ 0/6] plugins/policy: Fixes

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

These patches fixes several problems with policy that were causing it to
behave weirdly. It also fixes a couple of connection policy bugs when
reconnection is active and another policy attempts to request a connection
which could interfere with the desired intervals.

Luiz Augusto von Dentz (6):
  lib/uuid: Fix using unitialized values
  plugins/policy: Revert patch 96db78604252eeb17614b9982ced95fd66c6c6fc
  plugins/policy: Set list separator
  main.conf: Remove spaces in reconnect list parameters
  plugins/policy: Disable other connect policies while reconnect is
    active
  audio/a2dp: Fix always setting -EAGAIN for all errors

 lib/uuid.c            |  7 +++++--
 plugins/policy.c      | 45 +++++++++++++++++++++++++--------------------
 profiles/audio/a2dp.c | 10 +++++-----
 src/main.conf         |  4 ++--
 4 files changed, 37 insertions(+), 29 deletions(-)

-- 
2.7.4

[PATCH BlueZ 1/6] lib/uuid: Fix using unitialized values

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

The strings passed to bt_uuid_strcmp may not be valid UUIDs so the return
of bt_string_to_uuid needs to be checked otherwise bt_uuid_cmp may attempt
to access unitialized values:

Conditional jump or move depends on uninitialised value(s)
   at 0x4C1D4D: bt_uuid_to_uuid128 (uuid.c:78)
   by 0x4C1F22: bt_uuid_cmp (uuid.c:131)
   by 0x4C24A8: bt_uuid_strcmp (uuid.c:286)
   by 0x40F8A8: reconnect_match (policy.c:514)
   by 0x40F8A8: service_cb (policy.c:655)
   by 0x499331: change_state (service.c:109)
   by 0x499BBB: btd_service_connecting_complete (service.c:361)
   by 0x4178C1: stream_state_changed (source.c:163)
   by 0x422C78: avdtp_sep_set_state (avdtp.c:1013)
   by 0x42372A: handle_transport_connect (avdtp.c:844)
   by 0x423D8B: avdtp_connect_cb (avdtp.c:2326)
   by 0x465BBB: connect_cb (btio.c:232)
   by 0x50CA702: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.4800.1)
 Uninitialised value was created by a stack allocation
   at 0x4C2460: bt_uuid_strcmp (uuid.c:280)
---
 lib/uuid.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/uuid.c b/lib/uuid.c
index ac071fa..d4c7002 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -280,8 +280,11 @@ int bt_uuid_strcmp(const void *a, const void *b)
 {
 	bt_uuid_t u1, u2;
 
-	bt_string_to_uuid(&u1, a);
-	bt_string_to_uuid(&u2, b);
+	if (bt_string_to_uuid(&u1, a) < 0)
+		return -EINVAL;
+
+	if (bt_string_to_uuid(&u2, b) < 0)
+		return -EINVAL;
 
 	return bt_uuid_cmp(&u1, &u2);
 }
-- 
2.7.4

[PATCH BlueZ 2/6] plugins/policy: Revert patch 96db78604252eeb17614b9982ced95fd66c6c6fc

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Patch 96db78604252eeb17614b9982ced95fd66c6c6fc actually breaks
iterating to each attempt since the attempt++ is already done in
reconnect_timeout callback.
---
 plugins/policy.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/plugins/policy.c b/plugins/policy.c
index 218a3ed..bbb6961 100644
--- a/plugins/policy.c
+++ b/plugins/policy.c
@@ -685,8 +685,6 @@ static void reconnect_set_timer(struct reconnect_data *reconnect)
 {
 	static int timeout = 0;
 
-	reconnect->attempt++;
-
 	if (reconnect->attempt < reconnect_intervals_len)
 		timeout = reconnect_intervals[reconnect->attempt];
 
-- 
2.7.4

[PATCH BlueZ 3/6] plugins/policy: Set list separator

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Set list separator to ',' as it is used in main.conf.
---
 plugins/policy.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/plugins/policy.c b/plugins/policy.c
index bbb6961..3058824 100644
--- a/plugins/policy.c
+++ b/plugins/policy.c
@@ -775,6 +775,8 @@ static int policy_init(void)
 		goto done;
 	}
 
+	g_key_file_set_list_separator(conf, ',');
+
 	reconnect_uuids = g_key_file_get_string_list(conf, "Policy",
 							"ReconnectUUIDs",
 							NULL, &gerr);
-- 
2.7.4

[PATCH BlueZ 4/6] main.conf: Remove spaces in reconnect list parameters

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

These spaces are not removed when parsing the file which may lead to
unexpected behavior when using the values commented out as default.
---
 src/main.conf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/main.conf b/src/main.conf
index 372fd8c..49528b9 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -71,7 +71,7 @@
 # timeout). The policy plugin should contain a sane set of values by
 # default, but this list can be overridden here. By setting the list to
 # empty the reconnection feature gets disabled.
-#ReconnectUUIDs=00001112-0000-1000-8000-00805f9b34fb, 0000111f-0000-1000-8000-00805f9b34fb, 0000110a-0000-1000-8000-00805f9b34fb
+#ReconnectUUIDs=00001112-0000-1000-8000-00805f9b34fb,0000111f-0000-1000-8000-00805f9b34fb,0000110a-0000-1000-8000-00805f9b34fb
 
 # ReconnectAttempts define the number of attempts to reconnect after a link
 # lost. Setting the value to 0 disables reconnecting feature.
@@ -81,7 +81,7 @@
 # attempts.
 # If the number of attempts defined in ReconnectAttempts is bigger than the
 # set of intervals the last interval is repeated until the last attempt.
-#ReconnectIntervals=1, 2, 4, 8, 16, 32, 64
+#ReconnectIntervals=1,2,4,8,16,32,64
 
 # AutoEnable defines option to enable all controllers when they are found.
 # This includes adapters present on start as well as adapters that are plugged
-- 
2.7.4

[PATCH BlueZ 5/6] plugins/policy: Disable other connect policies while reconnect is active

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Other policies shall not interfere while reconnect is active.
---
 plugins/policy.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/plugins/policy.c b/plugins/policy.c
index 3058824..0330456 100644
--- a/plugins/policy.c
+++ b/plugins/policy.c
@@ -95,10 +95,29 @@ struct policy_data {
 	uint8_t tg_retries;
 };
 
+static struct reconnect_data *reconnect_find(struct btd_device *dev)
+{
+	GSList *l;
+
+	for (l = reconnects; l; l = g_slist_next(l)) {
+		struct reconnect_data *reconnect = l->data;
+
+		if (reconnect->dev == dev)
+			return reconnect;
+	}
+
+	return NULL;
+}
+
 static void policy_connect(struct policy_data *data,
 						struct btd_service *service)
 {
 	struct btd_profile *profile = btd_service_get_profile(service);
+	struct reconnect_data *reconnect;
+
+	reconnect = reconnect_find(btd_service_get_device(service));
+	if (reconnect && reconnect->active)
+		return;
 
 	DBG("%s profile %s", device_get_path(data->dev), profile->name);
 
@@ -496,6 +515,7 @@ static void target_cb(struct btd_service *service,
 static void reconnect_reset(struct reconnect_data *reconnect)
 {
 	reconnect->attempt = 0;
+	reconnect->active = false;
 
 	if (reconnect->timer > 0) {
 		g_source_remove(reconnect->timer);
@@ -518,20 +538,6 @@ static bool reconnect_match(const char *uuid)
 	return false;
 }
 
-static struct reconnect_data *reconnect_find(struct btd_device *dev)
-{
-	GSList *l;
-
-	for (l = reconnects; l; l = g_slist_next(l)) {
-		struct reconnect_data *reconnect = l->data;
-
-		if (reconnect->dev == dev)
-			return reconnect;
-	}
-
-	return NULL;
-}
-
 static struct reconnect_data *reconnect_add(struct btd_service *service)
 {
 	struct btd_device *dev = btd_service_get_device(service);
@@ -643,7 +649,6 @@ static void service_cb(struct btd_service *service,
 	 */
 	reconnect = reconnect_add(service);
 
-	reconnect->active = false;
 	reconnect_reset(reconnect);
 
 	/*
@@ -675,7 +680,6 @@ static gboolean reconnect_timeout(gpointer data)
 		return FALSE;
 	}
 
-	reconnect->active = true;
 	reconnect->attempt++;
 
 	return FALSE;
@@ -685,10 +689,13 @@ static void reconnect_set_timer(struct reconnect_data *reconnect)
 {
 	static int timeout = 0;
 
+	reconnect->active = true;
+
 	if (reconnect->attempt < reconnect_intervals_len)
 		timeout = reconnect_intervals[reconnect->attempt];
 
-	DBG("%d seconds", timeout);
+	DBG("attempt %u/%zu %d seconds", reconnect->attempt + 1,
+						reconnect_attempts, timeout);
 
 	reconnect->timer = g_timeout_add_seconds(timeout, reconnect_timeout,
 								reconnect);
@@ -726,8 +733,6 @@ static void conn_fail_cb(struct btd_device *dev, uint8_t status)
 	if (!reconnect->active)
 		return;
 
-	reconnect->active = false;
-
 	/* Give up if we were powered off */
 	if (status == MGMT_STATUS_NOT_POWERED) {
 		reconnect_reset(reconnect);
-- 
2.7.4

[PATCH BlueZ 6/6] audio/a2dp: Fix always setting -EAGAIN for all errors

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

avdtp_error_posix_errno always return the posix errno not the negative
one which is then assigned to perror negative and then again as negative
in the switch statement making it assume the original value which is the
positive errno which don't match with the expected values which are
negative causing -EAGAIN to always be returned.
---
 profiles/audio/a2dp.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index b391fc2..db0736d 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -235,11 +235,11 @@ static int error_to_errno(struct avdtp_error *err)
 	if (avdtp_error_category(err) != AVDTP_ERRNO)
 		return -EIO;
 
-	perr = -avdtp_error_posix_errno(err);
-	switch (-perr) {
-	case -EHOSTDOWN:
-	case -ECONNABORTED:
-		return perr;
+	perr = avdtp_error_posix_errno(err);
+	switch (perr) {
+	case EHOSTDOWN:
+	case ECONNABORTED:
+		return -perr;
 	default:
 		/*
 		 * An unexpect error has occurred setup may be attempted again.
-- 
2.7.4

Re: [PATCH BlueZ 0/6] plugins/policy: Fixes

[Luiz Augusto von Dentz]

Hi,

On Thu, Jul 28, 2016 at 5:27 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> These patches fixes several problems with policy that were causing it to
> behave weirdly. It also fixes a couple of connection policy bugs when
> reconnection is active and another policy attempts to request a connection
> which could interfere with the desired intervals.
>
> Luiz Augusto von Dentz (6):
>   lib/uuid: Fix using unitialized values
>   plugins/policy: Revert patch 96db78604252eeb17614b9982ced95fd66c6c6fc
>   plugins/policy: Set list separator
>   main.conf: Remove spaces in reconnect list parameters
>   plugins/policy: Disable other connect policies while reconnect is
>     active
>   audio/a2dp: Fix always setting -EAGAIN for all errors
>
>  lib/uuid.c            |  7 +++++--
>  plugins/policy.c      | 45 +++++++++++++++++++++++++--------------------
>  profiles/audio/a2dp.c | 10 +++++-----
>  src/main.conf         |  4 ++--
>  4 files changed, 37 insertions(+), 29 deletions(-)
>
> --
> 2.7.4

Applied.


-- 
Luiz Augusto von Dentz