[apparmor] [utils] [patch] Fix stack trace in aa-mergeprof

Christian Boltz apparmor at cboltz.de
Thu Sep 4 22:02:29 UTC 2014


Hello,

Am Freitag, 5. September 2014 schrieb Kshitij Gupta:
> On Fri, Sep 5, 2014 at 1:27 AM, Christian Boltz wrote:

> > I'm afraid you are a bit late with this patch ;-)

> Sorry I missed that patch. I've been having great troubles with net
> connection.
> 
> I did pull the latest version a few hours before sending this patch,
> maybe I missed it.

No problem, it's better to have two fixes than a bug ;-)

> Yes it seems like a good idea to be consistent.
> 
> But is list(netrules_other['rule'][fam].keys()) very
> intuitive/readable? Its not instantly obvious that a new list object
> is being created which is not affected by the pop operation being
> performed later on.

As long as it doesn't throw a stack trace (like it does without list()), 
the average programmer won't even care if it is the original or a copy ;-)
(and py3 is the only programming language I know that bails out if you
change the array you are using in the for loop. I'm still undecided if
this is a good or a bad idea ;-)

Besides that, list() has the advantage that we get a copy based on the 
latest netrules_other instead of the (possible outdated) copy you create 
at the start of the function.

Also, I'd expect that using list() is faster than copy.deepcopy(), but 
that's just a guess.
 
> Either way a consistent style would be better.

Indeed. Let me start in the function we were talking about:


=== modified file 'utils/apparmor/cleanprofile.py'
--- utils/apparmor/cleanprofile.py      2014-09-04 11:19:39 +0000
+++ utils/apparmor/cleanprofile.py      2014-09-04 21:45:06 +0000
@@ -123,14 +123,13 @@
 def delete_net_duplicates(netrules, netrules_other, same_profile=True):
     deleted = 0
     hasher_obj = apparmor.aa.hasher()
-    copy_netrules_other = copy.deepcopy(netrules_other)
     if netrules_other and netrules:
         netglob = False
         # Delete matching rules
         if netrules.get('all', False):
             netglob = True
         # Iterate over a copy of the rules in the other profile
-        for fam in copy_netrules_other['rule'].keys():
+        for fam in list(netrules_other['rule'].keys()):
             if netglob or (type(netrules['rule'][fam]) != type(hasher_obj) and netrules['rule'][fam]):
                 if not same_profile:
                     if type(netrules_other['rule'][fam]) == type(hasher_obj):




Regards,

Christian Boltz
-- 
Before it can be fixed, technical description of faulty behavior is
needed. Note adjective "technical". "Extremely messed up" usually 
does not qualify as technical description. 
[Andrey Borzenkov in opensuse-factory]




More information about the AppArmor mailing list