[apparmor] [patch] delete_duplicates(): don't modify self.rules while looping over it

[Christian Boltz]

Hello,

by calling self.delete() inside the delete_duplicates() loop, the
self.rules list was modified. This resulted in some rules not being
checked and therefore (some, not all) superfluous rules not being
removed.

This patch switches to a temporary variable to loop over, and rebuilds
self.rules with the rules that are not superfluous.

This also fixes some strange issues already marked with a "Huh?" comment
in the tests.


I propose this patch for trunk and 2.10.
Note that in 2.10 cleanprof_test.* doesn't contain a ptrace rule,
therefore the cleanprof_test.out change doesn't make sense for 2.10.



[ 01-delete_duplicates-dont-modify-list-in-loop.diff ]

--- utils/apparmor/rule/__init__.py     2016-07-31 19:12:31.537453276 +0200
+++ utils/apparmor/rule/__init__.py     2016-08-07 16:32:19.435814124 +0200
@@ -431,10 +431,13 @@
 
         # delete rules that are covered by include files
         if include_rules:
-            for rule in self.rules:
-                if include_rules.is_covered(rule, True, True):
-                    self.delete(rule)
+            oldrules = self.rules
+            self.rules = []
+            for rule in oldrules:
+                if include_rules.is_covered(rule, True, False):
                     deleted += 1
+                else:
+                    self.rules.append(rule)
 
         # de-duplicate rules inside the profile
         deleted += self.delete_in_profile_duplicates()
--- utils/test/cleanprof_test.out       2016-07-31 19:12:31.517453376 +0200
+++ utils/test/cleanprof_test.out       2016-08-07 16:37:22.402324691 +0200
@@ -16,8 +16,6 @@
 
   signal set=(abrt alrm bus chld fpe hup ill int kill pipe quit segv stkflt term trap usr1 usr2),
 
-  ptrace tracedby,
-
   unix (receive) type=dgram,
 
   allow /home/*/** r,
--- utils/test/test-capability.py       2015-11-19 17:42:26.325879118 +0100
+++ utils/test/test-capability.py       2016-08-07 16:40:33.097385534 +0200
@@ -817,7 +817,6 @@
             inc.add(CapabilityRule.parse(rule))
 
         expected_raw = [
-            '  allow capability sys_admin,',  # XXX huh? should be deleted!
             '  deny capability chgrp, # example comment',
             '',
         ]
@@ -825,11 +824,9 @@
         expected_clean = [
             '  deny capability chgrp, # example comment',
             '',
-            '  allow capability sys_admin,',  # XXX huh? should be deleted!
-            '',
         ]
 
-        self.assertEqual(self.ruleset.delete_duplicates(inc), 1)
+        self.assertEqual(self.ruleset.delete_duplicates(inc), 2)
         self.assertEqual(expected_raw, self.ruleset.get_raw(1))
         self.assertEqual(expected_clean, self.ruleset.get_clean(1))
 

Regards,

Christian Boltz
-- 
Über den Autor Marcus Meissner:
Marcus Meissner entwickelt seit über 10 Jahren Opensource Entwickler.
[gefunden auf http://www.linuxtag.org/2007/de/conf/events/vp-mittwoch/vortragsdetails.html?talkid=40]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160807/07b2e424/attachment.pgp>