From: guido@trentalancia.com (Guido Trentalancia) Date: Fri, 29 Sep 2017 20:35:45 +0200 Subject: [refpolicy] [PATCH v2] fc_sort: memory leakages In-Reply-To: <1506703966.24492.1.camel@trentalancia.com> References: <1506643940.32317.6.camel@trentalancia.com> <1506703966.24492.1.camel@trentalancia.com> Message-ID: <1506710145.25223.1.camel@trentalancia.com> To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com Might avoid memory leakages in the fc_sort executable in particular it should take more care in error conditions. Some NULL pointer checks with or without associated error reporting. Some white space and comment formatting fixes. The previous version of this patch is invalid as it causes a failure in getline() calls and hence produces empty output files. Signed-off-by: Guido Trentalancia --- support/fc_sort.c | 85 +++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 13 deletions(-) --- a/support/fc_sort.c 2017-09-29 19:01:28.012455758 +0200 +++ b/support/fc_sort.c 2017-09-29 20:26:39.438434861 +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,25 @@ 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; + } +} + + + /* main * This program takes in two arguments, the input filename and the * output filename. The input file should be syntactically correct. @@ -328,7 +350,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 []\n",argv[0]); @@ -348,15 +369,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 +396,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 +434,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 +443,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 +459,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 +498,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,21 +518,22 @@ 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; } @@ -492,6 +544,12 @@ 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; @@ -512,6 +570,7 @@ int main(int argc, char *argv[]) if (!(bcurrent->next)) { printf ("Error: failure allocating memory.\n"); + fc_free_file_context_node_list(head); exit(-1); } @@ -531,6 +590,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 { @@ -557,7 +617,6 @@ int main(int argc, char *argv[]) file_context_node_destroy(temp); free(temp); - } free(master);