[apparmor] [PATCH 18/24] apparmor: move replacedby to use labels instead of profiles

John Johansen john.johansen at canonical.com
Wed Feb 27 18:14:17 UTC 2013


moving replacedby to use labels will allow for faster label lookup
when a label is replaced and also remove the need to create replacement
labels to get a none stale version during task confinement lookup.

this is a first step just handling single profile labels, a second patch
to actually handle compound (multi-profile) labels will be needed

Signed-off-by: John Johansen <john.johansen at canonical.com>
---
 security/apparmor/apparmorfs.c      |   17 ++++----
 security/apparmor/context.c         |    4 +-
 security/apparmor/domain.c          |    6 +--
 security/apparmor/include/context.h |    2 +-
 security/apparmor/include/label.h   |   77 ++++++++++++++++++++++++++++++++++-
 security/apparmor/include/policy.h  |   51 -----------------------
 security/apparmor/label.c           |   30 ++++++++++++++
 security/apparmor/lsm.c             |    6 +--
 security/apparmor/policy.c          |   70 ++++++++++++++++---------------
 9 files changed, 159 insertions(+), 104 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 4493fdd..c0c0119 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -247,9 +247,10 @@ static int aa_fs_seq_profile_release(struct inode *inode, struct file *file)
 static int aa_fs_seq_profname_show(struct seq_file *seq, void *v)
 {
 	struct aa_replacedby *r = seq->private;
-	struct aa_profile *profile = aa_get_profile_rcu(&r->profile);
+	struct aa_label *label = aa_get_label_rcu(&r->label);
+	struct aa_profile *profile = labels_profile(label);
 	seq_printf(seq, "%s\n", profile->base.name);
-	aa_put_profile(profile);
+	aa_put_label(label);
 
 	return 0;
 }
@@ -270,9 +271,10 @@ static const struct file_operations aa_fs_profname_fops = {
 static int aa_fs_seq_profmode_show(struct seq_file *seq, void *v)
 {
 	struct aa_replacedby *r = seq->private;
-	struct aa_profile *profile = aa_get_profile_rcu(&r->profile);
+	struct aa_label *label = aa_get_label_rcu(&r->label);
+	struct aa_profile *profile = labels_profile(label);
 	seq_printf(seq, "%s\n", aa_profile_mode_names[profile->mode]);
-	aa_put_profile(profile);
+	aa_put_label(label);
 
 	return 0;
 }
@@ -293,14 +295,15 @@ static const struct file_operations aa_fs_profmode_fops = {
 static int aa_fs_seq_profattach_show(struct seq_file *seq, void *v)
 {
 	struct aa_replacedby *r = seq->private;
-	struct aa_profile *profile = aa_get_profile_rcu(&r->profile);
+	struct aa_label *label = aa_get_label_rcu(&r->label);
+	struct aa_profile *profile = labels_profile(label);
 	if (profile->attach)
 		seq_printf(seq, "%s\n", profile->attach);
 	else if (profile->xmatch)
 		seq_printf(seq, "<unknown>\n");
 	else
 		seq_printf(seq, "%s\n", profile->base.name);
-	aa_put_profile(profile);
+	aa_put_label(label);
 
 	return 0;
 }
@@ -357,7 +360,7 @@ static struct dentry *create_profile_file(struct dentry *dir, const char *name,
 					  struct aa_profile *profile,
 					  const struct file_operations *fops)
 {
-	struct aa_replacedby *r = aa_get_replacedby(profile->replacedby);
+	struct aa_replacedby *r = aa_get_replacedby(profile->label.replacedby);
 	struct dentry *dent;
 
 	dent = securityfs_create_file(name, S_IFREG | 0444, dir, r, fops);
diff --git a/security/apparmor/context.c b/security/apparmor/context.c
index 3064c6c..355c4ed 100644
--- a/security/apparmor/context.c
+++ b/security/apparmor/context.c
@@ -175,7 +175,7 @@ int aa_set_current_hat(struct aa_profile *profile, u64 token)
 		abort_creds(new);
 		return -EACCES;
 	}
-	cxt->profile = aa_get_newest_profile(profile);
+	cxt->profile = labels_profile(aa_get_newest_label(&profile->label));
 	/* clear exec on switching context */
 	aa_put_profile(cxt->onexec);
 	cxt->onexec = NULL;
@@ -212,7 +212,7 @@ int aa_restore_previous_profile(u64 token)
 	}
 
 	aa_put_profile(cxt->profile);
-	cxt->profile = aa_get_newest_profile(cxt->previous);
+	cxt->profile = labels_profile(aa_get_newest_label(&cxt->previous->label));
 	BUG_ON(!cxt->profile);
 	/* clear exec && prev information when restoring to previous context */
 	aa_clear_task_cxt_trans(cxt);
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index ccf19eb..66454ad 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -359,7 +359,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 	cxt = cred_cxt(bprm->cred);
 	BUG_ON(!cxt);
 
-	profile = aa_get_newest_profile(cxt->profile);
+	profile = labels_profile(aa_get_newest_label(&cxt->profile->label));
 	/*
 	 * get the namespace from the replacement profile as replacement
 	 * can change the namespace
@@ -417,7 +417,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 
 		if (!(cp.allow & AA_MAY_ONEXEC))
 			goto audit;
-		new_profile = aa_get_newest_profile(cxt->onexec);
+		new_profile = labels_profile(aa_get_newest_label(&cxt->onexec->label));
 		goto apply;
 	}
 
@@ -434,7 +434,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 				new_profile = aa_get_profile(profile);
 				goto x_clear;
 			} else if (perms.xindex & AA_X_UNCONFINED) {
-				new_profile = aa_get_newest_profile(ns->unconfined);
+				new_profile = labels_profile(aa_get_newest_label(&ns->unconfined->label));
 				info = "ux fallback";
 			} else {
 				error = -ENOENT;
diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h
index 6bf6579..10c4a30 100644
--- a/security/apparmor/include/context.h
+++ b/security/apparmor/include/context.h
@@ -153,7 +153,7 @@ static inline struct aa_profile *aa_current_profile(void)
 	BUG_ON(!cxt || !cxt->profile);
 
 	if (PROFILE_INVALID(cxt->profile)) {
-		profile = aa_get_newest_profile(cxt->profile);
+		profile = labels_profile(aa_get_newest_label(&cxt->profile->label));
 		aa_replace_current_profile(profile);
 		aa_put_profile(profile);
 		cxt = current_cxt();
diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h
index 70f5077..80391f6 100644
--- a/security/apparmor/include/label.h
+++ b/security/apparmor/include/label.h
@@ -120,11 +120,18 @@ enum label_flags {
 	FLAG_MEDIATE_DELETED = 0x10000, /* mediate instead delegate deleted */
 };
 
+struct aa_label;
+struct aa_replacedby {
+	struct kref count;
+	struct aa_label __rcu *label;
+};
+
 /* struct aa_label - lazy labeling struct
  * @count: ref count of active users
  * @node: rbtree position
  * @rcu: rcu callback struct
- * @name: text representation of the label (MAYBE_NULL)
+ * @replacedby: is set to the label that replaced this label
+ * @hname: text representation of the label (MAYBE_NULL)
  * @flags: invalid and other flags - values may change under label set lock
  * @sid: sid that references this label
  * @size: number of entries in @ent[]
@@ -134,6 +141,7 @@ struct aa_label {
 	struct kref count;
 	struct rb_node node;
 	struct rcu_head rcu;
+	struct aa_replacedby *replacedby;
 	__counted char *hname;
 	long flags;
 	u32 sid;
@@ -206,10 +214,77 @@ static inline struct aa_label *aa_get_label_not0(struct aa_label *l)
 	return NULL;
 }
 
+/**
+ * aa_get_label_rcu - increment refcount on a label that can be replaced
+ * @l: pointer to label that can be replaced (NOT NULL)
+ *
+ * Returns: pointer to a refcounted label.
+ *     else NULL if no label
+ */
+static inline struct aa_label *aa_get_label_rcu(struct aa_label __rcu **l)
+{
+	struct aa_label *c;
+
+	rcu_read_lock();
+	do {
+		c = rcu_dereference(*l);
+	} while (c && !kref_get_not0(&c->count));
+	rcu_read_unlock();
+
+	return c;
+}
+
+/**
+ * aa_get_newest_label - find the newest version of @l
+ * @l: the label to check for newer versions of
+ *
+ * Returns: refcounted newest version of @l taking into account
+ *          replacement, renames and removals
+ *          return @l.
+ */
+static inline struct aa_label *aa_get_newest_label(struct aa_label *l)
+{
+	if (!l)
+		return NULL;
+
+	if (label_invalid(l))
+		return aa_get_label_rcu(&l->replacedby->label);
+
+	return aa_get_label(l);
+}
+
 static inline void aa_put_label(struct aa_label *l)
 {
 	if (l)
 		kref_put(&l->count, aa_label_kref);
 }
 
+
+struct aa_replacedby *aa_alloc_replacedby(struct aa_label *l);
+void aa_free_replacedby_kref(struct kref *kref);
+
+static inline struct aa_replacedby *aa_get_replacedby(struct aa_replacedby *r)
+{
+	if (r)
+		kref_get(&(r->count));
+
+	return r;
+}
+
+static inline void aa_put_replacedby(struct aa_replacedby *r)
+{
+	if (r)
+		kref_put(&r->count, aa_free_replacedby_kref);
+}
+
+/* requires profile list write lock held */
+static inline void __aa_update_replacedby(struct aa_label *orig,
+					  struct aa_label *new)
+{
+	struct aa_label *tmp = rcu_dereference(orig->replacedby->label);
+	rcu_assign_pointer(orig->replacedby->label, aa_get_label(new));
+	orig->flags |= FLAG_INVALID;
+	aa_put_label(tmp);
+}
+
 #endif /* __AA_LABEL_H */
diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index 0a29372..932395d 100644
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -139,16 +139,10 @@ struct aa_policydb {
 
 };
 
-struct aa_replacedby {
-	struct kref count;
-	struct aa_profile __rcu *profile;
-};
-
 /* struct aa_profile - basic confinement data
  * @base - base components of the profile (name, refcount, lists, lock ...)
  * @parent: parent of profile
  * @ns: namespace the profile is in
- * @replacedby: is set to the profile that replaced this profile
  * @rename: optional profile name that this profile renamed
  * @attach: human readable attachment string
  * @xmatch: optional extended matching for unconfined executables names
@@ -187,7 +181,6 @@ struct aa_profile {
 	struct aa_profile __rcu *parent;
 
 	struct aa_namespace *ns;
-	struct aa_replacedby *replacedby;
 	const char *rename;
 
 	const char *attach;
@@ -223,7 +216,6 @@ struct aa_namespace *aa_find_namespace(struct aa_namespace *root,
 				       const char *name);
 
 
-void aa_free_replacedby_kref(struct kref *kref);
 struct aa_profile *aa_alloc_profile(const char *name);
 struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat);
 struct aa_profile *aa_setup_default_profile(void);
@@ -296,25 +288,6 @@ static inline struct aa_profile *aa_get_profile_rcu(struct aa_profile __rcu **p)
 }
 
 /**
- * aa_get_newest_profile - find the newest version of @profile
- * @profile: the profile to check for newer versions of
- *
- * Returns: refcounted newest version of @profile taking into account
- *          replacement, renames and removals
- *          return @profile.
- */
-static inline struct aa_profile *aa_get_newest_profile(struct aa_profile *p)
-{
-	if (!p)
-		return NULL;
-
-	if (PROFILE_INVALID(p))
-		return aa_get_profile_rcu(&p->replacedby->profile);
-
-	return aa_get_profile(p);
-}
-
-/**
  * aa_put_profile - decrement refcount on profile @p
  * @p: profile  (MAYBE NULL)
  */
@@ -324,30 +297,6 @@ static inline void aa_put_profile(struct aa_profile *p)
 		kref_put(&p->label.count, aa_label_kref);
 }
 
-static inline struct aa_replacedby *aa_get_replacedby(struct aa_replacedby *p)
-{
-	if (p)
-		kref_get(&(p->count));
-
-	return p;
-}
-
-static inline void aa_put_replacedby(struct aa_replacedby *p)
-{
-	if (p)
-		kref_put(&p->count, aa_free_replacedby_kref);
-}
-
-/* requires profile list write lock held */
-static inline void __aa_update_replacedby(struct aa_profile *orig,
-					  struct aa_profile *new)
-{
-	struct aa_profile *tmp = rcu_dereference(orig->replacedby->profile);
-	rcu_assign_pointer(orig->replacedby->profile, aa_get_profile(new));
-	orig->label.flags |= FLAG_INVALID;
-	aa_put_profile(tmp);
-}
-
 /**
  * aa_get_namespace - increment references count on @ns
  * @ns: namespace to increment reference count of (MAYBE NULL)
diff --git a/security/apparmor/label.c b/security/apparmor/label.c
index c369dcb..26c79fa 100644
--- a/security/apparmor/label.c
+++ b/security/apparmor/label.c
@@ -55,6 +55,35 @@
 #define AA_WARN(X) WARN((X), "APPARMOR WARN %s: %s\n", __FUNCTION__, #X)
 
 
+
+static void free_replacedby(struct aa_replacedby *r)
+{
+	if (r) {
+		aa_put_label(rcu_dereference(r->label));
+		kzfree(r);
+	}
+}
+
+void aa_free_replacedby_kref(struct kref *kref)
+{
+	struct aa_replacedby *r = container_of(kref, struct aa_replacedby,
+					       count);
+	free_replacedby(r);
+}
+
+struct aa_replacedby *aa_alloc_replacedby(struct aa_label *l)
+{
+	struct aa_replacedby *r;
+
+	r = kzalloc(sizeof(struct aa_replacedby), GFP_KERNEL);
+	if (r) {
+		kref_init(&r->count);
+		rcu_assign_pointer(r->label, aa_get_label(l));
+	}
+	return r;
+}
+
+
 /* helper fn for label_for_each_confined
  * - skip delegation? How exactly?
  * - skip profiles being cached on label with explicit rule access
@@ -103,6 +132,7 @@ void aa_label_destroy(struct aa_label *label)
 	}
 
 	aa_free_sid(label->sid);
+	aa_put_replacedby(label->replacedby);
 }
 
 void aa_label_free(struct aa_label *label)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 8e51e2e..31fb46d 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -511,11 +511,11 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
 	struct aa_profile *profile = NULL;
 
 	if (strcmp(name, "current") == 0)
-		profile = aa_get_newest_profile(cxt->profile);
+		profile = labels_profile(aa_get_newest_label(&cxt->profile->label));
 	else if (strcmp(name, "prev") == 0  && cxt->previous)
-		profile = aa_get_newest_profile(cxt->previous);
+		profile = labels_profile(aa_get_newest_label(&cxt->previous->label));
 	else if (strcmp(name, "exec") == 0 && cxt->onexec)
-		profile = aa_get_newest_profile(cxt->onexec);
+		profile = labels_profile(aa_get_newest_label(&cxt->onexec->label));
 	else
 		error = -EINVAL;
 
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 7732168..b396e19 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -298,6 +298,9 @@ static struct aa_namespace *alloc_namespace(const char *prefix,
 	ns->unconfined = aa_alloc_profile("unconfined");
 	if (!ns->unconfined)
 		goto fail_unconfined;
+	ns->unconfined->label.replacedby = aa_alloc_replacedby(NULL);
+	if (!ns->unconfined->label.replacedby)
+		goto fail_replacedby;
 
 	ns->unconfined->label.flags |= FLAG_IX_ON_NAME_ERROR |
 		FLAG_IMMUTABLE | FLAG_NS_COUNT | FLAG_UNCONFINED;
@@ -312,6 +315,9 @@ static struct aa_namespace *alloc_namespace(const char *prefix,
 
 	return ns;
 
+fail_replacedby:
+	aa_free_profile(ns->unconfined);
+
 fail_unconfined:
 	kzfree(ns->base.hname);
 fail_ns:
@@ -493,7 +499,7 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new)
 		list_splice_init_rcu(&old->base.profiles, &new->base.profiles,
 				     synchronize_rcu);
 	}
-	__aa_update_replacedby(old, new);
+	__aa_update_replacedby(&old->label, &new->label);
 	list_replace_rcu(&old->base.list, &new->base.list);
 	__aa_fs_profile_migrate_dents(old, new);
 	aa_get_profile(new);
@@ -513,7 +519,8 @@ static void __remove_profile(struct aa_profile *profile)
 	/* release any children lists first */
 	__profile_list_release(&profile->base.profiles);
 	/* released by free_profile */
-	__aa_update_replacedby(profile, profile->ns->unconfined);
+	__aa_update_replacedby(&profile->label,
+			       &profile->ns->unconfined->label);
 	__list_remove_profile(profile);
 	__aa_fs_profile_rmdir(profile);
 }
@@ -550,7 +557,8 @@ static void destroy_namespace(struct aa_namespace *ns)
 	__ns_list_release(&ns->sub_ns);
 
 	if (ns->parent)
-		__aa_update_replacedby(ns->unconfined, ns->parent->unconfined);
+		__aa_update_replacedby(&ns->unconfined->label,
+				       &ns->parent->unconfined->label);
 	__aa_fs_namespace_rmdir(ns);
 	mutex_unlock(&ns->lock);
 }
@@ -612,22 +620,6 @@ void __init aa_free_root_ns(void)
 }
 
 
-static void free_replacedby(struct aa_replacedby *r)
-{
-	if (r) {
-		aa_put_profile(rcu_dereference(r->profile));
-		kzfree(r);
-	}
-}
-
-
-void aa_free_replacedby_kref(struct kref *kref)
-{
-	struct aa_replacedby *r = container_of(kref, struct aa_replacedby,
-					       count);
-	free_replacedby(r);
-}
-
 /**
  * aa_free_profile - free a profile
  * @profile: the profile to free  (MAYBE NULL)
@@ -666,7 +658,6 @@ void aa_free_profile(struct aa_profile *profile)
 	kzfree(profile->dirname);
 	aa_put_dfa(profile->xmatch);
 	aa_put_dfa(profile->policy.dfa);
-	aa_put_replacedby(profile->replacedby);
 
 	kzfree(profile);
 }
@@ -686,11 +677,6 @@ struct aa_profile *aa_alloc_profile(const char *hname)
 	if (!profile)
 		return NULL;
 
-	profile->replacedby = kzalloc(sizeof(struct aa_replacedby), GFP_KERNEL);
-	if (!profile->replacedby)
-		goto fail;
-	kref_init(&profile->replacedby->count);
-
 	if (!policy_init(&profile->base, NULL, hname))
 		goto fail;
 	if (!aa_label_init(&profile->label, 1))
@@ -703,7 +689,6 @@ struct aa_profile *aa_alloc_profile(const char *hname)
 	return profile;
 
 fail:
-	kzfree(profile->replacedby);
 	kzfree(profile);
 
 	return NULL;
@@ -740,6 +725,10 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat)
 	if (!profile)
 		goto fail;
 
+	profile->label.replacedby = aa_alloc_replacedby(NULL);
+	if (!profile->label.replacedby)
+		goto fail;
+
 	profile->mode = APPARMOR_COMPLAIN;
 	profile->label.flags |= FLAG_NULL;
 	if (hat)
@@ -757,6 +746,7 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat)
 	return profile;
 
 fail:
+	aa_free_profile(profile);
 	return NULL;
 }
 
@@ -776,8 +766,11 @@ struct aa_profile *aa_setup_default_profile(void)
 	profile->ns = aa_get_namespace(root_ns);
 
 	/* replacedby being set needed by fs interface */
-	rcu_assign_pointer(profile->replacedby->profile,
-			   aa_get_profile(profile));
+	profile->label.replacedby = aa_alloc_replacedby(&profile->label);
+	if (!profile->label.replacedby) {
+		aa_free_profile(profile);
+		return NULL;
+	}
 	__list_add_profile(&root_ns->base.profiles, profile);
 
 	return profile;
@@ -932,7 +925,7 @@ struct aa_profile *aa_lookupn_profile(struct aa_namespace *ns,
 
 	/* the unconfined profile is not in the regular profile list */
 	if (!profile && strncmp(hname, "unconfined", n) == 0)
-		profile = aa_get_newest_profile(ns->unconfined);
+		profile = labels_profile(aa_get_newest_label(ns->unconfined));
 
 	/* refcount released by caller */
 	return profile;
@@ -1145,23 +1138,28 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace)
 	/* create new fs entries for introspection if needed */
 	list_for_each_entry(new, &lh, base.list) {
 		struct aa_profile *old, *rename;
+		struct aa_replacedby *r;
 		policy = __lookup_replace(ns, new, noreplace, &old, &rename,
 					  &info);
 		if (old) {
-			aa_put_replacedby(new->replacedby);
-			new->replacedby = aa_get_replacedby(old->replacedby);
+			r = aa_get_replacedby(old->label.replacedby);
 
 			if (rename) {
 				// ????? mkfiles for children
 				// replacedby not updated here
 			}
 		} else if (rename) {
-			aa_put_replacedby(new->replacedby);
-			new->replacedby = aa_get_replacedby(rename->replacedby);
+			r = aa_get_replacedby(rename->label.replacedby);
 
 			// ????
+		} else {
+			r = aa_alloc_replacedby(NULL);
+			if (!r)
+				error = -ENOMEM;
 		}
-		if (!old) {
+		new->label.replacedby = r;
+
+		if (!error && !old) {
 			struct dentry *parent;
 			if (new->parent) {
 				struct aa_profile *p;
@@ -1216,8 +1214,8 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace)
 			 * has a file in the interface fs. The backref will
 			 * be removed by replacement/removal
 			 */
-			rcu_assign_pointer(new->replacedby->profile,
-					   aa_get_profile(new));
+			rcu_assign_pointer(new->label.replacedby->label,
+					   aa_get_label(&new->label));
 			__list_add_profile(&policy->profiles, new);
 			l = aa_label_insert(&ns->labels, &new->label);
 			aa_put_label(l);
-- 
1.7.10.4




More information about the AppArmor mailing list