fix segfaults due to races & missing adapter

During start up, there are races between the
bluetooth, persistence, bluetooth-map and
bluetooth-pbap bindings; on top of it, in the
case there's no bluetooth controller in the system,
various key parameters can exhibit a null value.
The combination of these races and the lack of
bluetooth controller can manifest in a number of
segfaults in the bluetooth binding (it's very easy
to reproduce on qemu).

This commit brings in several changes to make the
bluetooth binding resilient to the races and lack
of bluetooth controller:
* on startup, retrieve the default adapter from
the persistence binding from the init thread
* store the system default adapter in the service
init data structure, and store the active adapter
in the bluetooth state structure.
* update get_default_adapter to return value
obtained from persistence biding, without further
processing;
* add guards to make sure the userdata retrieved
from the application framework is valid;
* on verbs processing, ensure the caller provides
an adapter to apply the verb to, or that at least
there's a valid adapter associated to the
bluetooth state structure;
* initialize agent_data's device_path following free
operation;
* add guards for mediaplayer_path.

Bug-AGL: SPEC-3301
Signed-off-by: Raquel Medina <raquel.medina@konsulko.com>
Change-Id: Ia5a0dc9a61024ff43cd247216d0dff6918046f7e

4 files changed, 66 insertions, 21 deletions

diff --git a/binding/bluetooth-api.c b/binding/bluetooth-api.c
index 24ff11c..8e4e1f9 100644
--- a/binding/bluetooth-api.c
+++ b/binding/bluetooth-api.c
@@ -231,7 +231,7 @@ void call_work_destroy_unlocked(struct call_work *cw)
 
 	/* agent struct data */
 	g_free(cw->agent_data.device_path);
-
+	cw->agent_data.device_path = NULL;
 	g_free(cw->access_type);
 	g_free(cw->type_arg);
 	g_free(cw->method);
@@ -646,6 +646,9 @@ static gpointer bluetooth_func(gpointer ptr)
 	id->ns = ns;
 	rc = bluetooth_get_adapters_count(id);
 	if (rc > 0) {
+		/* TODO: add logic to match managed objects response
+		against default_adapter */
+		id->ns->adapter = id->default_adapter;
 		rc = bluetooth_register_agent(id);
 		if (rc) {
 			AFB_ERROR("bluetooth_register_agent() failed");
@@ -713,6 +716,10 @@ static int init(afb_api_t api)
 		return ret;
 	}
 
+	id->default_adapter = get_default_adapter(id->api);
+	if (!id->default_adapter)
+		id->default_adapter = g_strdup(BLUEZ_DEFAULT_ADAPTER);
+
 	args = json_object_new_object();
 	json_object_object_add(args , "technology", json_object_new_string("bluetooth"));
 	afb_api_call_sync(api, "network-manager", "enable_technology", args, NULL, NULL, NULL);
@@ -743,13 +750,16 @@ static int init(afb_api_t api)
 	else
 		AFB_INFO("bluetooth-binding operational");
 
-	id->ns->default_adapter = get_default_adapter(id->api);
-
 	return id->rc;
 }
 
 static void mediaplayer1_send_event(struct bluetooth_state *ns)
 {
+	if (!ns->mediaplayer_path) {
+		AFB_ERROR("NULL mediaplayer_path");
+		return;
+	}
+
 	gchar *player = g_strdup(ns->mediaplayer_path);
 	json_object *jresp = mediaplayer_properties(ns, NULL, player);
 
@@ -775,6 +785,11 @@ static void bluetooth_subscribe_unsubscribe(afb_req_t request,
 	afb_event_t event;
 	int rc;
 
+	if (!ns) {
+		afb_req_fail(request, "failed", "Cannot process request");
+		return;
+	}
+
 	/* if value exists means to set offline mode */
 	value = afb_req_value(request, "value");
 	if (!value) {
@@ -826,9 +841,20 @@ static void bluetooth_list(afb_req_t request)
 	GError *error = NULL;
 	json_object *jresp;
 
+	if (!ns) {
+		afb_req_fail(request, "failed", "Cannot process request");
+		return;
+	}
+
 	jresp = object_properties(ns, &error);
+	if (error) {
+		AFB_INFO("object_properties error: %s",
+			error->message);
+		g_clear_error(&error);
+		afb_req_fail(request, "failed", "BlueZ managed objects error");
+	} else
+		afb_req_success(request, jresp, "BlueZ managed objects");
 
-	afb_req_success(request, jresp, "Bluetooth - managed objects");
 }
 
 static void bluetooth_state(afb_req_t request)
@@ -838,7 +864,12 @@ static void bluetooth_state(afb_req_t request)
 	json_object *jresp;
 	const char *adapter = afb_req_value(request, "adapter");
 
-	adapter = BLUEZ_ROOT_PATH(adapter ? adapter : ns->default_adapter);
+	if (!ns || (!adapter && !ns->adapter)) {
+		afb_req_fail(request, "failed", "No adapter");
+		return;
+	}
+
+	adapter = BLUEZ_ROOT_PATH(adapter ? adapter : ns->adapter);
 
 	jresp = adapter_properties(ns, &error, adapter);
 	if (!jresp) {
@@ -857,7 +888,12 @@ static void bluetooth_adapter(afb_req_t request)
 	const char *adapter = afb_req_value(request, "adapter");
 	const char *scan, *discoverable, *powered, *filter, *transport;
 
-	adapter = BLUEZ_ROOT_PATH(adapter ? adapter : ns->default_adapter);
+	if (!ns || (!adapter && !ns->adapter)) {
+		afb_req_fail(request, "failed", "No adapter");
+		return;
+	}
+
+	adapter = BLUEZ_ROOT_PATH(adapter ? adapter : ns->adapter);
 
 	scan = afb_req_value(request, "discovery");
 	if (scan) {
@@ -958,18 +994,25 @@ static void bluetooth_default_adapter(afb_req_t request)
 	struct bluetooth_state *ns = bluetooth_get_userdata(request);
 	const char *adapter = afb_req_value(request, "adapter");
 	json_object *jresp = json_object_new_object();
+	int rc;
+
+	if (!ns || !adapter ) {
+		afb_req_fail(request, "failed",
+			     "No default adapter");
+		return;
+	}
 
 	call_work_lock(ns);
-	if (adapter) {
-		set_default_adapter(afb_req_get_api(request), adapter);
+	rc = set_default_adapter(afb_req_get_api(request), adapter);
 
-		if (ns->default_adapter)
-			g_free(ns->default_adapter);
-		ns->default_adapter = g_strdup(adapter);
-	}
+	if (ns->adapter)
+		g_free(ns->adapter);
+	ns->adapter = g_strdup(adapter);
 
-	json_object_object_add(jresp, "adapter", json_object_new_string(ns->default_adapter));
+	json_object_object_add(jresp, "adapter", json_object_new_string(ns->adapter));
 	call_work_unlock(ns);
+	if (rc)
+		AFB_DEBUG("Request to save default adapter to persistent storage failed ");
 
 	afb_req_success(request, jresp, "Bluetooth - default adapter");
 }
@@ -1206,7 +1249,7 @@ static void bluetooth_confirm_pairing(afb_req_t request)
 		pin = (int)strtol(value, NULL, 10);
 
 	if (!value || !pin) {
-		afb_req_fail_f(request, "failed", "No pincode parameter");
+		afb_req_fail(request, "failed", "No pincode parameter");
 		return;
 	}
 
diff --git a/binding/bluetooth-common.h b/binding/bluetooth-common.h
index 0e719eb..70975a5 100644
--- a/binding/bluetooth-common.h
+++ b/binding/bluetooth-common.h
@@ -63,7 +63,7 @@ struct bluetooth_state {
 	gchar *mediaplayer_path;
 
 	/* adapter */
-	gchar *default_adapter;
+	gchar *adapter;
 };
 
 struct init_data {
@@ -71,6 +71,7 @@ struct init_data {
 	GMutex mutex;
 	gboolean init_done;
 	afb_api_t api;
+	gchar *default_adapter;
 	struct bluetooth_state *ns; /* before setting afb_api_set_userdata() */
 	int rc;
 };
diff --git a/binding/bluetooth-conf.c b/binding/bluetooth-conf.c
index d9edf98..2f6f475 100644
--- a/binding/bluetooth-conf.c
+++ b/binding/bluetooth-conf.c
@@ -38,7 +38,7 @@
 gchar *get_default_adapter(afb_api_t api)
 {
 	json_object *response, *query, *val;
-	gchar *adapter;
+	gchar *adapter = NULL;
 	int ret;
 
 	query = json_object_new_object();
@@ -46,13 +46,14 @@ gchar *get_default_adapter(afb_api_t api)
 
 	ret = afb_api_call_sync(api, "persistence", "read", query, &response, NULL, NULL);
 	if (ret < 0)
-		return g_strdup(BLUEZ_DEFAULT_ADAPTER);
+		goto out;
 
-	json_object_object_get_ex(response, "value", &val);
-	adapter = g_strdup(json_object_get_string(val));
+	if (json_object_object_get_ex(response, "value", &val))
+		adapter = g_strdup(json_object_get_string(val));
 	json_object_put(response);
 
-	return adapter ? adapter : g_strdup(BLUEZ_DEFAULT_ADAPTER);
+out:
+	return adapter;
 }
 
 int set_default_adapter(afb_api_t api, const char *adapter)
diff --git a/binding/bluetooth-util.c b/binding/bluetooth-util.c
index 8a50731..21bd0df 100644
--- a/binding/bluetooth-util.c
+++ b/binding/bluetooth-util.c
@@ -1048,7 +1048,7 @@ gchar *return_bluez_path(afb_req_t request) {
 	const char *device, *tmp;
 
 	call_work_lock(ns);
-	adapter = adapter ? adapter : ns->default_adapter;
+	adapter = adapter ? adapter : ns->adapter;
 	call_work_unlock(ns);
 
 	device = afb_req_value(request, "device");