From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Thu, 9 Jul 2015 03:48:19 +0000 (-0400)
Subject: Always free clients list if we allocated it Closes #1130
X-Git-Tag: release_3_0_10~371
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5b6d5e78acc4fc78566f33be8a575536a059c01a;p=thirdparty%2Ffreeradius-server.git

Always free clients list if we allocated it Closes #1130
---

diff --git a/src/main/client.c b/src/main/client.c
index f12650bd573..e4691ed2bdf 100644
--- a/src/main/client.c
+++ b/src/main/client.c
@@ -527,10 +527,8 @@ static const CONF_PARSER client_config[] = {
 	{ NULL, -1, 0, NULL, NULL }
 };
 
-/*
- *	Create the linked list of clients from the new configuration
- *	type.  This way we don't have to change too much in the other
- *	source-files.
+/** Create the linked list of clients from the new configuration type
+ *
  */
 #ifdef WITH_TLS
 RADCLIENT_LIST *client_list_parse_section(CONF_SECTION *section, bool tls_required)
@@ -540,8 +538,8 @@ RADCLIENT_LIST *client_list_parse_section(CONF_SECTION *section, UNUSED bool tls
 {
 	bool		global = false, in_server = false;
 	CONF_SECTION	*cs;
-	RADCLIENT	*c;
-	RADCLIENT_LIST	*clients;
+	RADCLIENT	*c = NULL;
+	RADCLIENT_LIST	*clients = NULL;
 
 	/*
 	 *	Be forgiving.  If there's already a clients, return
@@ -557,22 +555,12 @@ RADCLIENT_LIST *client_list_parse_section(CONF_SECTION *section, UNUSED bool tls
 
 	if (strcmp("server", cf_section_name1(section)) == 0) in_server = true;
 
-	/*
-	 *	Associate the clients structure with the section.
-	 */
-	if (cf_data_add(section, "clients", clients, NULL) < 0) {
-		cf_log_err_cs(section,
-			   "Failed to associate clients with section %s",
-		       cf_section_name1(section));
-		client_list_free(clients);
-		return NULL;
-	}
-
 	for (cs = cf_subsection_find_next(section, NULL, "client");
-	     cs != NULL;
+	     cs;
 	     cs = cf_subsection_find_next(section, cs, "client")) {
 		c = client_afrom_cs(cs, cs, in_server, false);
 		if (!c) {
+			client_list_free(clients);
 			return NULL;
 		}
 
@@ -583,6 +571,7 @@ RADCLIENT_LIST *client_list_parse_section(CONF_SECTION *section, UNUSED bool tls
 		 */
 		if (tls_required != c->tls_required) {
 			cf_log_err_cs(cs, "Client does not have the same TLS configuration as the listener");
+		error:
 			client_free(c);
 			client_list_free(clients);
 			return NULL;
@@ -597,12 +586,12 @@ RADCLIENT_LIST *client_list_parse_section(CONF_SECTION *section, UNUSED bool tls
 #ifdef WITH_DYNAMIC_CLIENTS
 #ifdef HAVE_DIRENT_H
 		if (c->client_server) {
-			char const *value;
-			CONF_PAIR *cp;
+			char const	*value;
+			CONF_PAIR	*cp;
 			DIR		*dir;
 			struct dirent	*dp;
-			struct stat stat_buf;
-			char buf2[2048];
+			struct stat	stat_buf;
+			char		buf2[2048];
 
 			/*
 			 *	Find the directory where individual
@@ -613,10 +602,8 @@ RADCLIENT_LIST *client_list_parse_section(CONF_SECTION *section, UNUSED bool tls
 
 			value = cf_pair_value(cp);
 			if (!value) {
-				cf_log_err_cs(cs,
-					   "The \"directory\" entry must not be empty");
-				client_free(c);
-				return NULL;
+				cf_log_err_cs(cs, "The \"directory\" entry must not be empty");
+				goto error;
 			}
 
 			DEBUG("including dynamic clients in %s", value);
@@ -624,8 +611,7 @@ RADCLIENT_LIST *client_list_parse_section(CONF_SECTION *section, UNUSED bool tls
 			dir = opendir(value);
 			if (!dir) {
 				cf_log_err_cs(cs, "Error reading directory %s: %s", value, fr_syserror(errno));
-				client_free(c);
-				return NULL;
+				goto error;
 			}
 
 			/*
@@ -645,33 +631,27 @@ RADCLIENT_LIST *client_list_parse_section(CONF_SECTION *section, UNUSED bool tls
 					    isdigit((int)*p) ||
 					    (*p == ':') ||
 					    (*p == '.')) continue;
-						break;
+					break;
 				}
 				if (*p != '\0') continue;
 
-				snprintf(buf2, sizeof(buf2), "%s/%s",
-					 value, dp->d_name);
+				snprintf(buf2, sizeof(buf2), "%s/%s", value, dp->d_name);
 
-				if ((stat(buf2, &stat_buf) != 0) ||
-				    S_ISDIR(stat_buf.st_mode)) continue;
+				if ((stat(buf2, &stat_buf) != 0) || S_ISDIR(stat_buf.st_mode)) continue;
 
 				dc = client_read(buf2, in_server, true);
 				if (!dc) {
-					cf_log_err_cs(cs,
-						   "Failed reading client file \"%s\"", buf2);
-					client_free(c);
+					cf_log_err_cs(cs, "Failed reading client file \"%s\"", buf2);
 					closedir(dir);
-					return NULL;
+					goto error;
 				}
 
 				/*
 				 *	Validate, and add to the list.
 				 */
 				if (!client_add_dynamic(clients, c, dc)) {
-
-					client_free(c);
 					closedir(dir);
-					return NULL;
+					goto error;
 				}
 			} /* loop over the directory */
 			closedir(dir);
@@ -681,23 +661,27 @@ RADCLIENT_LIST *client_list_parse_section(CONF_SECTION *section, UNUSED bool tls
 	add_client:
 #endif /* WITH_DYNAMIC_CLIENTS */
 		if (!client_add(clients, c)) {
-			cf_log_err_cs(cs,
-				   "Failed to add client %s",
-				   cf_section_name2(cs));
-			client_free(c);
-			return NULL;
+			cf_log_err_cs(cs, "Failed to add client %s", cf_section_name2(cs));
+			goto error;
 		}
 
 	}
 
+	/*
+	 *	Associate the clients structure with the section.
+	 */
+	if (cf_data_add(section, "clients", clients, NULL) < 0) {
+		cf_log_err_cs(section, "Failed to associate clients with section %s", cf_section_name1(section));
+		client_list_free(clients);
+		return NULL;
+	}
+
 	/*
 	 *	Replace the global list of clients with the new one.
 	 *	The old one is still referenced from the original
 	 *	configuration, and will be freed when that is freed.
 	 */
-	if (global) {
-		root_clients = clients;
-	}
+	if (global) root_clients = clients;
 
 	return clients;
 }