[PATCH BlueZ 1/2 v2] thermometer: Fix crash while unregistering adapter

[PATCH BlueZ 1/2 v2] thermometer: Fix crash while unregistering adapter

[Luiz Augusto von Dentz]

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

Invalid read of size 8
   at 0x448200: g_attrib_unregister (gattrib.c:722)
   by 0x440476: destroy_thermometer (thermometer.c:167)
   by 0x40D849: remove_interface (object.c:656)
   by 0x40DAA9: g_dbus_unregister_interface (object.c:1413)
   by 0x3DF7A63C9C: g_slist_foreach (gslist.c:894)
   by 0x469656: device_remove (device.c:2200)
   by 0x45CDC1: adapter_remove (adapter.c:3884)
   by 0x45F146: index_removed (adapter.c:5442)
   by 0x46BC17: received_data (mgmt.c:252)
   by 0x3DF7A47A74: g_main_context_dispatch (gmain.c:2715)
   by 0x3DF7A47DA7: g_main_context_iterate.isra.24 (gmain.c:3290)
   by 0x3DF7A481A1: g_main_loop_run (gmain.c:3484)
 Address 0x40 is not stack'd, malloc'd or (recently) free'd
---
v2: Print a warning if invalid id is passed to g_attrib_unregister

 profiles/thermometer/thermometer.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 0cf14e6..1b299e7 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -164,12 +164,12 @@ static void destroy_thermometer(gpointer user_data)
 	if (t->attioid > 0)
 		btd_device_remove_attio_callback(t->dev, t->attioid);
 
-	g_attrib_unregister(t->attrib, t->attio_measurement_id);
-	g_attrib_unregister(t->attrib, t->attio_intermediate_id);
-	g_attrib_unregister(t->attrib, t->attio_interval_id);
-
-	if (t->attrib != NULL)
+	if (t->attrib != NULL) {
+		g_attrib_unregister(t->attrib, t->attio_measurement_id);
+		g_attrib_unregister(t->attrib, t->attio_intermediate_id);
+		g_attrib_unregister(t->attrib, t->attio_interval_id);
 		g_attrib_unref(t->attrib);
+	}
 
 	btd_device_unref(t->dev);
 	g_free(t->svc_range);
-- 
1.8.1

[PATCH BlueZ 2/2 v2] attrib: Don't attempt to unregister event id 0

[Luiz Augusto von Dentz]

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

Id 0 is considered invalid so the code should not even try to lookup for
it in the event list instead print a warning and return FALSE
immediatelly.
---
 attrib/gattrib.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 58f19d0..01c19f9 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -719,6 +719,11 @@ gboolean g_attrib_unregister(GAttrib *attrib, guint id)
 	struct event *evt;
 	GSList *l;
 
+	if (id == 0) {
+		warn("%s: invalid id", __FUNCTION__);
+		return FALSE;
+	}
+
 	l = g_slist_find_custom(attrib->events, GUINT_TO_POINTER(id),
 							event_cmp_by_id);
 	if (l == NULL)
-- 
1.8.1

Re: [PATCH BlueZ 1/2 v2] thermometer: Fix crash while unregistering adapter

[Johan Hedberg]

Hi Luiz,

On Mon, Jan 28, 2013, Luiz Augusto von Dentz wrote:
> Invalid read of size 8
>    at 0x448200: g_attrib_unregister (gattrib.c:722)
>    by 0x440476: destroy_thermometer (thermometer.c:167)
>    by 0x40D849: remove_interface (object.c:656)
>    by 0x40DAA9: g_dbus_unregister_interface (object.c:1413)
>    by 0x3DF7A63C9C: g_slist_foreach (gslist.c:894)
>    by 0x469656: device_remove (device.c:2200)
>    by 0x45CDC1: adapter_remove (adapter.c:3884)
>    by 0x45F146: index_removed (adapter.c:5442)
>    by 0x46BC17: received_data (mgmt.c:252)
>    by 0x3DF7A47A74: g_main_context_dispatch (gmain.c:2715)
>    by 0x3DF7A47DA7: g_main_context_iterate.isra.24 (gmain.c:3290)
>    by 0x3DF7A481A1: g_main_loop_run (gmain.c:3484)
>  Address 0x40 is not stack'd, malloc'd or (recently) free'd
> ---
> v2: Print a warning if invalid id is passed to g_attrib_unregister
> 
>  profiles/thermometer/thermometer.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Both patches have been applied. Thanks.

Johan

[PATCH BlueZ 1/2 v2] thermometer: Fix crash while unregistering adapter

[Luiz Augusto von Dentz]

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

Invalid read of size 8
   at 0x448200: g_attrib_unregister (gattrib.c:722)
   by 0x440476: destroy_thermometer (thermometer.c:167)
   by 0x40D849: remove_interface (object.c:656)
   by 0x40DAA9: g_dbus_unregister_interface (object.c:1413)
   by 0x3DF7A63C9C: g_slist_foreach (gslist.c:894)
   by 0x469656: device_remove (device.c:2200)
   by 0x45CDC1: adapter_remove (adapter.c:3884)
   by 0x45F146: index_removed (adapter.c:5442)
   by 0x46BC17: received_data (mgmt.c:252)
   by 0x3DF7A47A74: g_main_context_dispatch (gmain.c:2715)
   by 0x3DF7A47DA7: g_main_context_iterate.isra.24 (gmain.c:3290)
   by 0x3DF7A481A1: g_main_loop_run (gmain.c:3484)
 Address 0x40 is not stack'd, malloc'd or (recently) free'd
---
v2: Print a warning if invalid id is passed to g_attrib_unregister

 profiles/thermometer/thermometer.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 0cf14e6..1b299e7 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -164,12 +164,12 @@ static void destroy_thermometer(gpointer user_data)
 	if (t->attioid > 0)
 		btd_device_remove_attio_callback(t->dev, t->attioid);
 
-	g_attrib_unregister(t->attrib, t->attio_measurement_id);
-	g_attrib_unregister(t->attrib, t->attio_intermediate_id);
-	g_attrib_unregister(t->attrib, t->attio_interval_id);
-
-	if (t->attrib != NULL)
+	if (t->attrib != NULL) {
+		g_attrib_unregister(t->attrib, t->attio_measurement_id);
+		g_attrib_unregister(t->attrib, t->attio_intermediate_id);
+		g_attrib_unregister(t->attrib, t->attio_interval_id);
 		g_attrib_unref(t->attrib);
+	}
 
 	btd_device_unref(t->dev);
 	g_free(t->svc_range);
-- 
1.8.1