[apparmor] Memory leaks in parser/parser_regex.c (at least)

[Steve Beattie]

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
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: parser-fix_memory_leaks_in_unit_tests.patch
Type: text/x-diff
Size: 14626 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150822/f8c71747/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: parser-fix_uninitialized_grouping_value.patch
Type: text/x-diff
Size: 780 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150822/f8c71747/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150822/f8c71747/attachment-0001.pgp>