Revert "UBUNTU: SAUCE: apparmor: Fix: convert replacedby update to be...

Revert "UBUNTU: SAUCE: apparmor: Fix: convert replacedby update to be protected by the labelset lock"

BugLink: http://bugs.launchpad.net/bugs/1379535

This reverts commit 6b05f95e2bd24c98400eae1c25d36a10242e7f5b.
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

security/apparmor/af_unix.c

@@ -288,11 +288,7 @@ static int unix_label_sock_perm(struct aa_label *label, int op, u32 request,
 /* revaliation, get/set attr */
 int aa_unix_sock_perm(int op, u32 request, struct socket *sock)
 {
-	struct aa_label *label = aa_begin_current_label(DO_UPDATE);
-	int error = unix_label_sock_perm(label, op, request, sock);
-	aa_end_current_label(label);
-
-	return error;
+	return unix_label_sock_perm(aa_current_label(), op, request, sock);
 }
 
 static int profile_bind_perm(struct aa_profile *profile, struct sock *sk,
@@ -329,17 +325,14 @@ int aa_unix_bind_perm(struct socket *sock, struct sockaddr *address,
 		      int addrlen)
 {
 	struct aa_profile *profile;
-	struct aa_label *label = aa_begin_current_label(DO_UPDATE);
-	int error = 0;
+	struct aa_label *label = aa_current_label();
 
 	 /* fs bind is handled by mknod */
-	if (!(unconfined(label) || unix_addr_fs(address, addrlen)))
-		error = fn_for_each_confined(label, profile,
-				profile_bind_perm(profile, sock->sk, address,
-						  addrlen));
-	aa_end_current_label(label);
+	if (unconfined(label) || unix_addr_fs(address, addrlen))
+		return 0;
 
-	return error;
+	return fn_for_each_confined(label, profile,
+			profile_bind_perm(profile, sock->sk, address, addrlen));
 }
 
 int aa_unix_connect_perm(struct socket *sock, struct sockaddr *address,
@@ -386,16 +379,13 @@ static int profile_listen_perm(struct aa_profile *profile, struct sock *sk,
 int aa_unix_listen_perm(struct socket *sock, int backlog)
 {
 	struct aa_profile *profile;
-	struct aa_label *label = aa_begin_current_label(DO_UPDATE);
-	int error = 0;
+	struct aa_label *label = aa_current_label();
 
-	if (!(unconfined(label) || UNIX_FS(sock->sk)))
-		error = fn_for_each_confined(label, profile,
-				profile_listen_perm(profile, sock->sk,
-						    backlog));
-	aa_end_current_label(label);
+	if (unconfined(label) || UNIX_FS(sock->sk))
+		return 0;
 
-	return error;
+	return fn_for_each_confined(label, profile,
+			profile_listen_perm(profile, sock->sk, backlog));
 }
 
 
@@ -428,16 +418,13 @@ static inline int profile_accept_perm(struct aa_profile *profile,
 int aa_unix_accept_perm(struct socket *sock, struct socket *newsock)
 {
 	struct aa_profile *profile;
-	struct aa_label *label = aa_begin_current_label(DO_UPDATE);
-	int error = 0;
+	struct aa_label *label = aa_current_label();
 
-	if (!(unconfined(label) || UNIX_FS(sock->sk)))
-		error = fn_for_each_confined(label, profile,
-				profile_accept_perm(profile, sock->sk,
-						    newsock->sk));
-	aa_end_current_label(label);
+	if (unconfined(label) || UNIX_FS(sock->sk))
+		return 0;
 
-	return error;
+	return fn_for_each_confined(label, profile,
+			profile_accept_perm(profile, sock->sk, newsock->sk));
 }
 
 
@@ -486,16 +473,14 @@ int aa_unix_opt_perm(int op, u32 request, struct socket *sock, int level,
 		     int optname)
 {
 	struct aa_profile *profile;
-	struct aa_label *label = aa_begin_current_label(DO_UPDATE);
-	int error = 0;
+	struct aa_label *label = aa_current_label();
 
-	if (!(unconfined(label) || UNIX_FS(sock->sk)))
-		error = fn_for_each_confined(label, profile,
-				profile_opt_perm(profile, op, request,
-						 sock->sk, level, optname));
-	aa_end_current_label(label);
+	if (unconfined(label) || UNIX_FS(sock->sk))
+		return 0;
 
-	return error;
+	return fn_for_each_confined(label, profile,
+			profile_opt_perm(profile, op, request, sock->sk,
+					 level, optname));
 }
 
 /* null peer_label is allowed, in which case the peer_sk label is used */

security/apparmor/apparmorfs.c

@@ -212,7 +212,7 @@ static ssize_t query_label(char *buf, size_t buf_len,
 			   char *query, size_t query_len)
 {
 	struct aa_profile *profile;
-	struct aa_label *label, *curr;
+	struct aa_label *label;
 	char *label_name, *match_str;
 	size_t label_name_len, match_len;
 	struct aa_perms perms;
@@ -236,9 +236,8 @@ static ssize_t query_label(char *buf, size_t buf_len,
 	match_str = label_name + label_name_len + 1;
 	match_len = query_len - label_name_len - 1;
 
-	curr = aa_begin_current_label(DO_UPDATE);
-	label = aa_label_parse(curr, label_name, GFP_KERNEL, false);
-	aa_end_current_label(curr);
+	label = aa_label_parse(aa_current_label(), label_name, GFP_KERNEL,
+			       false);
 	if (IS_ERR(label))
 		return PTR_ERR(label);
 
@@ -875,7 +874,7 @@ static struct aa_profile *next_profile(struct aa_namespace *root,
 static void *p_start(struct seq_file *f, loff_t *pos)
 {
 	struct aa_profile *profile = NULL;
-	struct aa_namespace *root = current_ns();
+	struct aa_namespace *root = labels_ns(aa_current_label());
 	loff_t l = *pos;
 	f->private = aa_get_namespace(root);
 

security/apparmor/include/context.h

@@ -137,6 +137,24 @@ static inline struct aa_label *aa_get_current_label(void)
 	return aa_get_label(l);
 }
 
+/**
+ * aa_begin_current_label - find newest version of the current tasks label
+ *
+ * Returns: newest version of confining label (NOT NULL)
+ *
+ * This fn will not update the tasks cred, so it is safe inside of locks
+ *
+ * The returned reference must be put with aa_end_current_label()
+ */
+static inline struct aa_label *aa_begin_current_label(void)
+{
+	struct aa_label *l = aa_current_raw_label();
+
+	if (label_invalid(l))
+		l = aa_get_newest_label(l);
+	return l;
+}
+
 /**
  * aa_end_current_label - put a reference found with aa_begin_current_label
  * @label: label reference to put
@@ -151,35 +169,29 @@ static inline void aa_end_current_label(struct aa_label *label)
 }
 
 /**
- * aa_begin_current_label - find the current tasks confining label and update it
- * @update: whether the current label can be updated
+ * aa_current_label - find the current tasks confining label and update it
  *
  * Returns: up to date confining label or the ns unconfined label (NOT NULL)
  *
- * If @update is true this fn will update the tasks cred structure if the
- *   label has been replaced.  Not safe to call inside locks
- * else
- *   just return the up to date label
- *
- * The returned reference must be put with aa_end_current_label()
+ * This fn will update the tasks cred structure if the label has been
+ * replaced.  Not safe to call inside locks
  */
-static inline struct aa_label *aa_begin_current_label(bool update)
+static inline struct aa_label *aa_current_label(void)
 {
-	struct aa_label *label = aa_current_raw_label();
+	const struct aa_task_cxt *cxt = current_cxt();
+	struct aa_label *label;
+	BUG_ON(!cxt || !cxt->label);
 
-	if (label_invalid(label)) {
-		label = aa_get_newest_label(label);
-		if (update && aa_replace_current_label(label) == 0)
-			/* task cred will keep the reference */
-			aa_put_label(label);
+	if (label_invalid(cxt->label)) {
+		label = aa_get_newest_label(cxt->label);
+		aa_replace_current_label(label);
+		aa_put_label(label);
+		cxt = current_cxt();
 	}
 
-	return label;
+	return cxt->label;
 }
 
-#define NO_UPDATE false
-#define DO_UPDATE true
-
 /**
  * aa_clear_task_cxt_trans - clear transition tracking info from the cxt
  * @cxt: task context to clear (NOT NULL)

security/apparmor/include/label.h

@@ -296,7 +296,8 @@ struct aa_label *aa_label_insert(struct aa_labelset *ls, struct aa_label *l);
 struct aa_label *aa_label_remove_and_insert(struct aa_labelset *ls,
 					    struct aa_label *remove,
 					    struct aa_label *insert);
-bool aa_label_replace(struct aa_label *old, struct aa_label *new);
+bool aa_label_replace(struct aa_labelset *ls, struct aa_label *old,
+		      struct aa_label *new);
 bool aa_label_make_newest(struct aa_labelset *ls, struct aa_label *old,
 			  struct aa_label *new);
 

security/apparmor/label.c

@@ -78,7 +78,7 @@ void __aa_update_replacedby(struct aa_label *orig, struct aa_label *new)
 
 	AA_BUG(!orig);
 	AA_BUG(!new);
-	AA_BUG(!write_is_locked(&labels_set(orig)->lock));
+	AA_BUG(!mutex_is_locked(&labels_ns(orig)->lock));
 
 	tmp = rcu_dereference_protected(orig->replacedby->label,
 					&labels_ns(orig)->lock);
@@ -356,7 +356,6 @@ bool aa_label_remove(struct aa_labelset *ls, struct aa_label *l)
 	bool res;
 
 	write_lock_irqsave(&ls->lock, flags);
-	__aa_update_replacedby(l, &labels_ns(l)->unconfined->label);
 	res = __aa_label_remove(ls, l);
 	write_unlock_irqrestore(&ls->lock, flags);
 
@@ -426,42 +425,25 @@ struct aa_label *aa_label_remove_and_insert(struct aa_labelset *ls,
 
 /**
  * aa_label_replace - replace a label @old with a new version @new
+ * @ls: labelset being manipulated
  * @old: label to replace
  * @new: label replacing @old
  *
  * Returns: true if @old was in tree and replaced
  *     else @old was not in tree, and @new was not inserted
  */
-bool aa_label_replace(struct aa_label *old, struct aa_label *new)
+bool aa_label_replace(struct aa_labelset *ls, struct aa_label *old,
+		      struct aa_label *new)
 {
+	struct aa_label *l;
 	unsigned long flags;
 	bool res;
 
-	if (old->hname == new->hname && labels_ns(old) == labels_ns(new)) {
-		write_lock_irqsave(&labels_set(old)->lock, flags);
-		if (old->replacedby != new->replacedby) {
-			free_replacedby(new->replacedby);
-			new->replacedby = aa_get_replacedby(old->replacedby);
-		}
-		__aa_update_replacedby(old, new);
-		res = __aa_label_replace(labels_set(old), old, new);
-		write_unlock_irqrestore(&labels_set(old)->lock, flags);
-	} else {
-		struct aa_label *l;
-		struct aa_labelset *ls = labels_set(old);
-		write_lock_irqsave(&ls->lock, flags);
-		__aa_update_replacedby(old, new);
-		res = __aa_label_remove(ls, old);
-		if (labels_ns(old) != labels_ns(new)) {
-			write_unlock_irqrestore(&ls->lock, flags);
-			ls = labels_set(new);
-			write_lock_irqsave(&ls->lock, flags);
-		}
-		l = __aa_label_insert(ls, new, true);
-		res = (l == new);
-		write_unlock_irqrestore(&ls->lock, flags);
-		aa_put_label(l);
-	}
+	write_lock_irqsave(&ls->lock, flags);
+	l = __aa_label_remove_and_insert(ls, old, new, true);
+	res = (l == new);
+	write_unlock_irqrestore(&ls->lock, flags);
+	aa_put_label(l);
 
 	return res;
 }
@@ -1142,9 +1124,11 @@ struct aa_label *aa_label_merge(struct aa_label *a, struct aa_label *b,
 		/* only label update will set replacedby so ns lock is enough */
 		new->replacedby = r;
 
+		mutex_lock(&labels_ns(a)->lock);
 		write_lock_irqsave(&ls->lock, flags);
 		label = __label_merge_insert(ls, new, a, b);
 		write_unlock_irqrestore(&ls->lock, flags);
+		mutex_unlock(&labels_ns(a)->lock);
 
 		if (label != new) {
 			/* new may not be fully setup so no put_label */
@@ -1187,9 +1171,11 @@ struct aa_label *aa_label_vec_merge(struct aa_profile **vec, int len,
 	for (i = 0; i < len; i++) {
 		new->ent[i] = aa_get_profile(vec[i]);
 	}
+	mutex_lock(&labels_ns(new)->lock);
 	write_lock_irqsave(&ls->lock, flags);
 	label = __aa_label_insert(ls, new, false);
 	write_unlock_irqrestore(&ls->lock, flags);
+	mutex_unlock(&labels_ns(new)->lock);
 	if (label != new)
 		/* not fully constructed don't put */
 		aa_label_free(new);

security/apparmor/net.c

@@ -253,17 +253,13 @@ static int aa_label_sk_perm(struct aa_label *label, int op, u32 request,
 static int aa_sk_perm(int op, u32 request, struct sock *sk)
 {
 	struct aa_label *label;
-	int error;
 
 	AA_BUG(!sk);
 	AA_BUG(in_interrupt());
 
 	/* TODO: switch to begin_current_label ???? */
-	label = aa_begin_current_label(DO_UPDATE);
-	error = aa_label_sk_perm(label, op, request, sk);
-	aa_end_current_label(label);
-
-	return error;
+	label = aa_current_label();
+	return aa_label_sk_perm(label, op, request, sk);
 }
 
 #define af_select(FAMILY, FN, DEF_FN)		\

security/apparmor/policy.c

@@ -412,7 +412,7 @@ static struct aa_namespace *aa_prepare_namespace(const char *name)
 {
 	struct aa_namespace *ns, *root;
 
-	root = current_ns();
+	root = labels_ns(aa_current_label());
 
 	mutex_lock(&root->lock);
 
@@ -517,6 +517,8 @@ static void __remove_profile(struct aa_profile *profile)
 	__profile_list_release(&profile->base.profiles);
 	/* released by free_profile */
 	aa_label_remove(&profile->ns->labels, &profile->label);
+	__aa_update_replacedby(&profile->label,
+			       &profile->ns->unconfined->label);
 	__aa_fs_profile_rmdir(profile);
 	__list_remove_profile(profile);
 }
@@ -1042,6 +1044,7 @@ static struct aa_profile *__list_lookup_parent(struct list_head *lh,
  * __replace_profile - replace @old with @new on a list
  * @old: profile to be replaced  (NOT NULL)
  * @new: profile to replace @old with  (NOT NULL)
+ * @share_replacedby: transfer @old->replacedby to @new
  *
  * Will duplicate and refcount elements that @new inherits from @old
  * and will inherit @old children.
@@ -1050,7 +1053,8 @@ static struct aa_profile *__list_lookup_parent(struct list_head *lh,
  *
  * Requires: namespace list lock be held, or list not be shared
  */
-static void __replace_profile(struct aa_profile *old, struct aa_profile *new)
+static void __replace_profile(struct aa_profile *old, struct aa_profile *new,
+			      bool share_replacedby)
 {
 	struct aa_profile *child, *tmp;
 
@@ -1065,7 +1069,7 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new)
 			p = __find_child(&new->base.profiles, child->base.name);
 			if (p) {
 				/* @p replaces @child  */
-				__replace_profile(child, p);
+				__replace_profile(child, p, share_replacedby);
 				continue;
 			}
 
@@ -1083,8 +1087,13 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new)
 		struct aa_profile *parent = aa_deref_parent(old);
 		rcu_assign_pointer(new->parent, aa_get_profile(parent));
 	}
-	aa_label_replace(&old->label, &new->label);
-	/* migrate dents must come after label replacement b/c replacedby */
+	__aa_update_replacedby(&old->label, &new->label);
+	if (share_replacedby)
+		new->label.replacedby = aa_get_replacedby(old->label.replacedby);
+	else if (!rcu_access_pointer(new->label.replacedby->label))
+		/* aafs interface uses replacedby */
+		rcu_assign_pointer(new->label.replacedby->label,
+				   aa_get_label(&new->label));
 	__aa_fs_profile_migrate_dents(old, new);
 
 	if (list_empty(&new->base.list)) {
@@ -1232,16 +1241,24 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace)
 
 	/* create new fs entries for introspection if needed */
 	list_for_each_entry(ent, &lh, list) {
-		struct aa_replacedby *r = aa_alloc_replacedby(&ent->new->label);
-		if (!r) {
-			info = "failed to create";
-			error = -ENOMEM;
-			goto fail_lock;
-		}
-		ent->new->label.replacedby = r;
+		struct aa_replacedby *r;
+		if (ent->old) {
+			/* inherit old interface files */
 
-		if (!ent->old) {
+			/* if (ent->rename)
+				TODO: support rename */
+		/* } else if (ent->rename) {
+			TODO: support rename */
+		} else {
 			struct dentry *parent;
+			r = aa_alloc_replacedby(NULL);
+			if (!r) {
+				info = "failed to create";
+				error = -ENOMEM;
+				goto fail_lock;
+			}
+			ent->new->label.replacedby = r;
+
 			if (rcu_access_pointer(ent->new->parent)) {
 				struct aa_profile *p;
 				p = aa_deref_parent(ent->new);
@@ -1266,12 +1283,19 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace)
 
 		if (ent->old) {
 			share_name(ent->old, ent->new);
-			__replace_profile(ent->old, ent->new);
-			if (ent->rename)
-				__replace_profile(ent->rename, ent->new);
+			__replace_profile(ent->old, ent->new, 1);
+			aa_label_replace(&ns->labels, &ent->old->label,
+					 &ent->new->label);
+			if (ent->rename) {
+				/* aafs interface uses replacedby */
+				rcu_assign_pointer(ent->new->label.replacedby->label,
+						   aa_get_label(&ent->new->label));
+				__replace_profile(ent->rename, ent->new, 0);
+			}
 		} else if (ent->rename) {
-			/* TODO: case not actually supported yet */
-			;
+			/* aafs interface uses replacedby */
+			rcu_assign_pointer(ent->new->label.replacedby->label,
+					   aa_get_label(&ent->new->label));
 		} else {
 			struct list_head *lh;
 			if (rcu_access_pointer(ent->new->parent)) {
@@ -1280,6 +1304,10 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace)
                                lh = &parent->base.profiles;
 			} else
 				lh = &ns->base.profiles;
+
+			/* aafs interface uses replacedby */
+			rcu_assign_pointer(ent->new->label.replacedby->label,
+					   aa_get_label(&ent->new->label));
 			__add_profile(lh, ent->new);
 		}
 		aa_load_ent_free(ent);
@@ -1332,7 +1360,7 @@ ssize_t aa_remove_profiles(char *fqname, size_t size)
 		goto fail;
 	}
 
-	root = current_ns();
+	root = labels_ns(aa_current_label());
 
 	if (fqname[0] == ':') {
 		char *ns_name;

security/apparmor/procattr.c

@@ -36,7 +36,7 @@
 int aa_getprocattr(struct aa_label *label, char **string)
 {
 	struct aa_namespace *ns = labels_ns(label);
-	struct aa_namespace *current_ns = current_ns();
+	struct aa_namespace *current_ns = labels_ns(aa_current_label());
 	int len;
 
 	if (!aa_ns_visible(current_ns, ns))