[apparmor] [patch] parser - add valgrind test script (was fix dbus peer_conds memory leak)
Steve Beattie
steve at nxnw.org
Wed Sep 11 14:33:59 UTC 2013
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.
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/6e6a8216/attachment.pgp>
More information about the AppArmor
mailing list