From mboxrd@z Thu Jan  1 00:00:00 1970
From: guido@trentalancia.com (Guido Trentalancia)
Date: Sat, 30 Sep 2017 01:30:50 +0200
Subject: [refpolicy] [PATCH v3] fc_sort: memory leakages
In-Reply-To: <1506710145.25223.1.camel@trentalancia.com>
References: <CAFftDdr6uZxWz9FPBnP4RW0Y-KLcS7aFTq0hHaF_dNB6K8wi2g@mail.gmail.com>
	<1506643940.32317.6.camel@trentalancia.com>
	<CAFftDdqy_BxjsPZjqqPFQZxhVA8sHG1Fq1adfCgjKAkuBsNk-w@mail.gmail.com>
	<1506703966.24492.1.camel@trentalancia.com>
	<1506710145.25223.1.camel@trentalancia.com>
Message-ID: <1506727850.25420.6.camel@trentalancia.com>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

Avoid memory leakages in the fc_sort executable (now passes
all valgrind tests fine).

Some NULL pointer checks with or without associated error
reporting.

Some white space and comment formatting fixes.

This is the third version of this patch. Please do not use
the first version as it introduces a serious bug.

Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
---
 support/fc_sort.c |  137 ++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 117 insertions(+), 20 deletions(-)

--- a/support/fc_sort.c	2017-09-29 19:01:28.012455758 +0200
+++ b/support/fc_sort.c	2017-09-30 01:21:59.316362417 +0200
@@ -46,6 +46,9 @@ typedef struct file_context_node {
 
 void file_context_node_destroy(file_context_node_t *x)
 {
+	if (!x)
+		return;
+
 	free(x->path);
 	free(x->file_type);
 	free(x->context);
@@ -307,6 +310,41 @@ void fc_fill_data(file_context_node_t *f
 	}
 }
 
+
+
+/* fc_free_file_context_node_list
+ * Free the memory allocated to the linked list and its elements.
+ */
+void fc_free_file_context_node_list(struct file_context_node *node)
+{
+	struct file_context_node *next;
+
+	while (node) {
+		next = node->next;
+		file_context_node_destroy(node);
+		free(node);
+		node = next;
+	}
+}
+
+
+
+/* fc_free_file_context_bucket_list
+ * Free the memory allocated to the linked list and its elements.
+ */
+void fc_free_file_context_bucket_list(struct file_context_bucket *element)
+{
+	struct file_context_bucket *next;
+
+	while (element) {
+		next = element->next;
+		file_context_node_destroy(element->data);
+		free(element->data);
+		free(element);
+		element = next;
+	}
+}
+
 /* main
  * This program takes in two arguments, the input filename and the
  *  output filename. The input file should be syntactically correct.
@@ -328,7 +366,6 @@ int main(int argc, char *argv[])
 
 	FILE *in_file, *out_file;
 
-
 	/* Check for the correct number of command line arguments. */
 	if (argc < 2 || argc > 3) {
 		fprintf(stderr, "Usage: %s <infile> [<outfile>]\n",argv[0]);
@@ -348,15 +385,26 @@ int main(int argc, char *argv[])
 
 	/* Initialize the head of the linked list. */
 	head = current = (file_context_node_t*)malloc(sizeof(file_context_node_t));
+	if (!head) {
+		fprintf(stderr, "Error: failure allocating memory.\n");
+		return 1;
+	}
 	head->next = NULL;
+	head->path = NULL;
+	head->file_type = NULL;
+	head->context = NULL;
 
 	/* Parse the file into a file_context linked list. */
 	line_buf = NULL;
 
 	while ( getline(&line_buf, &buf_len, in_file) != -1 ){
 		line_len = strlen(line_buf);
-		if( line_len == 0 || line_len == 1)
+		if( line_len == 0 || line_len == 1) {
+			free(line_buf);
+			line_buf = NULL;
 			continue;
+		}
+
 		/* Get rid of whitespace from the front of the line. */
 		for (i = 0; i < line_len; i++) {
 			if (!isspace(line_buf[i]))
@@ -364,16 +412,25 @@ int main(int argc, char *argv[])
 		}
 
 
-		if (i >= line_len)
+		if (i >= line_len) {
+			free(line_buf);
+			line_buf = NULL;
 			continue;
+		}
+
 		/* Check if the line isn't empty and isn't a comment */
-		if (line_buf[i] == '#')
+		if (line_buf[i] == '#') {
+			free(line_buf);
+			line_buf = NULL;
 			continue;
+		}
 
 		/* We have a valid line - allocate a new node. */
 		temp = (file_context_node_t *)malloc(sizeof(file_context_node_t));
 		if (!temp) {
+			free(line_buf);
 			fprintf(stderr, "Error: failure allocating memory.\n");
+			fc_free_file_context_node_list(head);
 			return 1;
 		}
 		temp->next = NULL;
@@ -393,8 +450,8 @@ int main(int argc, char *argv[])
 		if (regex_len == 0) {
 			file_context_node_destroy(temp);
 			free(temp);
-
-
+			free(line_buf);
+			line_buf = NULL;
 			continue;
 		}
 
@@ -402,7 +459,9 @@ int main(int argc, char *argv[])
 		if (!temp->path) {
 			file_context_node_destroy(temp);
 			free(temp);
+			free(line_buf);
 			fprintf(stderr, "Error: failure allocating memory.\n");
+			fc_free_file_context_node_list(head);
 			return 1;
 		}
 
@@ -416,22 +475,29 @@ int main(int argc, char *argv[])
 		if (i == line_len) {
 			file_context_node_destroy(temp);
 			free(temp);
+			free(line_buf);
+			line_buf = NULL;
 			continue;
 		}
 
 		/* Parse out the type from the line (if it
-			*  is there). */
+		 *  is there). */
 		if (line_buf[i] == '-') {
 			temp->file_type = (char *)malloc(sizeof(char) * 3);
 			if (!(temp->file_type)) {
+				file_context_node_destroy(temp);
+				free(temp);
+				free(line_buf);
 				fprintf(stderr, "Error: failure allocating memory.\n");
+				fc_free_file_context_node_list(head);
 				return 1;
 			}
 
 			if( i + 2 >= line_len ) {
 				file_context_node_destroy(temp);
 				free(temp);
-
+				free(line_buf);
+				line_buf = NULL;
 				continue;
 			}
 
@@ -448,9 +514,10 @@ int main(int argc, char *argv[])
 			}
 
 			if (i == line_len) {
-
 				file_context_node_destroy(temp);
 				free(temp);
+				free(line_buf);
+				line_buf = NULL;
 				continue;
 			}
 		}
@@ -467,24 +534,26 @@ int main(int argc, char *argv[])
 		if (!temp->context) {
 			file_context_node_destroy(temp);
 			free(temp);
+			free(line_buf);
 			fprintf(stderr, "Error: failure allocating memory.\n");
+			fc_free_file_context_node_list(head);
 			return 1;
 		}
 
 		/* Set all the data about the regular
-			*  expression. */
+		 * expression. */
 		fc_fill_data(temp);
 
 		/* Link this line of code at the end of
-			*  the linked list. */
+		 * the linked list. */
 		current->next = temp;
 		current = current->next;
 		lines++;
 
-
 		free(line_buf);
 		line_buf = NULL;
 	}
+	free(line_buf);
 	fclose(in_file);
 
 	/* Create the bucket linked list from the earlier linked list. */
@@ -492,13 +561,41 @@ int main(int argc, char *argv[])
 	bcurrent = master =
 	    (file_context_bucket_t *)
 	    malloc(sizeof(file_context_bucket_t));
+	if (!bcurrent) {
+		printf
+		    ("Error: failure allocating memory.\n"); 
+		fc_free_file_context_node_list(head);
+		return -1;
+	}
 	bcurrent->next = NULL;
 	bcurrent->data = NULL;
 
 	/* Go until all the nodes have been put in individual buckets. */
 	while (current) {
+		bcurrent->data = (file_context_node_t *)malloc(sizeof(file_context_node_t));
+		if (!(bcurrent->data)) {
+			printf
+			    ("Error: failure allocating memory.\n");
+			fc_free_file_context_node_list(head);
+			fc_free_file_context_bucket_list(master);
+		}
 		/* Copy over the file context line into the bucket. */
-		bcurrent->data = current;
+		if (current->path)
+			bcurrent->data->path = strdup(current->path);
+		else
+			bcurrent->data->path = NULL;
+		if (current->file_type)
+			bcurrent->data->file_type = strdup(current->file_type);
+		else
+			bcurrent->data->file_type = NULL;
+		if (current->context)
+			bcurrent->data->context = strdup(current->context);
+		else
+			bcurrent->data->context = NULL;
+		bcurrent->data->meta = current->meta;
+		bcurrent->data->stem_len = current->stem_len;
+		bcurrent->data->str_len = current->str_len;
+
 		current = current->next;
 
 		/* Detach the node in the bucket from the old list. */
@@ -512,6 +609,8 @@ int main(int argc, char *argv[])
 			if (!(bcurrent->next)) {
 				printf
 				    ("Error: failure allocating memory.\n");
+				fc_free_file_context_node_list(head);
+				fc_free_file_context_bucket_list(master);
 				exit(-1);
 			}
 
@@ -521,7 +620,6 @@ int main(int argc, char *argv[])
 
 			bcurrent = bcurrent->next;
 		}
-
 	}
 
 	/* Sort the bucket list. */
@@ -531,6 +629,7 @@ int main(int argc, char *argv[])
 	if (output_name) {
 		if (!(out_file = fopen(output_name, "w"))) {
 			printf("Error: failure opening output file for write.\n");
+			fc_free_file_context_node_list(head);
 			return -1;
 		}
 	} else {
@@ -539,6 +638,7 @@ int main(int argc, char *argv[])
 
 	/* Output the sorted file_context linked list to the output file. */
 	current = master->data;
+
 	while (current) {
 		/* Output the path. */
 		fprintf(out_file, "%s\t\t", current->path);
@@ -551,14 +651,11 @@ int main(int argc, char *argv[])
 		/* Output the context. */
 		fprintf(out_file, "%s\n", current->context);
 
-		/* Remove the node. */
-		temp = current;
 		current = current->next;
-
-		file_context_node_destroy(temp);
-		free(temp);
-
 	}
+
+	fc_free_file_context_node_list(head);
+	fc_free_file_context_node_list(master->data);
 	free(master);
 
 	if (output_name) {