On Thu, May 29, 2014 at 08:47:22PM +0200, Christian Boltz wrote: > 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".
Alright, after finally being able to reproduce this (need to use python 3, apparently), I can Acked-by: Steve Beattie <[email protected]> this patch with a couple of formatting nits (no need to resubmit): > === 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): For parameterized arguments like is_stub, the preferred pep8 style is to not have spaces around the assigning equal sign, i.e. 'is_stub=False'. This is pep8 error E251. > 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) And here, pythonic style is have spaces around the '%' variable substitution indicator, e.g. '[...] removing %s" % prof_name)'. This is pep8 error E228. > + 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 ;-) Sure, I'll happily fall on my sword, here. That said, long term, I want the deeply nested hashes/dictionaries to be replaced with actual python classes. -- Steve Beattie <[email protected]> http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
