[apparmor] [patch] fix save_profile() by fixing some other code

[Christian Boltz]

Hello,

when creating a child profile while using genprof, I get a backtrace:

Traceback (most recent call last):
  File "aa-genprof", line 160, in <module>
    lp_ret = apparmor.do_logprof_pass(logmark, passno)
  File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2291, in do_logprof_pass
    save_profiles()
  File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2309, in save_profiles
    for prof_name in changed.keys():
RuntimeError: dictionary changed size during iteration

(See https://bugs.launchpad.net/apparmor/+bug/1014304 for more details.)


After digging into the code, it seems for some reason the child profile 
is added to "changed" - I doubt this is correct (guess why it's removed 
later... ;-)

After digging a bit more, I found out that create_new_profile() is 
(ab)used to create a new stub profile to be used as child profile. 
create_new_profile then adds the new child (which looks like a normal 
profile to it) to "changed".

This patch most probably makes the cleanup round in save_profile() 
superfluous by adding a is_stub parameter to create_new_profile(). If
this parameter is set, the new (child) profile is not added to "created" 
and "changed".


=== modified file 'utils/apparmor/aa.py'
--- utils/apparmor/aa.py        2014-05-29 14:53:02 +0000
+++ utils/apparmor/aa.py        2014-05-29 18:22:37 +0000
@@ -386,7 +386,7 @@
         return {local_profile: extras[local_profile]}
     return dict()
 
-def create_new_profile(localfile):
+def create_new_profile(localfile, is_stub = False):
     local_profile = hasher()
     local_profile[localfile]['flags'] = 'complain'
     local_profile[localfile]['include']['abstractions/base'] = 1
@@ -428,8 +428,9 @@
             for hat in sorted(cfg['required_hats'][hatglob].split()):
                 local_profile[hat]['flags'] = 'complain'
 
-    created.append(localfile)
-    changed[localfile] = True
+    if not is_stub:
+        created.append(localfile)
+        changed[localfile] = True
 
     debug_logger.debug("Profile for %s:\n\t%s" % (localfile, local_profile.__str__()))
     return {localfile: local_profile}
@@ -1411,7 +1421,7 @@
                                 if profile != hat:
                                     aa[profile][hat]['flags'] = aa[profile][profile]['flags']
 
-                                stub_profile = create_new_profile(hat)
+                                stub_profile = create_new_profile(hat, True)
 
                                 aa[profile][hat]['flags'] = 'complain'
 
@@ -2299,6 +2309,8 @@
     # Ensure the changed profiles are actual active profiles
     for prof_name in changed.keys():
         if not is_active_profile(prof_name):
+            print("*** save_profiles(): removing %s"%prof_name)
+            print('*** This should not happen. Please open a bugreport!')
             changed.pop(prof_name)
 
     changed_list = sorted(changed.keys())


BTW: I intentionally added the two print() lines in safe_profile because
a) I think they will never be displayed
b) I want to know if a is wrong ;-)
c) it's always nice to have a "nice" error message before displaying
   a backtrace ;-)

A final note: I hereby officially blame Steve for introducing this bug 
in AppArmor.pm r2019 without checking the side effects of this function.
(Kshitij is innocent because he just translated this well-hidden bug to
python ;-)


Regards,

Christian Boltz
-- 
Möglicherweise laufe ich sogar mit fliegenden Fahnen von Gnome zu KDE
über. Jedenfalls, bis sich das Gnome-Projekt dazu entschliesst, Nautilus
durch /irgendwas/ zu ersetzen. Notfalls eine Parkuhr oder einen
Bratenwender. Aber nicht dieses.... "Ding". [Ratti in suse-linux]