On Sat, Aug 22, 2015 at 05:19:22PM +0200, intrigeri wrote: > with the attached (very rough, e.g. not sure if any of those > additional build-deps are needed) debdiff applied on top of the Debian > 2.10-1 packaging,
-ENO_DEBDIFF, though I think I get the gist of what you're trying to
accomplish from the context of your post.
> I see test suite failures such as:
>
> ==22749==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 34 byte(s) in 1 object(s) allocated from:
> #0 0x7f8faa42506f in strdup
> (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x6006f)
> #1 0x45dce0 in test_filter_slashes
> /tmp/buildd/apparmor-2.10/parser/parser_regex.c:800
> #2 0x45dce0 in main /tmp/buildd/apparmor-2.10/parser/parser_regex.c:1025
> #3 0x7f8fa8e10b44 in __libc_start_main
> (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
>
> My understanding is that the test suite exits immediately after this
> test case fails, so once this one is fixed, there may be more issues
> that ASAN & friends can detect later on.
These are memory leaks in the unit tests, which we've not fretted over
too much in the past (i.e. they're known; I don't recall which tools
have flagged them before). However, it's not a terrible idea to clean
them up; try the attached parser-fix_memory_leaks_in_unit_tests.patch
patch.
In fact, the act of cleaning these up and running valgrind on the unit
tests picked up another issue, a use-before-initialization bug in the
regex conversion code -- see the second attached patch. I'm curious if
ASAN would notice it.
However, once you get past the unit tests, you will still hit a
couple of memory leaks running the regular parser sanity checks,
in the backend conversion bits[0]. IIRC, I tracked what I could
down to error conditions occurring while walking the various trees
that caused things to jump back out and then attempt to cleanup;
unfortunately, I think things get left in an incosistent state that
causes the cleanup walkthrough to miss a few objects.
[0] The tst/valgrind_simply.py script, which is not part of the build
tests because it can take over 24 hours to run, can also be used to
find memory leaks.
--
Steve Beattie
<[email protected]>
http://NxNW.org/~steve/
Subject: parser: fix memory leaks in unit tests Signed-off-by: Steve Beattie <[email protected]> --- parser/parser_misc.c | 107 ++++++++++++++--------------------------------- parser/parser_regex.c | 66 +++++++++++++--------------- parser/parser_variable.c | 27 ++++++++++- 3 files changed, 87 insertions(+), 113 deletions(-) Index: b/parser/parser_regex.c =================================================================== --- a/parser/parser_regex.c +++ b/parser/parser_regex.c @@ -792,46 +792,40 @@ out: #include "unit_test.h" +#define MY_FILTER_TEST(input, expected_str) \ + do { \ + char *test_string = NULL; \ + char *output_string = NULL; \ + \ + test_string = strdup((input)); \ + filter_slashes(test_string); \ + asprintf(&output_string, "simple filter / conversion for '%s'\texpected = '%s'\tresult = '%s'", \ + (input), (expected_str), test_string); \ + MY_TEST(strcmp(test_string, (expected_str)) == 0, output_string); \ + \ + free(test_string); \ + free(output_string); \ + } \ + while (0) + static int test_filter_slashes(void) { int rc = 0; - char *test_string; - test_string = strdup("///foo//////f//oo////////////////"); - filter_slashes(test_string); - MY_TEST(strcmp(test_string, "/foo/f/oo/") == 0, "simple tests"); - - test_string = strdup("/foo/f/oo"); - filter_slashes(test_string); - MY_TEST(strcmp(test_string, "/foo/f/oo") == 0, "simple test for no changes"); - - test_string = strdup("/"); - filter_slashes(test_string); - MY_TEST(strcmp(test_string, "/") == 0, "simple test for '/'"); - - test_string = strdup(""); - filter_slashes(test_string); - MY_TEST(strcmp(test_string, "") == 0, "simple test for ''"); - - test_string = strdup("//usr"); - filter_slashes(test_string); - MY_TEST(strcmp(test_string, "//usr") == 0, "simple test for // namespace"); - - test_string = strdup("//"); - filter_slashes(test_string); - MY_TEST(strcmp(test_string, "//") == 0, "simple test 2 for // namespace"); - - test_string = strdup("///usr"); - filter_slashes(test_string); - MY_TEST(strcmp(test_string, "/usr") == 0, "simple test for ///usr"); - - test_string = strdup("///"); - filter_slashes(test_string); - MY_TEST(strcmp(test_string, "/") == 0, "simple test for ///"); - - test_string = strdup("/a/"); - filter_slashes(test_string); - MY_TEST(strcmp(test_string, "/a/") == 0, "simple test for /a/"); + MY_FILTER_TEST("///foo//////f//oo////////////////", "/foo/f/oo/"); + MY_FILTER_TEST("/foo/f/oo", "/foo/f/oo"); + MY_FILTER_TEST("/", "/"); + MY_FILTER_TEST("", ""); + + /* tests for "//" namespace */ + MY_FILTER_TEST("//usr", "//usr"); + MY_FILTER_TEST("//", "//"); + + /* tests for not "//" namespace */ + MY_FILTER_TEST("///usr", "/usr"); + MY_FILTER_TEST("///", "/"); + + MY_FILTER_TEST("/a/", "/a/"); return rc; } Index: b/parser/parser_misc.c =================================================================== --- a/parser/parser_misc.c +++ b/parser/parser_misc.c @@ -939,85 +939,40 @@ int test_str_to_boolean(void) return rc; } +#define MY_TEST_UNQUOTED(input, expected, description) \ + do { \ + char *result_str = NULL; \ + char *output_str = NULL; \ + \ + result_str = processunquoted((input), strlen((input))); \ + asprintf(&output_str, "processunquoted: %s\tinput = '%s'\texpected = '%s'\tresult = '%s'", \ + (description), (input), (expected), result_str); \ + MY_TEST(strcmp((expected), result_str) == 0, output_str); \ + \ + free(output_str); \ + free(result_str); \ + } \ + while(0) + int test_processunquoted(void) { int rc = 0; - const char *teststring; - const char *resultstring; - - teststring = ""; - MY_TEST(strcmp(teststring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on empty string"); - teststring = "\\1"; - resultstring = "\001"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on one digit octal"); - - teststring = "\\8"; - resultstring = "\\8"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on invalid octal digit \\8"); - - teststring = "\\18"; - resultstring = "\0018"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on one digit octal followed by invalid octal digit"); - - teststring = "\\1a"; - resultstring = "\001a"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on one digit octal followed by hex digit a"); - - teststring = "\\1z"; - resultstring = "\001z"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on one digit octal follow by char z"); - - teststring = "\\11"; - resultstring = "\011"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on two digit octal"); - - teststring = "\\118"; - resultstring = "\0118"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on two digit octal followed by invalid octal digit"); - - teststring = "\\11a"; - resultstring = "\011a"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on two digit octal followed by hex digit a"); - - teststring = "\\11z"; - resultstring = "\011z"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on two digit octal followed by char z"); - - teststring = "\\111"; - resultstring = "\111"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on three digit octal"); - - teststring = "\\378"; - resultstring = "\0378"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on three digit octal two large, taken as 2 digit octal plus trailing char"); - - teststring = "123\\421123"; - resultstring = "123\0421123"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on two character octal followed by valid octal digit \\421"); - - teststring = "123\\109123"; - resultstring = "123\109123"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on octal 109"); - - teststring = "123\\1089123"; - resultstring = "123\1089123"; - MY_TEST(strcmp(resultstring, processunquoted(teststring, strlen(teststring))) == 0, - "processunquoted on octal 108"); + MY_TEST_UNQUOTED("", "", "empty string"); + MY_TEST_UNQUOTED("\\1", "\001", "one digit octal"); + MY_TEST_UNQUOTED("\\8", "\\8", "invalid octal digit \\8"); + MY_TEST_UNQUOTED("\\18", "\0018", "one digit octal followed by invalid octal digit"); + MY_TEST_UNQUOTED("\\1a", "\001a", "one digit octal followed by hex digit a"); + MY_TEST_UNQUOTED("\\1z", "\001z", "one digit octal follow by char z"); + MY_TEST_UNQUOTED("\\11", "\011", "two digit octal"); + MY_TEST_UNQUOTED("\\118", "\0118", "two digit octal followed by invalid octal digit"); + MY_TEST_UNQUOTED("\\11a", "\011a", "two digit octal followed by hex digit a"); + MY_TEST_UNQUOTED("\\11z", "\011z", "two digit octal followed by char z"); + MY_TEST_UNQUOTED("\\111", "\111", "three digit octal"); + MY_TEST_UNQUOTED("\\378", "\0378", "three digit octal two large, taken as 2 digit octal plus trailing char"); + MY_TEST_UNQUOTED("123\\421123", "123\0421123", "two character octal followed by valid octal digit \\421"); + MY_TEST_UNQUOTED("123\\109123", "123\109123", "octal 109"); + MY_TEST_UNQUOTED("123\\1089123", "123\1089123", "octal 108"); return rc; } @@ -1116,12 +1071,14 @@ int test_processquoted(void) out = processquoted(teststring, strlen(teststring)); MY_TEST(strcmp(processedstring, out) == 0, "processquoted passthrough quoted one digit trailing octal \\4"); + free(out); teststring = "\"abcdefg\\04\""; processedstring = "abcdefg\004"; out = processquoted(teststring, strlen(teststring)); MY_TEST(strcmp(processedstring, out) == 0, "processquoted passthrough quoted two digit trailing octal \\04"); + free(out); teststring = "\"abcdefg\\004\""; processedstring = "abcdefg\004"; Index: b/parser/parser_variable.c =================================================================== --- a/parser/parser_variable.c +++ b/parser/parser_variable.c @@ -384,6 +384,7 @@ int test_split_string(void) MY_TEST(strcmp(ret_struct->var, var) == 0, "split string 1 var"); MY_TEST(strcmp(ret_struct->suffix, suffix) == 0, "split string 1 suffix"); free_var_string(ret_struct); + free(tst_string); asprintf(&tst_string, "@{%s}%s", var, suffix); var_start = tst_string; @@ -393,6 +394,7 @@ int test_split_string(void) MY_TEST(strcmp(ret_struct->var, var) == 0, "split string 2 var"); MY_TEST(strcmp(ret_struct->suffix, suffix) == 0, "split string 2 suffix"); free_var_string(ret_struct); + free(tst_string); asprintf(&tst_string, "%s@{%s}", prefix, var); var_start = tst_string + strlen(prefix); @@ -402,6 +404,7 @@ int test_split_string(void) MY_TEST(strcmp(ret_struct->var, var) == 0, "split string 3 var"); MY_TEST(ret_struct->suffix == NULL, "split string 3 suffix"); free_var_string(ret_struct); + free(tst_string); asprintf(&tst_string, "@{%s}", var); var_start = tst_string; @@ -411,6 +414,7 @@ int test_split_string(void) MY_TEST(strcmp(ret_struct->var, var) == 0, "split string 4 var"); MY_TEST(ret_struct->suffix == NULL, "split string 4 suffix"); free_var_string(ret_struct); + free(tst_string); return rc; } @@ -433,6 +437,7 @@ int test_split_out_var(void) MY_TEST(strcmp(ret_struct->var, var) == 0, "split out var 1 var"); MY_TEST(strcmp(ret_struct->suffix, suffix) == 0, "split out var 1 suffix"); free_var_string(ret_struct); + free(tst_string); /* no prefix */ asprintf(&tst_string, "@{%s}%s", var, suffix); @@ -441,6 +446,7 @@ int test_split_out_var(void) MY_TEST(strcmp(ret_struct->var, var) == 0, "split out var 2 var"); MY_TEST(strcmp(ret_struct->suffix, suffix) == 0, "split out var 2 suffix"); free_var_string(ret_struct); + free(tst_string); /* no suffix */ asprintf(&tst_string, "%s@{%s}", prefix, var); @@ -449,6 +455,7 @@ int test_split_out_var(void) MY_TEST(strcmp(ret_struct->var, var) == 0, "split out var 3 var"); MY_TEST(ret_struct->suffix == NULL, "split out var 3 suffix"); free_var_string(ret_struct); + free(tst_string); /* var only */ asprintf(&tst_string, "@{%s}", var); @@ -457,32 +464,39 @@ int test_split_out_var(void) MY_TEST(strcmp(ret_struct->var, var) == 0, "split out var 4 var"); MY_TEST(ret_struct->suffix == NULL, "split out var 4 suffix"); free_var_string(ret_struct); + free(tst_string); /* quoted var, shouldn't split */ asprintf(&tst_string, "%s\\@{%s}%s", prefix, var, suffix); ret_struct = split_out_var(tst_string); MY_TEST(ret_struct == NULL, "split out var - quoted @"); free_var_string(ret_struct); + free(tst_string); /* quoted \, split should succeed */ asprintf(&tst_string, "%s\\\\@{%s}%s", prefix, var, suffix); ret_struct = split_out_var(tst_string); - MY_TEST(strcmp(ret_struct->prefix, strndup(tst_string, strlen(prefix) + 2)) == 0, "split out var 5 prefix"); + tmp = strndup(tst_string, strlen(prefix) + 2); + MY_TEST(strcmp(ret_struct->prefix, tmp) == 0, "split out var 5 prefix"); MY_TEST(strcmp(ret_struct->var, var) == 0, "split out var 5 var"); MY_TEST(strcmp(ret_struct->suffix, suffix) == 0, "split out var 5 suffix"); free_var_string(ret_struct); + free(tst_string); + free(tmp); /* un terminated var, should fail */ asprintf(&tst_string, "%s@{%s%s", prefix, var, suffix); ret_struct = split_out_var(tst_string); MY_TEST(ret_struct == NULL, "split out var - un-terminated var"); free_var_string(ret_struct); + free(tst_string); /* invalid char in var, should fail */ asprintf(&tst_string, "%s@{%s^%s}%s", prefix, var, var, suffix); ret_struct = split_out_var(tst_string); MY_TEST(ret_struct == NULL, "split out var - invalid char in var"); free_var_string(ret_struct); + free(tst_string); /* two vars, should only strip out first */ asprintf(&tmp, "@{%s}%s}", suffix, suffix); @@ -492,14 +506,19 @@ int test_split_out_var(void) MY_TEST(strcmp(ret_struct->var, var) == 0, "split out var 6 var"); MY_TEST(strcmp(ret_struct->suffix, tmp) == 0, "split out var 6 suffix"); free_var_string(ret_struct); + free(tst_string); + free(tmp); /* quoted @ followed by var, split should succeed */ asprintf(&tst_string, "%s\\@@{%s}%s", prefix, var, suffix); ret_struct = split_out_var(tst_string); - MY_TEST(strcmp(ret_struct->prefix, strndup(tst_string, strlen(prefix) + 2)) == 0, "split out var 7 prefix"); + tmp = strndup(tst_string, strlen(prefix) + 2); + MY_TEST(strcmp(ret_struct->prefix, tmp) == 0, "split out var 7 prefix"); MY_TEST(strcmp(ret_struct->var, var) == 0, "split out var 7 var"); MY_TEST(strcmp(ret_struct->suffix, suffix) == 0, "split out var 7 suffix"); free_var_string(ret_struct); + free(tst_string); + free(tmp); /* numeric char in var, should succeed */ asprintf(&tst_string, "%s@{%s}%s", prefix, var2, suffix); @@ -508,12 +527,14 @@ int test_split_out_var(void) MY_TEST(ret_struct && strcmp(ret_struct->var, var2) == 0, "split numeric var var"); MY_TEST(ret_struct && strcmp(ret_struct->suffix, suffix) == 0, "split out numeric var suffix"); free_var_string(ret_struct); + free(tst_string); /* numeric first char in var, should fail */ asprintf(&tst_string, "%s@{6%s}%s", prefix, var2, suffix); ret_struct = split_out_var(tst_string); MY_TEST(ret_struct == NULL, "split out var - numeric first char in var"); free_var_string(ret_struct); + free(tst_string); /* underscore char in var, should succeed */ asprintf(&tst_string, "%s@{%s}%s", prefix, var3, suffix); @@ -522,12 +543,14 @@ int test_split_out_var(void) MY_TEST(ret_struct && strcmp(ret_struct->var, var3) == 0, "split out underscore var"); MY_TEST(ret_struct && strcmp(ret_struct->suffix, suffix) == 0, "split out underscore var suffix"); free_var_string(ret_struct); + free(tst_string); /* underscore first char in var, should fail */ asprintf(&tst_string, "%s@{_%s%s}%s", prefix, var2, var3, suffix); ret_struct = split_out_var(tst_string); MY_TEST(ret_struct == NULL, "split out var - underscore first char in var"); free_var_string(ret_struct); + free(tst_string); return rc; }
Subject: parser: fix uninitialized field in convert_aaregex_to_pcre() The first entry in the grouping_count array is never initialized to 0; subsequent depths are. This patch fixes the issue. Signed-off-by: Steve Beattie <[email protected]> --- parser/parser_regex.c | 1 + 1 file changed, 1 insertion(+) Index: b/parser/parser_regex.c =================================================================== --- a/parser/parser_regex.c +++ b/parser/parser_regex.c @@ -122,6 +122,7 @@ pattern_t convert_aaregex_to_pcre(const int ingrouping = 0; /* flag to indicate {} context */ int incharclass = 0; /* flag to indicate [ ] context */ int grouping_count[MAX_ALT_DEPTH]; + grouping_count[ingrouping] = 0; error = e_no_error; ptype = ePatternBasic; /* assume no regex */
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
