From 250568025c2fdebfc9b5e457f77cf24a6b898f8b Mon Sep 17 00:00:00 2001
From: Denis Kenzior <denkenz@gmail.com>
Date: Thu, 14 Jun 2018 16:57:51 -0500
Subject: [PATCH] network: Fix a bunch of double-frees

Missing secrets are freed by eap_send_agent_req() even in case of
failure, so it was erroneous to try to free them on error.

==1048== Invalid read of size 8
==1048==    at 0x1603EC: l_queue_clear (queue.c:101)
==1048==    by 0x1603B8: l_queue_destroy (queue.c:82)
==1048==    by 0x135328: network_connect_8021x (network.c:943)
==1048==    by 0x1354C4: network_connect (network.c:987)
==1048==    by 0x178DD2: _dbus_object_tree_dispatch (dbus-service.c:1690)
==1048==    by 0x16D32A: message_read_handler (dbus.c:285)
==1048==    by 0x166EC3: io_callback (io.c:123)
==1048==    by 0x165A1A: l_main_iterate (main.c:376)
==1048==    by 0x165B58: l_main_run (main.c:423)
==1048==    by 0x1102DA: main (main.c:458)
==1048==  Address 0x5461850 is 0 bytes inside a block of size 24 free'd
==1048==    at 0x4C2C13B: free (vg_replace_malloc.c:530)
==1048==    by 0x15ED03: l_free (util.c:136)
==1048==    by 0x1603C4: l_queue_destroy (queue.c:83)
==1048==    by 0x134BD5: eap_secret_request_free (network.c:719)
==1048==    by 0x134EF9: eap_send_agent_req (network.c:817)
==1048==    by 0x1352F7: network_connect_8021x (network.c:936)
==1048==    by 0x1354C4: network_connect (network.c:987)
==1048==    by 0x178DD2: _dbus_object_tree_dispatch (dbus-service.c:1690)
==1048==    by 0x16D32A: message_read_handler (dbus.c:285)
==1048==    by 0x166EC3: io_callback (io.c:123)
==1048==    by 0x165A1A: l_main_iterate (main.c:376)
==1048==    by 0x165B58: l_main_run (main.c:423)
---
 src/eap.c     | 49 +++++++++++++++++++++++++++++++++----------------
 src/network.c |  2 --
 2 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/src/eap.c b/src/eap.c
index 5cc321c3..84d08f73 100644
--- a/src/eap.c
+++ b/src/eap.c
@@ -414,12 +414,30 @@ void eap_secret_info_free(void *data)
 	l_free(info);
 }
 
+static int eap_setting_exists(struct l_settings *settings,
+				const char *setting,
+				struct l_queue *secrets,
+				struct l_queue *missing)
+{
+	if (l_settings_get_value(settings, "Security", setting))
+		return 0;
+
+	if (l_queue_find(secrets, eap_secret_info_match, setting))
+		return 0;
+
+	if (l_queue_find(missing, eap_secret_info_match, setting))
+		return 0;
+
+	return -ENOENT;
+}
+
 int eap_check_settings(struct l_settings *settings, struct l_queue *secrets,
 			const char *prefix, bool set_key_material,
 			struct l_queue **out_missing)
 {
 	char setting[64];
 	const char *method_name;
+	struct l_queue *missing = NULL;
 	const struct l_queue_entry *entry;
 	struct eap_method *method;
 	int ret = 0;
@@ -429,7 +447,6 @@ int eap_check_settings(struct l_settings *settings, struct l_queue *secrets,
 
 	if (!method_name) {
 		l_error("Property %s missing", setting);
-
 		return -ENOENT;
 	}
 
@@ -443,7 +460,6 @@ int eap_check_settings(struct l_settings *settings, struct l_queue *secrets,
 
 	if (!entry) {
 		l_error("EAP method \"%s\" unsupported", method_name);
-
 		return -ENOTSUP;
 	}
 
@@ -451,36 +467,37 @@ int eap_check_settings(struct l_settings *settings, struct l_queue *secrets,
 	if (set_key_material && !method->exports_msk) {
 		l_error("EAP method \"%s\" doesn't export key material",
 				method_name);
-
 		return -ENOTSUP;
 	}
 
 	if (method->check_settings)
 		ret = method->check_settings(settings, secrets,
-						prefix, out_missing);
+						prefix, &missing);
 	if (ret)
-		return ret;
+		goto error;
 
 	/*
 	 * Methods that provide the get_identity callback are responsible
 	 * for ensuring, inside check_settings(), that they have enough data
 	 * to return the identity after load_settings().
 	 */
-	if (method->get_identity)
-		return 0;
+	if (!method->get_identity) {
+		snprintf(setting, sizeof(setting), "%sIdentity", prefix);
 
-	snprintf(setting, sizeof(setting), "%sIdentity", prefix);
-	if (!l_settings_get_value(settings, "Security", setting) &&
-			!l_queue_find(secrets, eap_secret_info_match,
-					setting) &&
-			!l_queue_find(*out_missing, eap_secret_info_match,
-					setting)) {
-		l_error("Property %s is missing", setting);
-
-		return -ENOENT;
+		ret = eap_setting_exists(settings, setting, secrets, missing);
+		if (ret < 0) {
+			l_error("Property %s is missing", setting);
+			ret = -ENOENT;
+			goto error;
+		}
 	}
 
+	*out_missing = missing;
 	return 0;
+
+error:
+	l_queue_destroy(missing, eap_secret_info_free);
+	return ret;
 }
 
 bool eap_load_settings(struct eap_state *eap, struct l_settings *settings,
diff --git a/src/network.c b/src/network.c
index d0095d1c..6bf6efa5 100644
--- a/src/network.c
+++ b/src/network.c
@@ -940,8 +940,6 @@ static struct l_dbus_message *network_connect_8021x(struct network *network,
 	reply = dbus_error_no_agent(message);
 
 error:
-	l_queue_destroy(missing_secrets, eap_secret_info_free);
-
 	network_settings_close(network);
 
 	l_queue_destroy(network->secrets, eap_secret_info_free);