Using the new hasher() from my previous patch, I found some problems.

One fix is here:

-                            if aa[profile][hat][incname]:
+                            if aa[profile][hat]['include'].get(incname,
False):
                                 continue

I believe the old line would have created a new hasher (defaultdict) in
the wrong place. The dict where it should have been needs to be fetched
with the key 'include'. So this created lots of garbage in the aa[...]
dict, plus would always return False, so the "continue" which was likely
a performance optimization, was never called.

And another here:
-                                    if
aa[profile][hat]['allow']['path'][path].get('mode', False):
-                                        mode |=
aa[profile][hat]['allow']['path'][path]['mode']
+                                    if path in
aa[profile][hat]['allow']['path']:
+                                        if
aa[profile][hat]['allow']['path'][path].get('mode', False):
+                                            mode |=
aa[profile][hat]['allow']['path'][path]['mode']

And here:
-        if include[incfile][incfile][allow]['path'][path]:
+        if path in include[incfile][incfile][allow]['path']:

This one just prevents creating extra paths while looking them up. I
believe this had a significant impact on performance, because it was
going through more paths than it had to, to match against regexes, to
find the modes, to " |= " them to a list. But the resulting dict was
empty, so it would just add nothing and waste time.

The first fix uses get(...,"False") because we want the value, which is
either True or False. The others return strings or dicts. I don't
understand why get('mode'), False) should be done instead of "if 'mode'
in ..." but I left it the way it was since I think it works.



And FYI the performance bug I was looking for makes it take way longer
than 1 hour to process some interactive commands in aa-logprof, like
when you say "(I)nherit" for an exec, on a very large audit.log, >50 MB.
(for example firefox will do this reliably). I'm not completely sure
this patch fixes it... I'm still working on it. I'm going to test the
patch on firefox again today on my desktop since firefox won't run in my
apparmor dev vm due to stupid bugs.
Binary files p2/__pycache__/aa.cpython-34.pyc and p3/__pycache__/aa.cpython-34.pyc differ
Binary files p2/__pycache__/common.cpython-34.pyc and p3/__pycache__/common.cpython-34.pyc differ
diff -ur p2/aa.py p3/aa.py
--- p2/aa.py	2014-11-24 23:01:33.955379718 +0100
+++ p3/aa.py	2014-11-24 23:02:25.481005119 +0100
@@ -1709,7 +1709,7 @@
                         for incname in include.keys():
                             include_valid = False
                             # If already present skip
-                            if aa[profile][hat][incname]:
+                            if aa[profile][hat]['include'].get(incname, False):
                                 continue
                             if incname.startswith(profile_dir):
                                 incname = incname.replace(profile_dir + '/', '', 1)
@@ -1856,8 +1856,9 @@
                                         aaui.UI_Info(_('Deleted %s previous matching profile entries.') % deleted)
 
                                 else:
-                                    if aa[profile][hat]['allow']['path'][path].get('mode', False):
-                                        mode |= aa[profile][hat]['allow']['path'][path]['mode']
+                                    if path in aa[profile][hat]['allow']['path']:
+                                        if aa[profile][hat]['allow']['path'][path].get('mode', False):
+                                            mode |= aa[profile][hat]['allow']['path'][path]['mode']
                                     deleted = []
                                     for entry in aa[profile][hat]['allow']['path'].keys():
                                         if path == entry:
@@ -4550,7 +4551,7 @@
             combinedaudit |= am
             matches += m
 
-        if include[incfile][incfile][allow]['path'][path]:
+        if path in include[incfile][incfile][allow]['path']:
             combinedmode |= include[incfile][incfile][allow]['path'][path]['mode']
             combinedaudit |= include[incfile][incfile][allow]['path'][path]['audit']
 
@@ -4716,7 +4717,7 @@
 
 filename = conf.find_first_file(cfg['settings']['logfiles']) or '/var/log/syslog'
 if not os.path.isfile(filename):
-    raise AppArmorException('Can\'t find system log.')
+    raise AppArmorException("Can't find system log \"%s\"." % (filename))
 
 ldd = conf.find_first_file(cfg['settings']['ldd']) or '/usr/bin/ldd'
 if not os.path.isfile(ldd) or not os.access(ldd, os.EX_OK):
-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to