[apparmor] [patch] parser - add valgrind test script (was fix dbus peer_conds memory leak)
Tyler Hicks
tyhicks at canonical.com
Wed Sep 11 18:22:09 UTC 2013
On 2013-09-11 07:33:59, Steve Beattie wrote:
> On Wed, Sep 11, 2013 at 01:57:13AM -0700, Tyler Hicks wrote:
> > On 2013-09-09 11:29:28, Steve Beattie wrote:
> > > It's also finding what may be some false positives around
> > > invalid reads around profile serialization (e.g. on
> > > simple_tests/conditional/else_if_5.sd) and conditional
> > > jump or move depends on uninitialized value(s) (e.g.
> > > simple_tests/network/network_ok_1.sd) that need more investigation,
> > > to determine whether it's a legitimate issue or a valgrind check that
> > > hopefully can be disabled.
> >
> > The memory leak fixes get my
> >
> > Acked-by: Tyler Hicks <tyhicks at canonical.com>
> >
> > I rebased the C++ patches on that patch, so please go ahead and commit
> > it when you get a chance.
>
> Committed.
>
> > I need some more time to look at the valgrind test patch. It doesn't
> > like the C++ conversion and ever test was failing before I killed it.
> > I'm not immediately sure what is happening. Running the parser through
> > valgrind on the same testcases shows no issue. I'll have to look into
> > more.
>
> Try the following version. It seems gcc emits code for some string loops
> that reads past the end of the allocated area, even though the code
> check guarantees that things won't go out of bounds, but valgrind can't
> tell the difference. I added some suppressions to try to avoid that, but
> it's awfully brittle. I may just convert it to suppressing any of the 4
> byte read errors.
Interesting. The suppressions allow many of the tests to pass with the
C++ patches applied. There are still some new failures caused by the C++
patches. This new valgrind target will be helpful in fixing those
issues.
Acked-by: Tyler Hicks <tyhicks at canonical.com>
Tyler
>
> Subject: parser - add simple valgrind wrapper tests
>
> This patch adds a test wrapper that runs valgrind on the parser over the
> simple_tests tree (or other directory tree if passed on the command
> line). An alternate parser location can also be passed on the command
> line.
>
> Like the libapparmor python bindings test, this test uses a bit of magic
> to generate tests that doesn't work with auto-detecting test utilities
> like nose.
>
> Running valgrind on the parser over all 69000+ testcases takes several
> hours, so while this patch includes a make target 'make valgrind', it
> does not add it to the set of tests run when 'make check' is called.
> Perhaps a 'make extra-tests' target is in order.
>
> Patch history:
> v1: initial version
> v2: add valgrind suppressions for overaggressive 4 byte reads past the
> end of allocated storage.
>
> Signed-off-by: Steve Beattie <steve at nxnw.org>
> ---
> parser/tst/Makefile | 3
> parser/tst/valgrind_simple.py | 197 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 200 insertions(+)
>
> Index: b/parser/tst/valgrind_simple.py
> ===================================================================
> --- /dev/null
> +++ b/parser/tst/valgrind_simple.py
> @@ -0,0 +1,197 @@
> +#!/usr/bin/env python
> +# ------------------------------------------------------------------
> +#
> +# Copyright (C) 2013 Canonical Ltd.
> +# Author: Steve Beattie <steve at nxnw.org>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of version 2 of the GNU General Public
> +# License published by the Free Software Foundation.
> +#
> +# ------------------------------------------------------------------
> +
> +from optparse import OptionParser # deprecated, should move to argparse eventually
> +import os
> +import signal
> +import subprocess
> +import tempfile
> +import unittest
> +
> +DEFAULT_TESTDIR = "./simple_tests/vars"
> +DEFAULT_PARSER = '../apparmor_parser'
> +VALGRIND_ERROR_CODE = 151
> +TIMEOUT_ERROR_CODE = 152
> +VALGRIND_ARGS = ['--leak-check=full', '--error-exitcode=%d' % (VALGRIND_ERROR_CODE)]
> +
> +VALGRIND_SUPPRESSIONS = '''
> +{
> + valgrind-add_search_dir-obsessive-overreads
> + Memcheck:Addr4
> + fun:_Z*add_search_dir*
> + fun:_Z*process_arg*
> + fun:main
> +}
> +
> +{
> + valgrind-yylex-obsessive-overreads
> + Memcheck:Addr4
> + fun:_Z?yylex?
> + fun:_Z*yyparse*
> + fun:_Z*process_profile*
> + fun:main
> +}
> +
> +{
> + valgrind-serialize_profile-obsessive-overreads
> + Memcheck:Addr4
> + fun:_Z*sd_serialize_profile*
> + fun:_Z*sd_serialize_codomain*
> + fun:_Z*load_codomain*
> + fun:_Z*__load_flattened_hat*
> + ...
> + fun:twalk
> + fun:_Z*load_flattened_hats*
> + fun:_Z*sd_serialize_codomain*
> + fun:_Z*load_codomain*
> + fun:_Z*__load_policy*
> +}'''
> +
> +# http://www.chiark.greenend.org.uk/ucgi/~cjwatson/blosxom/2009-07-02-python-sigpipe.html
> +# This is needed so that the subprocesses that produce endless output
> +# actually quit when the reader goes away.
> +def subprocess_setup():
> + # Python installs a SIGPIPE handler by default. This is usually not
> + # what non-Python subprocesses expect.
> + signal.signal(signal.SIGPIPE, signal.SIG_DFL)
> +
> +
> +def run_cmd(command, input=None, stderr=subprocess.STDOUT, stdout=subprocess.PIPE, stdin=None, timeout=120):
> + '''Try to execute given command (array) and return its stdout, or
> + return a textual error if it failed.'''
> +
> + try:
> + sp = subprocess.Popen(command, stdin=stdin, stdout=stdout, stderr=stderr, close_fds=True, preexec_fn=subprocess_setup)
> + except OSError as e:
> + return [127, str(e)]
> +
> + timeout_communicate = TimeoutFunction(sp.communicate, timeout)
> + out, outerr = (None, None)
> + try:
> + out, outerr = timeout_communicate(input)
> + rc = sp.returncode
> + except TimeoutFunctionException as e:
> + sp.terminate()
> + outerr = b'test timed out, killed'
> + rc = TIMEOUT_ERROR_CODE
> +
> + # Handle redirection of stdout
> + if out is None:
> + out = b''
> + # Handle redirection of stderr
> + if outerr is None:
> + outerr = b''
> + return [rc, out.decode('utf-8') + outerr.decode('utf-8')]
> +
> +
> +# Timeout handler using alarm() from John P. Speno's Pythonic Avocado
> +class TimeoutFunctionException(Exception):
> + """Exception to raise on a timeout"""
> + pass
> +
> +
> +class TimeoutFunction:
> + def __init__(self, function, timeout):
> + self.timeout = timeout
> + self.function = function
> +
> + def handle_timeout(self, signum, frame):
> + raise TimeoutFunctionException()
> +
> + def __call__(self, *args, **kwargs):
> + old = signal.signal(signal.SIGALRM, self.handle_timeout)
> + signal.alarm(self.timeout)
> + try:
> + result = self.function(*args, **kwargs)
> + finally:
> + signal.signal(signal.SIGALRM, old)
> + signal.alarm(0)
> + return result
> +
> +
> +class AAParserValgrindTests(unittest.TestCase):
> + def setUp(self):
> + # REPORT ALL THE OUTPUT
> + self.maxDiff = None
> +
> + def _runtest(self, testname, config):
> + parser_args = ['-Q', '-I', config.testdir]
> + failure_rc = [VALGRIND_ERROR_CODE, TIMEOUT_ERROR_CODE]
> + command = ['valgrind']
> + command.extend(VALGRIND_ARGS)
> + command.append(config.parser)
> + command.extend(parser_args)
> + command.append(testname)
> + rc, output = run_cmd(command, timeout=120)
> + self.assertNotIn(rc, failure_rc,
> + "valgrind returned error code %d, gave the following output\n%s" % (rc, output))
> +
> +
> +def find_testcases(testdir):
> + '''dig testcases out of passed directory'''
> +
> + for (fdir, direntries, files) in os.walk(testdir):
> + for f in files:
> + if f.endswith(".sd"):
> + yield os.path.join(fdir, f)
> +
> +
> +def create_suppressions():
> + '''generate valgrind suppressions file'''
> +
> + handle, name = tempfile.mkstemp(suffix='.suppressions', prefix='aa-parser-valgrind')
> + os.close(handle)
> + handle = file(name,"w+")
> + handle.write(VALGRIND_SUPPRESSIONS)
> + handle.close()
> + return name
> +
> +def main():
> + usage = "usage: %prog [options] [test_directory]"
> + p = OptionParser(usage=usage)
> + p.add_option('-p', '--parser', default=DEFAULT_PARSER, action="store", type="string", dest='parser')
> + p.add_option('-v', '--verbose', action="store_true", dest="verbose")
> + p.add_option('-s', '--skip-suppressions', action="store_true", dest="skip_suppressions")
> + config, args = p.parse_args()
> +
> + verbosity = 1
> + if config.verbose:
> + verbosity = 2
> +
> + if len(args) == 1:
> + config.testdir = args[0]
> + else:
> + config.testdir = DEFAULT_TESTDIR
> +
> + if not config.skip_suppressions:
> + suppression_file = create_suppressions()
> + VALGRIND_ARGS.append('--suppressions=%s' % (suppression_file))
> +
> + for f in find_testcases(config.testdir):
> + def stub_test(self, testname=f):
> + self._runtest(testname, config)
> + stub_test.__doc__ = "test %s" % (f)
> + setattr(AAParserValgrindTests, 'test_%s' % (f), stub_test)
> + test_suite = unittest.TestSuite()
> + test_suite.addTest(unittest.TestLoader().loadTestsFromTestCase(AAParserValgrindTests))
> + rc = 0
> + try:
> + rc = unittest.TextTestRunner(verbosity=verbosity).run(test_suite)
> + except:
> + rc = 1
> + finally:
> + os.remove(suppression_file)
> +
> + return rc
> +
> +if __name__ == "__main__":
> + main()
> Index: b/parser/tst/Makefile
> ===================================================================
> --- a/parser/tst/Makefile
> +++ b/parser/tst/Makefile
> @@ -50,6 +50,9 @@ minimize: $(PARSER)
> equality: $(PARSER)
> LANG=C APPARMOR_PARSER="$(PARSER)" ./equality.sh
>
> +valgrind: $(PARSER)
> + LANG=C ./valgrind_simple.py -p "$(PARSER)" -v simple_tests
> +
> $(PARSER):
> make -C $(PARSER_DIR) $(PARSER_BIN)
>
> --
> Steve Beattie
> <sbeattie at ubuntu.com>
> http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130911/af9bb4ea/attachment-0001.pgp>
More information about the AppArmor
mailing list