apparmor/kernel-patches/for-mainline/rework-locking-2.diff

Index: b/security/apparmor/main.c
===================================================================
--- a/security/apparmor/main.c
+++ b/security/apparmor/main.c
@@ -720,41 +720,46 @@ int aa_link(struct aa_profile *profile,
  * aa_fork - initialize the task context for a new task
  * @task: task that is being created
  */
-int aa_fork(struct task_struct *task)
+int aa_fork(struct task_struct *child)
 {
-	struct aa_task_context *cxt = aa_task_context(current);
-	struct aa_task_context *newcxt = NULL;
-
-	AA_DEBUG("%s\n", __FUNCTION__);
+	struct aa_task_context *cxt, *child_cxt;
+	struct aa_profile *profile;
 
-	if (cxt && cxt->profile) {
-		newcxt = aa_alloc_task_context(task);
+	child_cxt = aa_alloc_task_context(child);
+	if (!child_cxt)
+		return -ENOMEM;
 
-		/* FIXME: The alloc above is a blocking operation, so
-		 *        cxt->profile may have vanished by now.
-		 *        We really need to do the full stale checking
-		 *        thing here, too.
-		 */
+repeat:
+	profile = aa_get_profile(current);
+	if (profile) {
+		lock_profile(profile);
+		cxt = aa_task_context(current);
+		if (unlikely(profile->isstale || !cxt ||
+			     cxt->profile != profile)) {
+			/**
+			 * Race with profile replacement or removal, or with
+			 * task context removal.
+			 */
+			unlock_profile(profile);
+			aa_put_profile(profile);
+			goto repeat;
+		}
 
-		if (!newcxt)
-			return -ENOMEM;
+		/* No need to grab the child's task lock here. */
+		rcu_assign_pointer(child->security, child_cxt);
+		aa_change_profile(child_cxt, profile, cxt->hat_magic);
+		unlock_profile(profile);
 
-		/*
-		 * Use locking here instead of getting the reference
-		 * because we need both the old reference and the
-		 * new reference to be consistent: otherwise, we could
-		 * race with profile replacement or removal here, and
-		 * he new task would end up with an obsolete profile.
-		 */
-		aa_change_profile(newcxt, cxt->profile, cxt->hat_magic);
+		if (APPARMOR_COMPLAIN(child_cxt) &&
+		    profile == null_complain_profile) {
+			LOG_HINT(profile, GFP_KERNEL, HINT_FORK,
+				 "pid=%d child=%d\n",
+				 current->pid, child->pid);
+		}
+		aa_put_profile(profile);
+	} else
+		aa_free_task_context(child_cxt);
 
-		if (APPARMOR_COMPLAIN(cxt) &&
-		    cxt->profile == null_complain_profile)
-			LOG_HINT(cxt->profile, GFP_KERNEL, HINT_FORK,
-				"pid=%d child=%d\n",
-				current->pid, task->pid);
-	}
-	task->security = newcxt;
 	return 0;
 }
 
@@ -808,7 +813,7 @@ int aa_register(struct linux_binprm *bpr
 {
 	char *filename, *buffer = NULL;
 	struct file *filp = bprm->file;
-	struct aa_profile *profile, *newprofile = NULL;
+	struct aa_profile *profile, *old_profile, *new_profile = NULL;
 	int exec_mode = AA_EXEC_UNSAFE, complain = 0;
 
 	AA_DEBUG("%s\n", __FUNCTION__);
@@ -819,8 +824,8 @@ int aa_register(struct linux_binprm *bpr
 		return -ENOENT;
 	}
 
-	profile = aa_get_profile(current);
 repeat:
+	profile = aa_get_profile(current);
 	if (profile) {
 		complain = PROFILE_COMPLAIN(profile);
 
@@ -843,16 +848,16 @@ repeat:
 					 __FUNCTION__,
 					 filename);
 
-				/* unload profile */
-				newprofile = NULL;
+				/* detach current profile */
+				new_profile = NULL;
 				break;
 
 			case MAY_EXEC | AA_EXEC_PROFILE:
 				AA_DEBUG("%s: PROFILE %s\n",
 					 __FUNCTION__,
 					 filename);
-				newprofile = aa_register_find(filename, 1,
-							      complain);
+				new_profile = aa_register_find(filename, 1,
+							       complain);
 				break;
 
 			default:
@@ -865,7 +870,7 @@ repeat:
 					 current->comm, current->pid,
 					 profile->parent->name,
 					 profile->name);
-				newprofile = ERR_PTR(-EPERM);
+				new_profile = ERR_PTR(-EPERM);
 				break;
 			}
 
@@ -874,7 +879,7 @@ repeat:
 			 * describing mode to execute image in.
 			 * Drop into null-profile (disabling secure exec).
 			 */
-			newprofile = aa_dup_profile(null_complain_profile);
+			new_profile = aa_dup_profile(null_complain_profile);
 			exec_mode |= AA_EXEC_UNSAFE;
 		} else {
 			AA_WARN("%s: Rejecting exec(2) of image '%s'. "
@@ -884,117 +889,58 @@ repeat:
 				filename,
 				current->comm, current->pid,
 				profile->parent->name, profile->name);
-			newprofile = ERR_PTR(-EPERM);
+			new_profile = ERR_PTR(-EPERM);
 		}
 	} else {
 		/* Unconfined task, load profile if it exists */
-		newprofile = aa_register_find(filename, 0, 0);
-		if (newprofile == NULL)
+		new_profile = aa_register_find(filename, 0, 0);
+		if (new_profile == NULL)
 			goto cleanup;
 	}
 
-	/* Apply the new profile, or switch to unconfined if NULL. */
-	if (!IS_ERR(newprofile)) {
-		struct aa_task_context *cxt, *lazy_cxt = NULL;
+	if (IS_ERR(new_profile))
+		goto cleanup;
 
-		/* grab a lock - this is to guarentee consistency against
-		 * other writers of aa_task_context (replacement/removal)
-		 *
-		 * Several things may have changed since the code above
-		 *
-		 * - Task may be presently unconfined (have no cxt). In which
-		 *   case we have to lazily allocate one.  Note we may be raced
-		 *   to this allocation by a setprofile.
-		 *
-		 * - If we are a confined process, profile is a refcounted copy
-		 *   of the profile that was on the aa_task_context at entry.
-		 *   This allows us to not have to hold a lock around
-		 *   all this code.   If profile replacement has taken place
-		 *   our profile may not equal cxt->profile any more.
-		 *   This is okay since the operation is treated as if
-		 *   the transition occured before replacement.
-		 *
-		 * - If newprofile points to an actual profile (result of
-		 *   aa_find_profile above), this profile may have been
-		 *   replaced.  We need to fix it up.  Doing this to avoid
-		 *   having to hold a lock around all this code.
-		 */
-
-		if (!profile && !(cxt = aa_task_context(current))) {
-			lazy_cxt = aa_alloc_task_context(current);
-			if (!lazy_cxt) {
-				AA_ERROR("%s: Failed to allocate aa_task_context\n",
-					 __FUNCTION__);
-				newprofile = ERR_PTR(-ENOMEM);
-				goto cleanup;
-			}
-		}
-
-		cxt = aa_task_context(current);
-		if (lazy_cxt) {
-			if (cxt) {
-				/* raced by setprofile - created cxt */
-				aa_free_task_context(lazy_cxt);
-				lazy_cxt = NULL;
-			} else {
-				/* Not rcu used to get the write barrier
-				 * correct */
-				rcu_assign_pointer(current->security, lazy_cxt);
-				cxt = lazy_cxt;
-			}
-		}
-
-		/* Determine if profile we found earlier is stale.
-		 * If so, reobtain it.  N.B stale flag should never be
-		 * set on null_complain profile.
-		 */
-		if (newprofile && unlikely(newprofile->isstale)) {
-			WARN_ON(newprofile == null_complain_profile);
-
-			/* drop refcnt obtained from earlier aa_dup_profile */
-			aa_put_profile(newprofile);
-
-			newprofile = aa_find_profile(filename);
-
-			if (!newprofile) {
-				/* Race, profile was removed, not replaced.
-				 * Redo with error checking
-				 */
-				goto repeat;
-			}
-		}
-
-		/* Handle confined exec.
-		 * Can be at this point for the following reasons:
-		 * 1. unconfined switching to confined
-		 * 2. confined switching to different confinement
-		 * 3. confined switching to unconfined
-		 *
-		 * Cases 2 and 3 are marked as requiring secure exec
-		 * (unless policy specified "unsafe exec")
-		 */
-		if (newprofile && !(exec_mode & AA_EXEC_UNSAFE)) {
-			unsigned long bprm_flags;
+	old_profile = aa_replace_profile(current, new_profile, 0);
+	if (IS_ERR(old_profile)) {
+		aa_put_profile(new_profile);
+		aa_put_profile(profile);
+		if (PTR_ERR(old_profile) == -ESTALE)
+			goto repeat;
+		new_profile = old_profile;
+		goto cleanup;
+	}
+	aa_put_profile(old_profile);
+	aa_put_profile(profile);
 
-			bprm_flags = AA_SECURE_EXEC_NEEDED;
-			bprm->security = (void*)
-				((unsigned long)bprm->security | bprm_flags);
-		}
+	/* Handle confined exec.
+	 * Can be at this point for the following reasons:
+	 * 1. unconfined switching to confined
+	 * 2. confined switching to different confinement
+	 * 3. confined switching to unconfined
+	 *
+	 * Cases 2 and 3 are marked as requiring secure exec
+	 * (unless policy specified "unsafe exec")
+	 */
+	if (new_profile && !(exec_mode & AA_EXEC_UNSAFE)) {
+		unsigned long bprm_flags;
 
-		aa_change_profile(cxt, newprofile, 0);
+		bprm_flags = AA_SECURE_EXEC_NEEDED;
+		bprm->security = (void*)
+			((unsigned long)bprm->security | bprm_flags);
+	}
 
-		if (complain && newprofile == null_complain_profile)
-			LOG_HINT(newprofile, GFP_ATOMIC, HINT_CHGPROF,
-				"pid=%d\n",
-				current->pid);
+	if (complain && new_profile == null_complain_profile) {
+		LOG_HINT(new_profile, GFP_ATOMIC, HINT_CHGPROF,
+			"pid=%d\n",
+			current->pid);
 	}
 
 cleanup:
 	aa_put_name_buffer(buffer);
-	aa_put_profile(profile);
-	if (IS_ERR(newprofile))
-		return PTR_ERR(newprofile);
-	aa_put_profile(newprofile);
+	if (IS_ERR(new_profile))
+		return PTR_ERR(new_profile);
+	aa_put_profile(new_profile);
 	return 0;
 }
 
@@ -1006,7 +952,6 @@ cleanup:
  */
 void aa_release(struct task_struct *task)
 {
-	struct aa_task_context *cxt = NULL;
 	struct aa_profile *profile;
 
 	/*
@@ -1024,10 +969,12 @@ void aa_release(struct task_struct *task
 repeat:
 	profile = aa_get_profile(task);
 	if (profile) {
+		struct aa_task_context *cxt;
+
 		lock_profile(profile);
 		task_lock(task);
 		cxt = aa_task_context(task);
-		if (unlikely(profile != cxt->profile)) {
+		if (unlikely(!cxt || cxt->profile != profile)) {
 			task_unlock(task);
 			unlock_profile(profile);
 			aa_put_profile(profile);
@@ -1042,8 +989,9 @@ repeat:
 		list_del(&cxt->list);
 		unlock_profile(profile);
 		aa_put_profile(profile);
+		/* FIXME: Also put the task context safely. */
 		kfree(cxt);
-		task->security = NULL;
+		rcu_assign_pointer(task->security, NULL);
 	}
 }
 
@@ -1074,14 +1022,16 @@ static inline int do_change_hat(const ch
 		/* change hat */
 		aa_change_profile(cxt, sub, hat_magic);
 	} else {
+		struct aa_profile *profile = cxt->profile;
+
 		if (APPARMOR_COMPLAIN(cxt)) {
-			LOG_HINT(cxt->profile, GFP_ATOMIC, HINT_UNKNOWN_HAT,
+			LOG_HINT(profile, GFP_ATOMIC, HINT_UNKNOWN_HAT,
  				"%s pid=%d "
 				"profile=%s active=%s\n",
 				hat_name,
 				current->pid,
-				cxt->profile->parent->name,
-				cxt->profile->name);
+				profile->parent->name,
+				profile->name);
 		} else {
 			AA_DEBUG("%s: Unknown hatname '%s'. "
 				"Changing to NULL profile "
@@ -1089,8 +1039,8 @@ static inline int do_change_hat(const ch
 				 __FUNCTION__,
 				 hat_name,
 				 current->comm, current->pid,
-				 cxt->profile->parent->name,
-				 cxt->profile->name);
+				 profile->parent->name,
+				 profile->name);
 			error = -EACCES;
 		}
 		/*
@@ -1119,57 +1069,49 @@ static inline int do_change_hat(const ch
  */
 int aa_change_hat(const char *hat_name, u32 hat_magic)
 {
-	struct aa_task_context *cxt = aa_task_context(current);
+	struct aa_task_context *cxt;
+	struct aa_profile *profile;
 	int error = 0;
 
-	AA_DEBUG("%s: %p, 0x%x (pid %d)\n",
-		 __FUNCTION__,
-		 hat_name, hat_magic,
-		 current->pid);
-
 	/* Dump out above debugging in WARN mode if we are in AUDIT mode */
-	if (APPARMOR_AUDIT(cxt)) {
+	if (APPARMOR_AUDIT(aa_task_context(current))) {
 		AA_WARN("%s: %s, 0x%x (pid %d)\n",
 			__FUNCTION__, hat_name ? hat_name : "NULL",
 			hat_magic, current->pid);
 	}
 
-	/* check to see if an unconfined process is doing a changehat. */
-	if (!cxt || !cxt->profile) {
-		error = -EPERM;
-		goto out;
+repeat:
+	profile = aa_get_profile(current);
+	if (!profile) {
+		/* An unconfined process cannot change_hat(). */
+		return -EPERM;
+	}
+	lock_profile(profile);
+	task_lock(current);
+	cxt = aa_task_context(current);
+	if (unlikely(profile->isstale || !cxt || cxt->profile != profile)) {
+		task_unlock(current);
+		unlock_profile(profile);
+		aa_put_profile(profile);
+		goto repeat;
 	}
 
 	/* check to see if the confined process has any hats. */
-	if (list_empty(&cxt->profile->parent->sub) &&
-	    !PROFILE_COMPLAIN(cxt->profile)) {
+	if (list_empty(&profile->parent->sub) && !PROFILE_COMPLAIN(profile)) {
 		error = -ECHILD;
 		goto out;
 	}
 
-	/* Check whether current domain is parent
-	 * or one of the sibling children
-	 */
-	if (cxt->profile != cxt->profile->parent) {
-		/*
-		 * parent
-		 */
+	if (profile != profile->parent) {
+		/* We are in the parent profile. */
 		if (hat_name) {
 			AA_DEBUG("%s: switching to %s, 0x%x\n",
 				 __FUNCTION__,
 				 hat_name,
 				 hat_magic);
-
-			/*
-			 * N.B hat_magic == 0 has a special meaning
-			 * this indicates that the task may never changehat
-			 * back to it's parent, it will stay in this subhat
-			 * (or null-profile, if the hat doesn't exist) until
-			 * the task terminates
-			 */
 			error = do_change_hat(hat_name, cxt, hat_magic);
 		} else {
-			/* Got here via changehat(NULL, magic)
+			/* Got here via change_hat(NULL, magic)
 			 *
 			 * We used to simply update the magic cookie.
 			 * That's an odd behaviour, so just do nothing.
@@ -1177,17 +1119,16 @@ int aa_change_hat(const char *hat_name, 
 		}
 	} else {
 		/*
-		 * child -- check to make sure magic is same as what was
-		 * passed when we switched into this profile,
-		 * Handle special casing of NULL magic which confines task
-		 * to subprofile and prohibits further changehats
+		 * We are in a child profile.
+		 *
+		 * Check to make sure magic is same as what was passed when
+		 * we switched into this profile.  Handle special casing of
+		 * NULL magic which confines task to subprofile and prohibits
+		 * further change_hats.
 		 */
 		if (hat_magic && hat_magic == cxt->hat_magic) {
 			if (!hat_name) {
-				/*
-				 * Got here via changehat(NULL, magic)
-				 * Return from subprofile, back to parent
-				 */
+				/* Return from subprofile back to parent. */
 				aa_change_profile(cxt, cxt->profile->parent, 0);
 			} else {
 				/*
@@ -1204,8 +1145,8 @@ int aa_change_hat(const char *hat_name, 
 				 current->comm, current->pid,
 				 hat_magic,
 				 hat_name ? hat_name : "NULL",
-				 cxt->profile->parent->name,
-				 cxt->profile->name);
+				 profile->parent->name,
+				 profile->name);
 
 			/* terminate current process */
 			(void)send_sig_info(SIGKILL, NULL, current);
@@ -1214,8 +1155,8 @@ int aa_change_hat(const char *hat_name, 
 				 "Task was confined to current subprofile "
 				 "(profile %s active %s)\n",
 				 current->comm, current->pid,
-				 cxt->profile->parent->name,
-				 cxt->profile->name);
+				 profile->parent->name,
+				 profile->name);
 
 			/* terminate current process */
 			(void)send_sig_info(SIGKILL, NULL, current);
@@ -1224,5 +1165,61 @@ int aa_change_hat(const char *hat_name, 
 	}
 
 out:
+	task_unlock(current);
+	unlock_profile(profile);
+	aa_put_profile(profile);
 	return error;
 }
+
+/**
+ * aa_replace_profile - replace a task's profile
+ */
+struct aa_profile *aa_replace_profile(struct task_struct *task,
+				      struct aa_profile *profile,
+				      u32 hat_magic)
+{
+	struct aa_task_context *cxt, *new_cxt = NULL;
+	struct aa_profile *old_profile;
+	int isstale;
+
+	/* If the task has no task context yet, allocate one. */
+	if (!aa_task_context(task)) {
+		new_cxt = aa_alloc_task_context(task);
+		if (!new_cxt)
+			return ERR_PTR(-ENOMEM);
+	}
+
+repeat:
+	old_profile = aa_get_profile(task);
+	lock_both_profiles(old_profile, profile);
+	task_lock(task);
+	cxt = aa_task_context(task);
+	isstale = (profile && profile->isstale);
+	if (unlikely((cxt && old_profile != cxt->profile) || isstale)) {
+		/*
+		 * Someone invalidated the profile we wanted to attach
+		 * to this task, or the task switched profiles before we
+		 * could get hold of the locks.
+		 */
+		task_unlock(task);
+		unlock_both_profiles(old_profile, profile);
+		aa_put_profile(old_profile);
+		old_profile = ERR_PTR(-ESTALE);
+		if (isstale)
+			goto out;
+		goto repeat;
+	}
+	if (!cxt) {
+		rcu_assign_pointer(task->security, new_cxt);
+		cxt = new_cxt;
+		new_cxt = NULL;
+	}
+	aa_change_profile(cxt, profile, hat_magic);
+	task_unlock(task);
+	unlock_both_profiles(old_profile, profile);
+
+out:
+	if (new_cxt)
+		aa_free_task_context(new_cxt);
+	return old_profile;
+}
Index: b/security/apparmor/apparmor.h
===================================================================
--- a/security/apparmor/apparmor.h
+++ b/security/apparmor/apparmor.h
@@ -149,7 +149,7 @@ struct aa_task_context {
 
 static inline struct aa_task_context *aa_task_context(struct task_struct *task)
 {
-	return (struct aa_task_context *)task->security;
+	return rcu_dereference((struct aa_task_context *)task->security);
 }
 
 extern struct aa_profile *null_complain_profile;
@@ -233,13 +233,16 @@ extern void aa_release(struct task_struc
 extern int aa_change_hat(const char *id, u32 hat_magic);
 extern struct aa_profile *__aa_find_profile(const char *name,
 					    struct list_head *list);
+extern struct aa_profile *aa_replace_profile(struct task_struct *task,
+					     struct aa_profile *profile,
+					     u32 hat_magic);
 
 /* list.c */
 extern void aa_profilelist_release(void);
 
 /* module_interface.c */
 extern ssize_t aa_file_prof_add(void *, size_t);
-extern ssize_t aa_file_prof_repl(void *, size_t);
+extern ssize_t aa_file_prof_replace(void *, size_t);
 extern ssize_t aa_file_prof_remove(const char *, size_t);
 extern void free_aa_profile(struct aa_profile *profile);
 extern void free_aa_profile_kref(struct kref *kref);
Index: b/security/apparmor/inline.h
===================================================================
--- a/security/apparmor/inline.h
+++ b/security/apparmor/inline.h
@@ -76,6 +76,10 @@ static inline void aa_change_profile(str
 	cxt->caps_logged = CAP_EMPTY_SET;
 	cxt->hat_magic = hat_magic;
 	rcu_assign_pointer(cxt->profile, aa_dup_profile(profile));
+	if (profile)
+		list_move(&cxt->list, &profile->task_contexts);
+	else
+		list_del_init(&cxt->list);
 	aa_put_profile(old_profile);
 }
 
@@ -204,6 +208,8 @@ static inline void lock_both_profiles(st
  *
  * The order in which profiles are passed into lock_both_profiles() /
  * unlock_both_profiles() does not matter.
+ * While the profile is locked, local interrupts are disabled. This also
+ * gives us RCU reader safety.
  */
 static inline void unlock_both_profiles(struct aa_profile *profile1,
 				        struct aa_profile *profile2)
Index: b/security/apparmor/lsm.c
===================================================================
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -756,6 +756,7 @@ static void __exit apparmor_exit(void)
 
 		/* Remove the profile from each task context it is on. */
 		lock_profile(profile);
+		profile->isstale = 1;
 		while (!list_empty(&profile->task_contexts)) {
 			struct aa_task_context *cxt =
 				list_entry(profile->task_contexts.next,
@@ -767,14 +768,9 @@ static void __exit apparmor_exit(void)
 			 */
 			task_lock(cxt->task);
 			rcu_assign_pointer(cxt->task->security, NULL);
-			task_unlock(cxt->task);
 			aa_change_profile(cxt, NULL, 0);
-
-			/*
-			 * Defer release: the task context may still have
-			 * active readers.
-			 */
-			list_move(&cxt->list, &task_contexts_bucket);
+			list_add(&cxt->list, &task_contexts_bucket);
+			task_unlock(cxt->task);
 		}
 		unlock_profile(profile);
 
Index: b/security/apparmor/module_interface.c
===================================================================
--- a/security/apparmor/module_interface.c
+++ b/security/apparmor/module_interface.c
@@ -42,7 +42,7 @@ static void free_aa_profile_rcu(struct r
  * task will be placed in the special null_profile state.
  */
 static inline void task_replace(struct aa_task_context *cxt,
-				struct aa_profile *new)
+				struct aa_profile *new_profile)
 {
 	AA_DEBUG("%s: replacing profile for task %s(%d) "
 		 "profile=%s (%p) hat=%s (%p)\n",
@@ -52,19 +52,19 @@ static inline void task_replace(struct a
 		 cxt->profile->name, cxt->profile);
 
 	if (cxt->profile != cxt->profile->parent) {
-		struct aa_profile *nactive;
+		struct aa_profile *hat;
 
 		/* The old profile was in a hat, check to see if the new
 		 * profile has an equivalent hat */
-		nactive = __aa_find_profile(cxt->profile->name, &new->sub);
+		hat = __aa_find_profile(cxt->profile->name, &new_profile->sub);
 
-		if (!nactive)
-			nactive = aa_dup_profile(new->null_profile);
+		if (!hat)
+			hat = aa_dup_profile(new_profile->null_profile);
 
-		aa_change_profile(cxt, nactive, cxt->hat_magic);
-		aa_put_profile(nactive);
+		aa_change_profile(cxt, hat, cxt->hat_magic);
+		aa_put_profile(hat);
 	} else
-		aa_change_profile(cxt, new, cxt->hat_magic);
+		aa_change_profile(cxt, new_profile, cxt->hat_magic);
 }
 
 static inline int aa_inbounds(struct aa_ext *e, size_t size)
@@ -434,7 +434,7 @@ ssize_t aa_file_prof_add(void *data, siz
 }
 
 /**
- * aa_file_prof_repl - replace a profile on the profile list
+ * aa_file_prof_replace - replace a profile on the profile list
  * @udata: serialized data stream
  * @size: size of the serialized data stream
  *
@@ -442,7 +442,7 @@ ssize_t aa_file_prof_add(void *data, siz
  * by any aa_task_context.  If the profile does not exist on the profile list
  * it is added.  Return %0 or error.
  */
-ssize_t aa_file_prof_repl(void *udata, size_t size)
+ssize_t aa_file_prof_replace(void *udata, size_t size)
 {
 	struct aa_profile *old_profile, *new_profile;
 	struct aa_ext e = {
@@ -459,22 +459,16 @@ ssize_t aa_file_prof_repl(void *udata, s
 	write_lock(&profile_list_lock);
 	old_profile = __aa_find_profile(new_profile->name, &profile_list);
 	if (old_profile) {
-		struct aa_task_context *cxt;
-
 		lock_profile(old_profile);
 		old_profile->isstale = 1;
-
-		list_for_each_entry(cxt, &old_profile->task_contexts, list) {
-			/*
-			 * Protect from racing with aa_release by taking
-			 * the task lock here.
-			 */
+		while (!list_empty(&old_profile->task_contexts)) {
+			struct aa_task_context *cxt =
+				list_entry(old_profile->task_contexts.next,
+					   struct aa_task_context, list);
 			task_lock(cxt->task);
 			task_replace(cxt, new_profile);
 			task_unlock(cxt->task);
 		}
-		list_splice_init(&old_profile->task_contexts,
-				 &new_profile->task_contexts);
 		unlock_profile(old_profile);
 
 		list_del_init(&old_profile->list);
@@ -517,7 +511,6 @@ ssize_t aa_file_prof_remove(const char *
 		list_del_init(&cxt->list);
 	}
 	unlock_profile(profile);
-
 	list_del_init(&profile->list);
 	write_unlock(&profile_list_lock);
 	aa_put_profile(profile);
Index: b/security/apparmor/procattr.c
===================================================================
--- a/security/apparmor/procattr.c
+++ b/security/apparmor/procattr.c
@@ -168,142 +168,85 @@ out:
 	return error;
 }
 
-int aa_setprocattr_setprofile(struct task_struct *task, char *profilename,
-			      size_t profilesize)
+int aa_setprocattr_setprofile(struct task_struct *task, char *name, size_t size)
 {
-	int error = -EINVAL;
-	struct aa_profile *profile = NULL;
-	struct aa_task_context *cxt;
-	char *name = NULL;
+	struct aa_profile *old_profile, *new_profile;
+	char *name_copy = NULL;
+	int error;
 
 	AA_DEBUG("%s: current %s(%d)\n",
 		 __FUNCTION__, current->comm, current->pid);
 
 	/* strip leading white space */
-	while (profilesize && isspace(*profilename)) {
-		profilename++;
-		profilesize--;
+	while (size && isspace(*name)) {
+		name++;
+		size--;
 	}
+	if (size == 0)
+		return -EINVAL;
 
-	if (profilesize == 0)
-		goto out;
-
-	/*
-	 * Copy string to a new buffer so we guarantee it is zero
-	 * terminated
-	 */
-	name = kmalloc(profilesize + 1, GFP_KERNEL);
-
-	if (!name) {
-		error = -ENOMEM;
-		goto out;
-	}
-
-	strncpy(name, profilename, profilesize);
-	name[profilesize] = 0;
-
- repeat:
-	if (strcmp(name, "unconstrained") != 0) {
-		profile = aa_find_profile(name);
-		if (!profile) {
+	/* Create a zero-terminated copy if the name. */
+	name_copy = kmalloc(size + 1, GFP_KERNEL);
+	if (!name_copy)
+		return -ENOMEM;
+
+	strncpy(name_copy, name, size);
+	name_copy[size] = 0;
+
+repeat:
+	if (strcmp(name_copy, "unconstrained") != 0) {
+		new_profile = aa_find_profile(name_copy);
+		if (!new_profile) {
 			AA_WARN("%s: Unable to switch task %s(%d) to profile"
 				"'%s'. No such profile.\n",
 				__FUNCTION__,
 				task->comm, task->pid,
-				name);
+				name_copy);
 
 			error = -EINVAL;
 			goto out;
 		}
-	}
+	} else
+		new_profile = NULL;
 
-	cxt = aa_task_context(task);
+	old_profile = aa_replace_profile(task, new_profile, 0);
+	if (IS_ERR(old_profile)) {
+		aa_put_profile(new_profile);
+		error = PTR_ERR(old_profile);
+		if (error == -ESTALE)
+			goto repeat;
+		goto out;
+	}
 
-	/* switch to unconstrained */
-	if (!profile) {
-		if (cxt && cxt->profile) {
+	if (new_profile) {
+		AA_WARN("%s: Switching task %s(%d) "
+			"profile %s active %s to new profile %s\n",
+			__FUNCTION__,
+			task->comm, task->pid,
+			old_profile ? old_profile->parent->name :
+				"unconstrained",
+			old_profile ? old_profile->name : "unconstrained",
+			name_copy);
+	} else {
+		if (old_profile) {
 			AA_WARN("%s: Unconstraining task %s(%d) "
 				"profile %s active %s\n",
 				__FUNCTION__,
 				task->comm, task->pid,
-				cxt->profile->parent->name,
-				cxt->profile->name);
-
-			aa_change_profile(cxt, NULL, 0);
+				old_profile->parent->name,
+				old_profile->name);
 		} else {
 			AA_WARN("%s: task %s(%d) "
 				"is already unconstrained\n",
 				__FUNCTION__, task->comm, task->pid);
 		}
-	} else {
-		if (!cxt) {
-			/* this task was created before module was
-			 * loaded, allocate a aa_task_context
-			 */
-			AA_WARN("%s: task %s(%d) has no aa_task_context\n",
-				__FUNCTION__, task->comm, task->pid);
-
-			cxt = aa_alloc_task_context(task);
-			if (!cxt) {
-				AA_WARN("%s: Unable to allocate "
-					"aa_task_context for task %s(%d). "
-					"Cannot confine task to profile %s\n",
-					__FUNCTION__,
-					task->comm, task->pid,
-					name);
-
-				error = -ENOMEM;
-				aa_put_profile(profile);
-
-				goto out;
-			}
-
-			if (!aa_task_context(task)) {
-				task->security = cxt;
-			} else { /* race */
-				aa_free_task_context(cxt);
-				cxt = aa_task_context(task);
-			}
-		}
-
-		/* ensure the profile hasn't been replaced */
-
-		if (unlikely(profile->isstale)) {
-			WARN_ON(profile == null_complain_profile);
-
-			/* drop refcnt obtained from earlier aa_dup_profile */
-			aa_put_profile(profile);
-			profile = aa_find_profile(name);
-
-			if (!profile) {
-				/* Race, profile was removed. */
-				goto repeat;
-			}
-		}
-
-		/* we do not do a normal task replace since we are not
-		 * replacing with the same profile.
-		 * If existing process is in a hat, it will be moved
-		 * into the new parent profile, even if this new
-		 * profile has a identical named hat.
-		 */
-
-		AA_WARN("%s: Switching task %s(%d) "
-			"profile %s active %s to new profile %s\n",
-			__FUNCTION__,
-			task->comm, task->pid,
-			cxt->profile ? cxt->profile->parent->name :
-				"unconstrained",
-			cxt->profile ? cxt->profile->name : "unconstrained",
-			name);
-
-		aa_change_profile(cxt, profile, 0);
-		aa_put_profile(profile);
 	}
 
+	aa_put_profile(old_profile);
+	aa_put_profile(new_profile);
 	error = 0;
-out:
-	kfree(name);
 
+out:
+	kfree(name_copy);
 	return error;
 }
Index: b/security/apparmor/apparmorfs.c
===================================================================
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -219,7 +219,7 @@ static ssize_t aa_profile_replace(struct
 	data = aa_simple_write_to_buffer(buf, size, size, pos, "replacement");
 
 	if (!IS_ERR(data)) {
-		error = aa_file_prof_repl(data, size);
+		error = aa_file_prof_replace(data, size);
 		vfree(data);
 	} else {
 		error = PTR_ERR(data);
Index: b/security/apparmor/locking.txt
===================================================================
--- /dev/null
+++ b/security/apparmor/locking.txt
@@ -0,0 +1,46 @@
+Locking in AppArmor
+===================
+
+Lock hierarchy:
+
+	profile_list_lock
+	  aa_profile->lock
+	    task_lock()
+
+
+Which lock protects what?
+
+	/-----------------------+-------------------------------\
+	| Variable		| Lock				|
+	>-----------------------+-------------------------------<
+	| profile_list,		| profile_list_lock		|
+	+-----------------------+-------------------------------+
+	| aa_profile->		| aa_profile->lock		|
+	|   isstale,		|				|
+	|   task_contexts	|				|
+	| aa_profile->count	| RCU				|
+	+-----------------------+-------------------------------+
+	| aa_task_context->	|				|
+	|   profile		| read: RCU			|
+	|			| write: aa_profile->lock +	|
+	|			|	 task_lock()		|
+	+-----------------------+-------------------------------+
+	| task_struct->security	| read: RCU			|
+	|			| write: task_lock()		|
+	+-----------------------+-------------------------------+
+	| aa_profile->sub	| handle on the profile (list	|
+	|			| is never modified)		|
+	\-----------------------+-------------------------------/
+
+(Obviously, the list_heads embedded in data structures are always
+protected with the lock that also protects the list.)
+
+When moving a task context from one profile to another, we grab both
+profile locks with lock_both_profiles(). This ensures that both locks
+are always taken in the same order, and so we won't deadlock.
+
+Since aa_task_struct->profile is RCU protected, it can change under a
+reader at any time. Therefore, we should grab the pointer and use the
+cached result, but we can only do this after all blocking operations (or
+else the pointer could just change again). The ->profile pointer may
+change or become NULL at any time; we must be careful about this.