Merge pull request #1222 from pi-hole/fix/linebuffer_freeing

Prevent race collisions when parsing FTL config file

src/config.c

@@ -39,17 +39,31 @@ FTLFileNamesStruct FTLfiles = {
 	NULL
 };
 
+pthread_mutex_t lock;
+
 // Private global variables
 static char *conflinebuffer = NULL;
 static size_t size = 0;
 
 // Private prototypes
 static char *parse_FTLconf(FILE *fp, const char * key);
-static void release_config_memory(void);
 static void getpath(FILE* fp, const char *option, const char *defaultloc, char **pointer);
 static void set_nice(const char *buffer, int fallback);
 static bool read_bool(const char *option, const bool fallback);
 
+void init_config_mutex(void)
+{
+	// Initialize the lock attributes
+	pthread_mutexattr_t lock_attr = {};
+	pthread_mutexattr_init(&lock_attr);
+
+	// Initialize the lock
+	pthread_mutex_init(&lock, &lock_attr);
+
+	// Destroy the lock attributes since we're done with it
+	pthread_mutexattr_destroy(&lock_attr);
+}
+
 void getLogFilePath(void)
 {
 	FILE *fp;
@@ -643,9 +657,6 @@ void read_FTLconf(void)
 
 	logg("Finished config file parsing");
 
-	// Release memory
-	release_config_memory();
-
 	if(fp != NULL)
 		fclose(fp);
 }
@@ -684,7 +695,7 @@ static void getpath(FILE* fp, const char *option, const char *defaultloc, char *
 	}
 }
 
-static char *parse_FTLconf(FILE *fp, const char * key)
+static char *parse_FTLconf(FILE *fp, const char *key)
 {
 	// Return NULL if fp is an invalid file pointer
 	if(fp == NULL)
@@ -698,12 +709,24 @@ static char *parse_FTLconf(FILE *fp, const char * key)
 	}
 	sprintf(keystr, "%s=", key);
 
+	// Lock mutex
+	const int lret = pthread_mutex_lock(&lock);
+	if(config.debug & DEBUG_LOCKS)
+		logg("Obtained config lock");
+	if(lret != 0)
+		logg("Error when obtaining config lock: %s", strerror(lret));
+
 	// Go to beginning of file
 	fseek(fp, 0L, SEEK_SET);
 
 	if(config.debug & DEBUG_EXTRA)
 		logg("initial: conflinebuffer = %p, keystr = %p, size = %zu", conflinebuffer, keystr, size);
 
+	// Set size to zero if conflinebuffer is not available here
+	// This causes getline() to allocate memory for the buffer itself
+	if(conflinebuffer == NULL && size != 0)
+		size = 0;
+
 	errno = 0;
 	while(getline(&conflinebuffer, &size, fp) != -1)
 	{
@@ -724,36 +747,42 @@ static char *parse_FTLconf(FILE *fp, const char * key)
 		if((strstr(conflinebuffer, keystr)) == NULL)
 			continue;
 
-		// otherwise: key found
-		free(keystr);
-		// Note: value is still a pointer into the conflinebuffer
-		//       its memory will get released in release_config_memory()
+		// *** MATCH ****
+
+		// Note: value is still a pointer into the conflinebuffer,
+		// no need to duplicate memory here
 		char *value = find_equals(conflinebuffer) + 1;
-		// Trim whitespace at beginning and end, this function
-		// modifies the string inplace
+
+		// Trim whitespace at beginning and end, this function modifies
+		// the string inplace
 		trim_whitespace(value);
+
+		const int uret = pthread_mutex_unlock(&lock);
+		if(config.debug & DEBUG_LOCKS)
+			logg("Released config lock (match)");
+		if(uret != 0)
+			logg("Error when releasing config lock (match): %s", strerror(uret));
+
+		// Free keystr memory
+		free(keystr);
 		return value;
 	}
 
 	if(errno == ENOMEM)
 		logg("WARN: parse_FTLconf failed: could not allocate memory for getline");
 
+	const int uret = pthread_mutex_unlock(&lock);
+	if(config.debug & DEBUG_LOCKS)
+		logg("Released config lock (no match)");
+	if(uret != 0)
+		logg("Error when releasing config lock (no match): %s", strerror(uret));
+
 	// Key not found or memory error -> return NULL
 	free(keystr);
 
 	return NULL;
 }
 
-void release_config_memory(void)
-{
-	if(conflinebuffer != NULL)
-	{
-		free(conflinebuffer);
-		conflinebuffer = NULL;
-		size = 0;
-	}
-}
-
 void get_privacy_level(FILE *fp)
 {
 	// See if we got a file handle, if not we have to open
@@ -781,9 +810,6 @@ void get_privacy_level(FILE *fp)
 		}
 	}
 
-	// Release memory
-	release_config_memory();
-
 	// Have to close the config file if we opened it
 	if(opened)
 		fclose(fp);
@@ -823,9 +849,6 @@ void get_blocking_mode(FILE *fp)
 			logg("Ignoring unknown blocking mode, fallback is NULL blocking");
 	}
 
-	// Release memory
-	release_config_memory();
-
 	// Have to close the config file if we opened it
 	if(opened)
 		fclose(fp);
@@ -992,14 +1015,7 @@ void read_debuging_settings(FILE *fp)
 
 	// Have to close the config file if we opened it
 	if(opened)
-	{
 		fclose(fp);
-
-		// Release memory only when we opened the file
-		// Otherwise, it may still be needed outside of
-		// this function (initial config parsing)
-		release_config_memory();
-	}
 }
 
 

src/config.h

@@ -22,6 +22,7 @@
 // struct in_addr, in6_addr
 #include <netinet/in.h>
 
+void init_config_mutex(void);
 void getLogFilePath(void);
 void read_FTLconf(void);
 void get_privacy_level(FILE *fp);

src/main.c

@@ -47,6 +47,7 @@ int main (int argc, char* argv[])
 	parse_args(argc, argv);
 
 	// Try to open FTL log
+	init_config_mutex();
 	init_FTL_log();
 	timer_start(EXIT_TIMER);
 	logg("########## FTL started on %s! ##########", hostname());