apparmor: move task specific domain change info out of cred

Commit Message

John Johansen Feb. 1, 2017, 11:17 a.m. UTC

Now that security_task_alloc() hook and "struct task_struct"->security
field are revived, move task specific domain change information for
change_onexec (equiv of setexeccon) and change_hat out of the cred
into a context off of the task_struct.

This cleans up apparmor's use of the cred structure so that it only
carries the reference to current mediation.

Signed-off-by: John Johansen <[e-mail]>
---
 security/apparmor/context.c         | 129 +++++++++++++++++-------------------
 security/apparmor/domain.c          |  28 ++++----
 security/apparmor/include/context.h |  63 ++++++++----------
 security/apparmor/lsm.c             |  65 ++++++++++--------
 security/apparmor/policy.c          |   5 +-
 5 files changed, 143 insertions(+), 147 deletions(-)

Patch

diff --git a/security/apparmor/context.c b/security/apparmor/context.c
index 1fc16b8..8afb304 100644
--- a/security/apparmor/context.c
+++ b/security/apparmor/context.c
@@ -13,11 +13,9 @@ 
  * License.
  *
  *
- * AppArmor sets confinement on every task, via the the aa_task_ctx and
- * the aa_task_ctx.profile, both of which are required and are not allowed
- * to be NULL.  The aa_task_ctx is not reference counted and is unique
- * to each cred (which is reference count).  The profile pointed to by
- * the task_ctx is reference counted.
+ * AppArmor sets confinement on every task, via the cred_profile() which
+ * is required and is not allowed to be NULL.  The cred_profile is
+ * reference counted.
  *
  * TODO
  * If a task uses change_hat it currently does not return to the old
@@ -29,25 +27,42 @@ 
 #include "include/context.h"
 #include "include/policy.h"
 
+
+/**
+ * aa_get_task_profile - Get another task's profile
+ * @task: task to query  (NOT NULL)
+ *
+ * Returns: counted reference to @task's profile
+ */
+struct aa_profile *aa_get_task_profile(struct task_struct *task)
+{
+	struct aa_profile *p;
+
+	rcu_read_lock();
+	p = aa_get_profile(__aa_task_profile(task));
+	rcu_read_unlock();
+
+	return p;
+}
+
 /**
- * aa_alloc_task_context - allocate a new task_ctx
+ * aa_alloc_task_ctx - allocate a new task_ctx
  * @flags: gfp flags for allocation
  *
  * Returns: allocated buffer or NULL on failure
  */
-struct aa_task_ctx *aa_alloc_task_context(gfp_t flags)
+struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags)
 {
 	return kzalloc(sizeof(struct aa_task_ctx), flags);
 }
 
 /**
- * aa_free_task_context - free a task_ctx
+ * aa_free_task_ctx - free a task_ctx
  * @ctx: task_ctx to free (MAYBE NULL)
  */
-void aa_free_task_context(struct aa_task_ctx *ctx)
+void aa_free_task_ctx(struct aa_task_ctx *ctx)
 {
 	if (ctx) {
-		aa_put_profile(ctx->profile);
 		aa_put_profile(ctx->previous);
 		aa_put_profile(ctx->onexec);
 
@@ -56,36 +71,18 @@  void aa_free_task_context(struct aa_task_ctx *ctx)
 }
 
 /**
- * aa_dup_task_context - duplicate a task context, incrementing reference counts
+ * aa_dup_task_ctx - duplicate a task context, incrementing reference counts
  * @new: a blank task context      (NOT NULL)
  * @old: the task context to copy  (NOT NULL)
  */
-void aa_dup_task_context(struct aa_task_ctx *new, const struct aa_task_ctx *old)
+void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old)
 {
 	*new = *old;
-	aa_get_profile(new->profile);
 	aa_get_profile(new->previous);
 	aa_get_profile(new->onexec);
 }
 
 /**
- * aa_get_task_profile - Get another task's profile
- * @task: task to query  (NOT NULL)
- *
- * Returns: counted reference to @task's profile
- */
-struct aa_profile *aa_get_task_profile(struct task_struct *task)
-{
-	struct aa_profile *p;
-
-	rcu_read_lock();
-	p = aa_get_profile(__aa_task_profile(task));
-	rcu_read_unlock();
-
-	return p;
-}
-
-/**
  * aa_replace_current_profile - replace the current tasks profiles
  * @profile: new profile  (NOT NULL)
  *
@@ -93,36 +90,37 @@  struct aa_profile *aa_get_task_profile(struct task_struct *task)
  */
 int aa_replace_current_profile(struct aa_profile *profile)
 {
-	struct aa_task_ctx *ctx = current_ctx();
+	struct aa_profile *old = __aa_current_profile();
 	struct cred *new;
+
 	AA_BUG(!profile);
 
-	if (ctx->profile == profile)
+	if (old == profile)
 		return 0;
 
 	if (current_cred() != current_real_cred())
 		return -EBUSY;
 
 	new  = prepare_creds();
+	old = cred_profile(new);
 	if (!new)
 		return -ENOMEM;
 
-	ctx = cred_ctx(new);
-	if (unconfined(profile) || (ctx->profile->ns != profile->ns))
+	if (unconfined(profile) || (old->ns != profile->ns))
 		/* if switching to unconfined or a different profile namespace
 		 * clear out context state
 		 */
-		aa_clear_task_ctx_trans(ctx);
+		aa_clear_task_ctx(current_task_ctx());
 
 	/*
-	 * be careful switching ctx->profile, when racing replacement it
-	 * is possible that ctx->profile->proxy->profile is the reference
+	 * be careful switching cred profile, when racing replacement it
+	 * is possible that the cred profile's->proxy->profile is the reference
 	 * keeping @profile valid, so make sure to get its reference before
-	 * dropping the reference on ctx->profile
+	 * dropping the reference on the cred's profile
 	 */
 	aa_get_profile(profile);
-	aa_put_profile(ctx->profile);
-	ctx->profile = profile;
+	aa_put_profile(old);
+	cred_profile(new) = profile;
 
 	commit_creds(new);
 	return 0;
@@ -137,16 +135,12 @@  int aa_replace_current_profile(struct aa_profile *profile)
 int aa_set_current_onexec(struct aa_profile *profile)
 {
 	struct aa_task_ctx *ctx;
-	struct cred *new = prepare_creds();
-	if (!new)
-		return -ENOMEM;
 
-	ctx = cred_ctx(new);
+	ctx = current_task_ctx();
 	aa_get_profile(profile);
 	aa_put_profile(ctx->onexec);
 	ctx->onexec = profile;
 
-	commit_creds(new);
 	return 0;
 }
 
@@ -162,25 +156,28 @@  int aa_set_current_onexec(struct aa_profile *profile)
  */
 int aa_set_current_hat(struct aa_profile *profile, u64 token)
 {
-	struct aa_task_ctx *ctx;
-	struct cred *new = prepare_creds();
+	struct aa_task_ctx *ctx = current_task_ctx();
+	struct cred *new;
+
+	AA_BUG(!profile);
+
+	new = prepare_creds();
 	if (!new)
 		return -ENOMEM;
-	AA_BUG(!profile);
 
-	ctx = cred_ctx(new);
 	if (!ctx->previous) {
 		/* transfer refcount */
-		ctx->previous = ctx->profile;
+		ctx->previous = cred_profile(new);
 		ctx->token = token;
 	} else if (ctx->token == token) {
-		aa_put_profile(ctx->profile);
+		aa_put_profile(cred_profile(new));
 	} else {
 		/* previous_profile && ctx->token != token */
 		abort_creds(new);
 		return -EACCES;
 	}
-	ctx->profile = aa_get_newest_profile(profile);
+
+	cred_profile(new) = aa_get_newest_profile(profile);
 	/* clear exec on switching context */
 	aa_put_profile(ctx->onexec);
 	ctx->onexec = NULL;
@@ -200,28 +197,26 @@  int aa_set_current_hat(struct aa_profile *profile, u64 token)
  */
 int aa_restore_previous_profile(u64 token)
 {
-	struct aa_task_ctx *ctx;
-	struct cred *new = prepare_creds();
-	if (!new)
-		return -ENOMEM;
+	struct aa_task_ctx *ctx = current_task_ctx();
+	struct cred *new;
 
-	ctx = cred_ctx(new);
-	if (ctx->token != token) {
-		abort_creds(new);
+	if (ctx->token != token)
 		return -EACCES;
-	}
 	/* ignore restores when there is no saved profile */
-	if (!ctx->previous) {
-		abort_creds(new);
+	if (!ctx->previous)
 		return 0;
-	}
 
-	aa_put_profile(ctx->profile);
-	ctx->profile = aa_get_newest_profile(ctx->previous);
-	AA_BUG(!ctx->profile);
+	new = prepare_creds();
+	if (!new)
+		return -ENOMEM;
+
+	aa_put_profile(cred_profile(new));
+	cred_profile(new) = aa_get_newest_profile(ctx->previous);
+	AA_BUG(!cred_profile(new));
 	/* clear exec && prev information when restoring to previous context */
-	aa_clear_task_ctx_trans(ctx);
+	aa_clear_task_ctx(ctx);
 
 	commit_creds(new);
+
 	return 0;
 }
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index ef4beef..1994c02 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -353,10 +353,10 @@  int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 	if (bprm->cred_prepared)
 		return 0;
 
-	ctx = cred_ctx(bprm->cred);
+	ctx = current_task_ctx();
 	AA_BUG(!ctx);
-
-	profile = aa_get_newest_profile(ctx->profile);
+	AA_BUG(!cred_profile(bprm->cred));
+	profile = aa_get_newest_profile(cred_profile(bprm->cred));
 	/*
 	 * get the namespace from the replacement profile as replacement
 	 * can change the namespace
@@ -499,14 +499,11 @@  int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 	bprm->per_clear |= PER_CLEAR_ON_SETID;
 
 x_clear:
-	aa_put_profile(ctx->profile);
-	/* transfer new profile reference will be released when ctx is freed */
-	ctx->profile = new_profile;
+	aa_put_profile(profile);
+	/* transfer new profile reference will be released when cred is freed */
+	cred_profile(bprm->cred) = new_profile;
 	new_profile = NULL;
 
-	/* clear out all temporary/transitional state from the context */
-	aa_clear_task_ctx_trans(ctx);
-
 audit:
 	error = aa_audit_file(profile, &perms, OP_EXEC, MAY_EXEC, name,
 			      new_profile ? new_profile->base.hname : NULL,
@@ -544,17 +541,16 @@  int apparmor_bprm_secureexec(struct linux_binprm *bprm)
 void apparmor_bprm_committing_creds(struct linux_binprm *bprm)
 {
 	struct aa_profile *profile = __aa_current_profile();
-	struct aa_task_ctx *new_ctx = cred_ctx(bprm->cred);
+	struct aa_profile *new = cred_profile(bprm->cred);
 
 	/* bail out if unconfined or not changing profile */
-	if ((new_ctx->profile == profile) ||
-	    (unconfined(new_ctx->profile)))
+	if (new == profile || unconfined(new))
 		return;
 
 	current->pdeath_signal = 0;
 
 	/* reset soft limits and set hard limits for the new profile */
-	__aa_transition_rlimits(profile, new_ctx->profile);
+	__aa_transition_rlimits(profile, new);
 }
 
 /**
@@ -564,6 +560,10 @@  void apparmor_bprm_committing_creds(struct linux_binprm *bprm)
 void apparmor_bprm_committed_creds(struct linux_binprm *bprm)
 {
 	/* TODO: cleanup signals - ipc mediation */
+
+	/* clear out all temporary/transitional state from the context */
+	aa_clear_task_ctx(current_task_ctx());
+
 	return;
 }
 
@@ -621,7 +621,7 @@  int aa_change_hat(const char *hats[], int count, u64 token, bool permtest)
 
 	/* released below */
 	cred = get_current_cred();
-	ctx = cred_ctx(cred);
+	ctx = current_task_ctx();
 	profile = aa_get_newest_profile(aa_cred_profile(cred));
 	previous_profile = aa_get_newest_profile(ctx->previous);
 
diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h
index 5b18fed..9943969 100644
--- a/security/apparmor/include/context.h
+++ b/security/apparmor/include/context.h
@@ -22,8 +22,9 @@ 
 #include "policy.h"
 #include "policy_ns.h"
 
-#define cred_ctx(X) ((X)->security)
-#define current_ctx() cred_ctx(current_cred())
+#define task_ctx(X) ((X)->security)
+#define current_task_ctx() (task_ctx(current))
+#define cred_profile(X) ((X)->security)
 
 /* struct aa_file_ctx - the AppArmor context the file was opened in
  * @perms: the permission the file was opened with
@@ -58,28 +59,23 @@  static inline void aa_free_file_context(struct aa_file_ctx *ctx)
 }
 
 /**
- * struct aa_task_ctx - primary label for confined tasks
- * @profile: the current profile   (NOT NULL)
- * @exec: profile to transition to on next exec  (MAYBE NULL)
- * @previous: profile the task may return to     (MAYBE NULL)
+ * struct aa_task_ctx - information for current task label change
+ * @onexec: profile to transition to on next exec  (MAY BE NULL)
+ * @previous: profile the task may return to     (MAY BE NULL)
  * @token: magic value the task must know for returning to @previous_profile
  *
- * Contains the task's current profile (which could change due to
- * change_hat).  Plus the hat_magic needed during change_hat.
- *
  * TODO: make so a task can be confined by a stack of contexts
  */
 struct aa_task_ctx {
-	struct aa_profile *profile;
 	struct aa_profile *onexec;
 	struct aa_profile *previous;
 	u64 token;
 };
 
-struct aa_task_ctx *aa_alloc_task_context(gfp_t flags);
-void aa_free_task_context(struct aa_task_ctx *ctx);
-void aa_dup_task_context(struct aa_task_ctx *new,
-			 const struct aa_task_ctx *old);
+struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags);
+void aa_free_task_ctx(struct aa_task_ctx *ctx);
+void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old);
+
 int aa_replace_current_profile(struct aa_profile *profile);
 int aa_set_current_onexec(struct aa_profile *profile);
 int aa_set_current_hat(struct aa_profile *profile, u64 token);
@@ -97,10 +93,11 @@  struct aa_profile *aa_get_task_profile(struct task_struct *task);
  */
 static inline struct aa_profile *aa_cred_profile(const struct cred *cred)
 {
-	struct aa_task_ctx *ctx = cred_ctx(cred);
+	struct aa_profile *profile = cred_profile(cred);
 
-	AA_BUG(!ctx || !ctx->profile);
-	return ctx->profile;
+	AA_BUG(!profile);
+
+	return profile;
 }
 
 /**
@@ -117,17 +114,6 @@  static inline struct aa_profile *__aa_task_profile(struct task_struct *task)
 }
 
 /**
- * __aa_task_is_confined - determine if @task has any confinement
- * @task: task to check confinement of  (NOT NULL)
- *
- * If @task != current needs to be called in RCU safe critical section
- */
-static inline bool __aa_task_is_confined(struct task_struct *task)
-{
-	return !unconfined(__aa_task_profile(task));
-}
-
-/**
  * __aa_current_profile - find the current tasks confining profile
  *
  * Returns: up to date confining profile or the ns unconfined profile (NOT NULL)
@@ -150,19 +136,17 @@  static inline struct aa_profile *__aa_current_profile(void)
  */
 static inline struct aa_profile *aa_current_profile(void)
 {
-	const struct aa_task_ctx *ctx = current_ctx();
-	struct aa_profile *profile;
+	struct aa_profile *profile = __aa_current_profile();
 
-	AA_BUG(!ctx || !ctx->profile);
+	AA_BUG(!profile);
 
-	if (profile_is_stale(ctx->profile)) {
-		profile = aa_get_newest_profile(ctx->profile);
+	if (profile_is_stale(profile)) {
+		profile = aa_get_newest_profile(profile);
 		aa_replace_current_profile(profile);
 		aa_put_profile(profile);
-		ctx = current_ctx();
 	}
 
-	return ctx->profile;
+	return profile;
 }
 
 static inline struct aa_ns *aa_get_current_ns(void)
@@ -170,12 +154,17 @@  static inline struct aa_ns *aa_get_current_ns(void)
 	return aa_get_ns(__aa_current_profile()->ns);
 }
 
+
+
+
 /**
- * aa_clear_task_ctx_trans - clear transition tracking info from the ctx
+ * aa_clear_task_ctx - clear transition tracking info from the ctx
  * @ctx: task context to clear (NOT NULL)
  */
-static inline void aa_clear_task_ctx_trans(struct aa_task_ctx *ctx)
+static inline void aa_clear_task_ctx(struct aa_task_ctx *ctx)
 {
+	AA_BUG(!ctx);
+
 	aa_put_profile(ctx->previous);
 	aa_put_profile(ctx->onexec);
 	ctx->previous = NULL;
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 1f2000d..ed9bf71 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -49,12 +49,12 @@  DEFINE_PER_CPU(struct aa_buffers, aa_buffers);
  */
 
 /*
- * free the associated aa_task_ctx and put its profiles
+ * put the associated profiles
  */
 static void apparmor_cred_free(struct cred *cred)
 {
-	aa_free_task_context(cred_ctx(cred));
-	cred_ctx(cred) = NULL;
+	aa_put_profile(cred_profile(cred));
+	cred_profile(cred) = NULL;
 }
 
 /*
@@ -62,30 +62,19 @@  static void apparmor_cred_free(struct cred *cred)
  */
 static int apparmor_cred_alloc_blank(struct cred *cred, gfp_t gfp)
 {
-	/* freed by apparmor_cred_free */
-	struct aa_task_ctx *ctx = aa_alloc_task_context(gfp);
-
-	if (!ctx)
-		return -ENOMEM;
-
-	cred_ctx(cred) = ctx;
+	cred_profile(cred) = NULL;
 	return 0;
 }
 
 /*
- * prepare new aa_task_ctx for modification by prepare_cred block
+ * prepare new cred profile for modification by prepare_cred block
  */
 static int apparmor_cred_prepare(struct cred *new, const struct cred *old,
 				 gfp_t gfp)
 {
-	/* freed by apparmor_cred_free */
-	struct aa_task_ctx *ctx = aa_alloc_task_context(gfp);
-
-	if (!ctx)
-		return -ENOMEM;
-
-	aa_dup_task_context(ctx, cred_ctx(old));
-	cred_ctx(new) = ctx;
+	struct aa_profile *tmp = cred_profile(new);
+	cred_profile(new) = aa_get_newest_profile(cred_profile(old));
+	aa_put_profile(tmp);
 	return 0;
 }
 
@@ -94,10 +83,30 @@  static int apparmor_cred_prepare(struct cred *new, const struct cred *old,
  */
 static void apparmor_cred_transfer(struct cred *new, const struct cred *old)
 {
-	const struct aa_task_ctx *old_ctx = cred_ctx(old);
-	struct aa_task_ctx *new_ctx = cred_ctx(new);
+	struct aa_profile *tmp = cred_profile(new);
+	cred_profile(new) = aa_get_newest_profile(cred_profile(old));
+	aa_put_profile(tmp);
+}
 
-	aa_dup_task_context(new_ctx, old_ctx);
+static void apparmor_task_free(struct task_struct *task)
+{
+
+	aa_free_task_ctx(task_ctx(task));
+	task_ctx(task) = NULL;
+}
+
+static int apparmor_task_alloc(struct task_struct *task,
+			       unsigned long clone_flags)
+{
+	struct aa_task_ctx *new = aa_alloc_task_ctx(GFP_KERNEL);
+
+	if (!new)
+		return -ENOMEM;
+
+	aa_dup_task_ctx(new, current_task_ctx());
+	task_ctx(task) = new;
+
+	return 0;
 }
 
 static int apparmor_ptrace_access_check(struct task_struct *child,
@@ -484,11 +493,11 @@  static int apparmor_getprocattr(struct task_struct *task, char *name,
 	int error = -ENOENT;
 	/* released below */
 	const struct cred *cred = get_task_cred(task);
-	struct aa_task_ctx *ctx = cred_ctx(cred);
+	struct aa_task_ctx *ctx = current_task_ctx();
 	struct aa_profile *profile = NULL;
 
 	if (strcmp(name, "current") == 0)
-		profile = aa_get_newest_profile(ctx->profile);
+		profile = aa_get_newest_profile(cred_profile(cred));
 	else if (strcmp(name, "prev") == 0  && ctx->previous)
 		profile = aa_get_newest_profile(ctx->previous);
 	else if (strcmp(name, "exec") == 0 && ctx->onexec)
@@ -629,6 +638,8 @@  static struct security_hook_list apparmor_hooks[] = {
 	LSM_HOOK_INIT(bprm_committed_creds, apparmor_bprm_committed_creds),
 	LSM_HOOK_INIT(bprm_secureexec, apparmor_bprm_secureexec),
 
+	LSM_HOOK_INIT(task_free, apparmor_task_free),
+	LSM_HOOK_INIT(task_alloc, apparmor_task_alloc),
 	LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit),
 };
 
@@ -871,12 +882,12 @@  static int __init set_init_ctx(void)
 	struct cred *cred = (struct cred *)current->real_cred;
 	struct aa_task_ctx *ctx;
 
-	ctx = aa_alloc_task_context(GFP_KERNEL);
+	ctx = aa_alloc_task_ctx(GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
 
-	ctx->profile = aa_get_profile(root_ns->unconfined);
-	cred_ctx(cred) = ctx;
+	cred_profile(cred) = aa_get_profile(root_ns->unconfined);
+	task_ctx(current) = ctx;
 
 	return 0;
 }
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index f44312a..b9c300b 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -827,8 +827,9 @@  static int __lookup_replace(struct aa_ns *ns, const char *hname,
  * @udata: serialized data stream  (NOT NULL)
  *
  * unpack and replace a profile on the profile list and uses of that profile
- * by any aa_task_ctx.  If the profile does not exist on the profile list
- * it is added.
+ * by any task creds via invalidating the old version of the profile, which
+ * tasks will notice to update their own cred.  If the profile does not exist
+ * on the profile list it is added.
  *
  * Returns: size of data consumed else error code on failure.
  */