apparmor: convert to profile block critical sections

There are still a few places where profile replacement fails to update
and a stale profile is used for mediation. Fix this by moving to
accessing the current label through a critical section that will
always ensure mediation is using the current label regardless of
whether the tasks cred has been updated or not.

Signed-off-by: John Johansen <john.johansen@canonical.com>

1 files changed, 100 insertions, 23 deletions

diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h
index 420cfd041218..7665fae7131f 100644
--- a/security/apparmor/include/context.h
+++ b/security/apparmor/include/context.h
@@ -56,14 +56,14 @@ struct aa_profile *aa_get_task_profile(struct task_struct *task);
 
 
 /**
- * aa_cred_profile - obtain cred's profiles
+ * aa_cred_raw_profile - obtain cred's profiles
  * @cred: cred to obtain profiles from  (NOT NULL)
  *
  * Returns: confining profile
  *
  * does NOT increment reference count
  */
-static inline struct aa_profile *aa_cred_profile(const struct cred *cred)
+static inline struct aa_profile *aa_cred_raw_profile(const struct cred *cred)
 {
 	struct aa_task_ctx *ctx = cred_ctx(cred);
 
@@ -72,16 +72,28 @@ static inline struct aa_profile *aa_cred_profile(const struct cred *cred)
 }
 
 /**
- * __aa_task_profile - retrieve another task's profile
+ * aa_get_newest_cred_profile - obtain the newest profile on a cred
+ * @cred: cred to obtain profile from (NOT NULL)
+ *
+ * Returns: newest version of confining profile
+ */
+static inline
+struct aa_profile *aa_get_newest_cred_profile(const struct cred *cred)
+{
+	return aa_get_newest_profile(aa_cred_raw_profile(cred));
+}
+
+/**
+ * __aa_task_raw_profile - retrieve another task's profile
  * @task: task to query  (NOT NULL)
  *
  * Returns: @task's profile without incrementing its ref count
  *
  * If @task != current needs to be called in RCU safe critical section
  */
-static inline struct aa_profile *__aa_task_profile(struct task_struct *task)
+static inline struct aa_profile *__aa_task_raw_profile(struct task_struct *task)
 {
-	return aa_cred_profile(__task_cred(task));
+	return aa_cred_raw_profile(__task_cred(task));
 }
 
 /**
@@ -92,50 +104,115 @@ static inline struct aa_profile *__aa_task_profile(struct task_struct *task)
  */
 static inline bool __aa_task_is_confined(struct task_struct *task)
 {
-	return !unconfined(__aa_task_profile(task));
+	return !unconfined(__aa_task_raw_profile(task));
 }
 
 /**
- * __aa_current_profile - find the current tasks confining profile
+ * aa_current_raw_profile - find the current tasks confining profile
  *
  * Returns: up to date confining profile or the ns unconfined profile (NOT NULL)
  *
  * This fn will not update the tasks cred to the most up to date version
  * of the profile so it is safe to call when inside of locks.
  */
-static inline struct aa_profile *__aa_current_profile(void)
+static inline struct aa_profile *aa_current_raw_profile(void)
 {
-	return aa_cred_profile(current_cred());
+	return aa_cred_raw_profile(current_cred());
 }
 
 /**
- * aa_current_profile - find the current tasks confining profile and do updates
+ * aa_get_current_profile - get the newest version of the current tasks profile
  *
- * Returns: up to date confining profile or the ns unconfined profile (NOT NULL)
+ * Returns: newest version of confining profile (NOT NULL)
+ *
+ * This fn will not update the tasks cred, so it is safe inside of locks
  *
- * This fn will update the tasks cred structure if the profile has been
- * replaced.  Not safe to call inside locks
+ * The returned reference must be put with aa_put_profile()
  */
-static inline struct aa_profile *aa_current_profile(void)
+static inline struct aa_profile *aa_get_current_profile(void)
 {
-	const struct aa_task_ctx *ctx = current_ctx();
-	struct aa_profile *profile;
+	struct aa_profile *p = aa_current_raw_profile();
 
-	AA_BUG(!ctx || !ctx->profile);
+	if (profile_is_stale(p))
+		return aa_get_newest_profile(p);
+	return aa_get_profile(p);
+}
 
-	if (profile_is_stale(ctx->profile)) {
-		profile = aa_get_newest_profile(ctx->profile);
-		aa_replace_current_profile(profile);
+#define __end_current_profile_crit_section(X) \
+	end_current_profile_crit_section(X)
+
+/**
+ * end_profile_crit_section - put a reference found with begin_current_profile..
+ * @profile: profile reference to put
+ *
+ * Should only be used with a reference obtained with
+ * begin_current_profile_crit_section and never used in situations where the
+ * task cred may be updated
+ */
+static inline void end_current_profile_crit_section(struct aa_profile *profile)
+{
+	if (profile != aa_current_raw_profile())
 		aa_put_profile(profile);
-		ctx = current_ctx();
+}
+
+/**
+ * __begin_current_profile_crit_section - current's confining profile
+ *
+ * Returns: up to date confining profile or the ns unconfined profile (NOT NULL)
+ *
+ * safe to call inside locks
+ *
+ * The returned reference must be put with __end_current_profile_crit_section()
+ * This must NOT be used if the task cred could be updated within the
+ * critical section between __begin_current_profile_crit_section() ..
+ * __end_current_profile_crit_section()
+ */
+static inline struct aa_profile *__begin_current_profile_crit_section(void)
+{
+	struct aa_profile *profile = aa_current_raw_profile();
+
+	if (profile_is_stale(profile))
+		profile = aa_get_newest_profile(profile);
+
+	return profile;
+}
+
+/**
+ * begin_current_profile_crit_section - current's profile and update if needed
+ *
+ * Returns: up to date confining profile or the ns unconfined profile (NOT NULL)
+ *
+ * Not safe to call inside locks
+ *
+ * The returned reference must be put with end_current_profile_crit_section()
+ * This must NOT be used if the task cred could be updated within the
+ * critical section between begin_current_profile_crit_section() ..
+ * end_current_profile_crit_section()
+ */
+static inline struct aa_profile *begin_current_profile_crit_section(void)
+{
+	struct aa_profile *profile = aa_current_raw_profile();
+
+	if (profile_is_stale(profile)) {
+		profile = aa_get_newest_profile(profile);
+		if (aa_replace_current_profile(profile) == 0)
+			/* task cred will keep the reference */
+			aa_put_profile(profile);
 	}
 
-	return ctx->profile;
+	return profile;
 }
 
 static inline struct aa_ns *aa_get_current_ns(void)
 {
-	return aa_get_ns(__aa_current_profile()->ns);
+	struct aa_profile *profile;
+	struct aa_ns *ns;
+
+	profile  = __begin_current_profile_crit_section();
+	ns = aa_get_ns(profile->ns);
+	__end_current_profile_crit_section(profile);
+
+	return ns;
 }
 
 /**