Patchwork [06/11] AppArmor: Stop unconfined from inheriting replaced/removed children

login
register
mail settings
Submitter John Johansen
Date April 13, 2010, 7:09 a.m.
Message ID <1271142580-26555-7-git-send-email-john.johansen@canonical.com>
Download mbox | patch
Permalink /patch/50040/
State Accepted
Delegated to: Andy Whitcroft
Headers show

Comments

John Johansen - April 13, 2010, 7:09 a.m.
From: John Johansen <john.johansen@canonical.com>

OriginalAuthor: John Johansen <john.johansen@canonical.com>
OriginalLocation: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparm$
commit: 7c19603812a662c5138bc3a2bfc59db8a9338140
BugLink: http://bugs.launchpad.net/bugs/562052

When profile replacement/removal is done children are inherited by the
replacement profile.  However when there is no replacement specified
replacement by the namespace's unconfined profile is done.

However unconfined should not inherit children as they lose visibility
and can not be removed resulting in a "leak" as those profiles memory
will never get freed.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/policy.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)
John Johansen - April 13, 2010, 9:15 a.m.
On 04/13/2010 12:09 AM, john.johansen@canonical.com wrote:
> From: John Johansen <john.johansen@canonical.com>
> 
> OriginalAuthor: John Johansen <john.johansen@canonical.com>
> OriginalLocation: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparm$
> commit: 7c19603812a662c5138bc3a2bfc59db8a9338140
> BugLink: http://bugs.launchpad.net/bugs/562052
> 
> When profile replacement/removal is done children are inherited by the
> replacement profile.  However when there is no replacement specified
> replacement by the namespace's unconfined profile is done.
> 
> However unconfined should not inherit children as they lose visibility
> and can not be removed resulting in a "leak" as those profiles memory
> will never get freed.
> 
please ignore this patch.  It does address a bug in this function
(being part of the upstream cleanup patches) however the bug can not be
triggered in the Lucid kernel as the calling function
aa_interface_remove_profiles removes the all children with
__aa_profile_list_release before calling this fn.

		__aa_profile_list_release(&profile->base.profiles);
		__aa_replace_profile(profile, NULL);

Patch

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 61f0043..45248be 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -444,19 +444,20 @@  static void __aa_replace_profile(struct aa_profile *old,
 		new->ns = aa_get_namespace(old->ns);
 		new->sid = old->sid;
 		__aa_add_profile(&policy->profiles, new);
+
+		/* inherit children */
+		list_for_each_entry_safe(child, tmp, &old->base.profiles,
+					 base.list) {
+			aa_put_profile(child->parent);
+			child->parent = aa_get_profile(new);
+			/* list refcount transfered to @new*/
+			list_move(&child->base.list, &new->base.profiles);
+		}
 	} else {
 		/* refcount not taken, held via @old refcount */
 		new = old->ns->unconfined;
 	}
 
-	/* inherit children */
-	list_for_each_entry_safe(child, tmp, &old->base.profiles, base.list) {
-		aa_put_profile(child->parent);
-		child->parent = aa_get_profile(new);
-		/* list refcount transfered to @new*/
-		list_move(&child->base.list, &new->base.profiles);
-	}
-
 	/* released by aa_free_profile */
 	old->replacedby = aa_get_profile(new);
 	__aa_remove_profile(old);