Commit 0f520fac authored by William Roberts's avatar William Roberts
Browse files

fix memory leaks and uninitialized jump


Some error's were reported by valgrind (below) fix them. The test
cases on which these leaks were detected:

1. properly formed file_contexts file.
2. malformed file_contexts file, unknown type.
3. malformed file_contexts file, type that fails on validate callback.
4. malformed file_contexts file, invalid regex.
5. malformed file_contexts file, invalid mode.

==3819== Conditional jump or move depends on uninitialised value(s)
==3819==    at 0x12A682: closef (label_file.c:577)
==3819==    by 0x12A196: selabel_close (label.c:163)
==3819==    by 0x10A2FD: cleanup (checkfc.c:218)
==3819==    by 0x5089258: __run_exit_handlers (exit.c:82)
==3819==    by 0x50892A4: exit (exit.c:104)
==3819==    by 0x10A231: main (checkfc.c:361)
==3819==  Uninitialised value was created by a heap allocation
==3819==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3819==    by 0x4C2CF1F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3819==    by 0x12BB31: process_file (label_file.h:273)
==3819==    by 0x12A2BA: selabel_file_init (label_file.c:522)
==3819==    by 0x12A0BB: selabel_open (label.c:88)
==3819==    by 0x10A038: main (checkfc.c:292)
==3819==
==3819==
==3819== HEAP SUMMARY:
==3819==     in use at exit: 729 bytes in 19 blocks
==3819==   total heap usage: 21,126 allocs, 21,107 frees, 923,854 bytes allocated
==3819==
==3819== 81 bytes in 1 blocks are definitely lost in loss record 1 of 2
==3819==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3819==    by 0x50D5839: strdup (strdup.c:42)
==3819==    by 0x12A2A6: selabel_file_init (label_file.c:517)
==3819==    by 0x12A0BB: selabel_open (label.c:88)
==3819==    by 0x10A038: main (checkfc.c:292)
==3819==

==4238== 40 bytes in 1 blocks are definitely lost in loss record 1 of 6
==4238==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==    by 0x12A1D2: selabel_file_init (label_file.c:886)
==4238==    by 0x12A0BB: selabel_open (label.c:88)
==4238==    by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 81 bytes in 1 blocks are definitely lost in loss record 2 of 6
==4238==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==    by 0x50D5839: strdup (strdup.c:42)
==4238==    by 0x12A2A6: selabel_file_init (label_file.c:517)
==4238==    by 0x12A0BB: selabel_open (label.c:88)
==4238==    by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 386 bytes in 24 blocks are definitely lost in loss record 3 of 6
==4238==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==    by 0x50D5889: strndup (strndup.c:45)
==4238==    by 0x12CDDF: read_spec_entries (label_support.c:37)
==4238==    by 0x12B72D: process_file (label_file.h:392)
==4238==    by 0x12A2BA: selabel_file_init (label_file.c:522)
==4238==    by 0x12A0BB: selabel_open (label.c:88)
==4238==    by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 648 bytes in 18 blocks are definitely lost in loss record 4 of 6
==4238==    at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==    by 0x117C9B: avtab_insert_node (avtab.c:105)
==4238==    by 0x117C10: avtab_insert (avtab.c:163)
==4238==    by 0x11880A: avtab_read_item (avtab.c:566)
==4238==    by 0x118BD3: avtab_read (avtab.c:600)
==4238==    by 0x125BDD: policydb_read (policydb.c:3854)
==4238==    by 0x109F87: main (checkfc.c:273)
==4238==
==4238== 1,095 bytes in 12 blocks are definitely lost in loss record 5 of 6
==4238==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==    by 0x12D8D1: pcre_compile2 (pcre_compile.c:9217)
==4238==    by 0x12B239: compile_regex (label_file.h:357)
==4238==    by 0x12B9C7: process_file (label_file.h:429)
==4238==    by 0x12A2BA: selabel_file_init (label_file.c:522)
==4238==    by 0x12A0BB: selabel_open (label.c:88)
==4238==    by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 1,296 bytes in 12 blocks are definitely lost in loss record 6 of 6
==4238==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==    by 0x13EBE5: pcre_study (pcre_study.c:1565)
==4238==    by 0x12B25D: compile_regex (label_file.h:366)
==4238==    by 0x12B9C7: process_file (label_file.h:429)
==4238==    by 0x12A2BA: selabel_file_init (label_file.c:522)
==4238==    by 0x12A0BB: selabel_open (label.c:88)
==4238==    by 0x10A038: main (checkfc.c:292)

Change-Id: I2f7ed4ffbdcc3d0646f7caf66187d87347220c60
Signed-off-by: default avatarWilliam Roberts <william.c.roberts@intel.com>
parent 6d5e6edc
......@@ -86,6 +86,7 @@ struct selabel_handle *selabel_open(unsigned int backend,
rec->validating = selabel_is_validate_set(opts, nopts);
if ((*initfuncs[backend])(rec, opts, nopts)) {
free(rec->spec_file);
free(rec);
rec = NULL;
}
......@@ -161,6 +162,7 @@ enum selabel_cmp_result selabel_cmp(struct selabel_handle *h1,
void selabel_close(struct selabel_handle *rec)
{
rec->func_close(rec);
free(rec->spec_file);
free(rec);
}
......
......@@ -492,6 +492,8 @@ out:
return rc;
}
static void closef(struct selabel_handle *rec);
static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
unsigned n)
{
......@@ -543,7 +545,8 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
finish:
if (status)
free(data->spec_arr);
closef(rec);
return status;
}
......
......@@ -278,6 +278,7 @@ static inline int store_stem(struct saved_data *data, char *buf, int stem_len)
}
data->stem_arr[num].len = stem_len;
data->stem_arr[num].buf = buf;
data->stem_arr[num].from_mmap = 0;
data->num_stems++;
return num;
......@@ -425,6 +426,18 @@ static inline int process_line(struct selabel_handle *rec,
/* process and store the specification in spec. */
spec_arr[nspec].stem_id = find_stem_from_spec(data, regex);
spec_arr[nspec].regex_str = regex;
spec_arr[nspec].type_str = type;
spec_arr[nspec].mode = 0;
spec_arr[nspec].lr.ctx_raw = context;
/*
* bump data->nspecs to cause closef() to cover it in its free
* but do not bump nspec since it's used below.
*/
data->nspec++;
if (rec->validating &&
compile_regex(data, &spec_arr[nspec], &errbuf)) {
selinux_log(SELINUX_ERROR,
......@@ -435,9 +448,6 @@ static inline int process_line(struct selabel_handle *rec,
return -1;
}
/* Convert the type string to a mode format */
spec_arr[nspec].type_str = type;
spec_arr[nspec].mode = 0;
if (type) {
mode_t mode = string_to_mode(type);
......@@ -451,8 +461,6 @@ static inline int process_line(struct selabel_handle *rec,
spec_arr[nspec].mode = mode;
}
spec_arr[nspec].lr.ctx_raw = context;
/* Determine if specification has
* any meta characters in the RE */
spec_hasMetaChars(&spec_arr[nspec]);
......@@ -467,8 +475,6 @@ static inline int process_line(struct selabel_handle *rec,
}
}
data->nspec = ++nspec;
return 0;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment