[apparmor] [PATCH 12/15] apparmor: do not reuse the sid for replaced profiles

[John Johansen]

Up to this point the sid has remained unused by apparmor except for the
stub code that was put in place planning for its eventual use. The stub
code reused a previously allocated sid during profile replacement. This
behavior is not desired so remove it in preparation for the actual use
of sids.

Signed-off-by: John Johansen <john.johansen at canonical.com>
---
 security/apparmor/include/policy.h |    2 +-
 security/apparmor/include/sid.h    |    4 +++-
 security/apparmor/policy.c         |   23 +++++++++--------------
 security/apparmor/policy_unpack.c  |    2 +-
 4 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index 95979c4..5798135 100644
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -246,7 +246,7 @@ static inline void aa_put_namespace(struct aa_namespace *ns)
 		kref_put(&ns->base.count, aa_free_namespace_kref);
 }
 
-struct aa_profile *aa_alloc_profile(const char *name);
+struct aa_profile *aa_alloc_profile(const char *name, u32 sid);
 struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat);
 void aa_free_profile_kref(struct kref *kref);
 struct aa_profile *aa_find_child(struct aa_profile *parent, const char *name);
diff --git a/security/apparmor/include/sid.h b/security/apparmor/include/sid.h
index 020db35..513ca0e 100644
--- a/security/apparmor/include/sid.h
+++ b/security/apparmor/include/sid.h
@@ -16,7 +16,9 @@
 
 #include <linux/types.h>
 
-struct aa_profile;
+/* sid value that will not be allocated */
+#define AA_SID_INVALID 0
+#define AA_SID_ALLOC AA_SID_INVALID
 
 u32 aa_alloc_sid(void);
 void aa_free_sid(u32 sid);
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index aa4f64f..1fc2cff 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -288,11 +288,10 @@ static struct aa_namespace *alloc_namespace(const char *prefix,
 	rwlock_init(&ns->lock);
 
 	/* released by free_namespace */
-	ns->unconfined = aa_alloc_profile("unconfined");
+	ns->unconfined = aa_alloc_profile("unconfined", AA_SID_ALLOC);
 	if (!ns->unconfined)
 		goto fail_unconfined;
 
-	ns->unconfined->sid = aa_alloc_sid();
 	ns->unconfined->flags = PFLAG_UNCONFINED | PFLAG_IX_ON_NAME_ERROR |
 	    PFLAG_IMMUTABLE;
 
@@ -497,7 +496,6 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new)
 	/* released when @new is freed */
 	new->parent = aa_get_profile(old->parent);
 	new->ns = aa_get_namespace(old->ns);
-	new->sid = old->sid;
 	__list_add_profile(&policy->profiles, new);
 	/* inherit children */
 	list_for_each_entry_safe(child, tmp, &old->base.profiles, base.list) {
@@ -638,10 +636,11 @@ void __init aa_free_root_ns(void)
 /**
  * aa_alloc_profile - allocate, initialize and return a new profile
  * @hname: name of the profile  (NOT NULL)
+ * @sid: sid to use or AA_SID_ALLOC if a sid should be allocated
  *
  * Returns: refcount profile or NULL on failure
  */
-struct aa_profile *aa_alloc_profile(const char *hname)
+struct aa_profile *aa_alloc_profile(const char *hname, u32 sid)
 {
 	struct aa_profile *profile;
 
@@ -655,6 +654,10 @@ struct aa_profile *aa_alloc_profile(const char *hname)
 		return NULL;
 	}
 
+	if (sid == AA_SID_INVALID)
+		sid = aa_alloc_sid();
+	profile->sid = sid;
+
 	/* refcount released by caller */
 	return profile;
 }
@@ -685,12 +688,11 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat)
 		goto fail;
 	sprintf(name, "%s//null-%x", parent->base.hname, sid);
 
-	profile = aa_alloc_profile(name);
+	profile = aa_alloc_profile(name, sid);
 	kfree(name);
 	if (!profile)
 		goto fail;
 
-	profile->sid = sid;
 	profile->mode = APPARMOR_COMPLAIN;
 	profile->flags = PFLAG_NULL;
 	if (hat)
@@ -954,7 +956,6 @@ static void __add_new_profile(struct aa_namespace *ns, struct aa_policy *policy,
 		profile->parent = aa_get_profile((struct aa_profile *) policy);
 	__list_add_profile(&policy->profiles, profile);
 	/* released on free_profile */
-	profile->sid = aa_alloc_sid();
 	profile->ns = aa_get_namespace(ns);
 }
 
@@ -1092,14 +1093,8 @@ audit:
 	if (!error) {
 		if (rename_profile)
 			__replace_profile(rename_profile, new_profile);
-		if (old_profile) {
-			/* when there are both rename and old profiles
-			 * inherit old profiles sid
-			 */
-			if (rename_profile)
-				aa_free_sid(new_profile->sid);
+		if (old_profile)
 			__replace_profile(old_profile, new_profile);
-		}
 		if (!(old_profile || rename_profile))
 			__add_new_profile(ns, policy, new_profile);
 	}
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index ca48a7d..4b077ce 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -484,7 +484,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e)
 	if (!unpack_str(e, &name, NULL))
 		goto fail;
 
-	profile = aa_alloc_profile(name);
+	profile = aa_alloc_profile(name, AA_SID_ALLOC);
 	if (!profile)
 		return ERR_PTR(-ENOMEM);
 
-- 
1.7.9.5