[trusty] [PATCH] UBUNTU: SAUCE: (no-up) apparmor: Fix tasks not subject to, reloaded policy

John Johansen john.johansen at canonical.com
Thu Nov 14 23:19:09 UTC 2013


This patch is against code that is not upstream.

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

When profile replacement is used the cred may not be updated for various
reasons (cached, locking ...). In these cases directly using the cred
without checking for an updated profile is wrong, and can result in
inconsistent application of old vs. new policy. This is not just an issue
of when the new policy takes over from the old but both old and new being
applied simultaneously in inconsistent ways.

Signed-off-by: John Johansen <john.johansen at canonical.com>
---
 security/apparmor/domain.c          |  7 +++++--
 security/apparmor/include/context.h | 11 +++++++++++
 security/apparmor/include/policy.h  |  1 -
 security/apparmor/lsm.c             | 18 ++++++++++++------
 security/apparmor/resource.c        |  2 +-
 5 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 8cc0472..90f5c87 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -641,7 +641,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest)
 	/* released below */
 	cred = get_current_cred();
 	cxt = cred_cxt(cred);
-	label = aa_cred_label(cred);
+	label = aa_get_newest_cred_label(cred);
 	previous = cxt->previous;
 
 	profile = labels_profile(label);
@@ -739,6 +739,7 @@ audit:
 
 out:
 	aa_put_profile(hat);
+	aa_put_label(label);
 	kfree(name);
 	put_cred(cred);
 
@@ -784,7 +785,7 @@ int aa_change_profile(const char *ns_name, const char *hname, bool onexec,
 	}
 
 	cred = get_current_cred();
-	label = aa_cred_label(cred);
+	label = aa_get_newest_cred_label(cred);
 	profile = labels_profile(label);
 
 	/*
@@ -795,6 +796,7 @@ int aa_change_profile(const char *ns_name, const char *hname, bool onexec,
 	 * of permissions.
 	 */
 	if (current->no_new_privs && !unconfined(label)) {
+		aa_put_label(label);
 		put_cred(cred);
 		return -EPERM;
 	}
@@ -866,6 +868,7 @@ audit:
 
 	aa_put_namespace(ns);
 	aa_put_profile(target);
+	aa_put_label(label);
 	put_cred(cred);
 
 	return error;
diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h
index 7641967..c3d8fe4 100644
--- a/security/apparmor/include/context.h
+++ b/security/apparmor/include/context.h
@@ -103,6 +103,17 @@ static inline struct aa_label *aa_cred_label(const struct cred *cred)
 }
 
 /**
+ * aa_get_newest_cred_label - obtain the newest version of the label on a cred
+ * @cred: cred to obtain label from (NOT NULL)
+ *
+ * Returns: newest version of confining label
+ */
+static inline struct aa_label *aa_get_newest_cred_label(const struct cred *cred)
+{
+	return aa_get_newest_label(aa_cred_label(cred));
+}
+
+/**
  * __aa_task_label - retrieve another task's label
  * @task: task to query  (NOT NULL)
  *
diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index 09d3ccf..f5053da 100644
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -255,7 +255,6 @@ static inline struct aa_profile *aa_get_newest_profile(struct aa_profile *p)
 	return labels_profile(aa_get_newest_label(&p->label));
 }
 
-
 static inline struct aa_profile *aa_deref_parent(struct aa_profile *p)
 {
 	return rcu_dereference_protected(p->parent,
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index b612c10..2a69990 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -126,7 +126,7 @@ static int apparmor_capget(struct task_struct *target, kernel_cap_t *effective,
 
 	rcu_read_lock();
 	cred = __task_cred(target);
-	label = aa_cred_label(cred);
+	label = aa_get_newest_cred_label(cred);
 
 	*effective = cred->cap_effective;
 	*inheritable = cred->cap_inheritable;
@@ -145,6 +145,7 @@ static int apparmor_capget(struct task_struct *target, kernel_cap_t *effective,
 		}
 	}
 	rcu_read_unlock();
+	aa_put_label(label);
 
 	return 0;
 }
@@ -159,15 +160,17 @@ static int apparmor_capable(const struct cred *cred, struct user_namespace *ns,
 	if (error)
 		return error;
 
-	label = aa_cred_label(cred);
+	label = aa_get_newest_cred_label(cred);
 	if (unconfined(label))
-		return 0;
+		goto out;
 
 	label_for_each_confined(i, label, profile) {
 		int e = aa_capable(current, profile, cap, audit);
 		if (e)
 			error = e;
 	}
+out:
+	aa_put_label(label);
 
 	return error;
 }
@@ -426,7 +429,7 @@ static int apparmor_file_open(struct file *file, const struct cred *cred)
 		return 0;
 	}
 
-	label = aa_cred_label(cred);
+	label = aa_get_newest_cred_label(cred);
 	if (!unconfined(label)) {
 		struct inode *inode = file_inode(file);
 		struct path_cond cond = { inode->i_uid, inode->i_mode };
@@ -436,6 +439,7 @@ static int apparmor_file_open(struct file *file, const struct cred *cred)
 		/* todo cache full allowed permissions set and state */
 		fcxt->allow = aa_map_file_to_perms(file);
 	}
+	aa_put_label(label);
 
 	return error;
 }
@@ -460,14 +464,14 @@ static void apparmor_file_free_security(struct file *file)
 static int common_file_perm(int op, struct file *file, u32 mask)
 {
 	struct aa_file_cxt *fcxt = file->f_security;
-	struct aa_label *label, *flabel = aa_cred_label(file->f_cred);
+	struct aa_label *label, *flabel = aa_get_newest_cred_label(file->f_cred);
 	int error = 0;
 
 	BUG_ON(!flabel);
 
 	if (!file->f_path.mnt ||
 	    !mediated_filesystem(file_inode(file)))
-		return 0;
+		goto out;
 
 	label = __aa_get_current_label();
 
@@ -482,6 +486,8 @@ static int common_file_perm(int op, struct file *file, u32 mask)
 	    ((flabel != label) || (mask & ~fcxt->allow)))
 		error = aa_file_perm(op, label, file, mask);
 	__aa_put_current_label(label);
+out:
+	aa_put_label(flabel);
 
 	return error;
 }
diff --git a/security/apparmor/resource.c b/security/apparmor/resource.c
index 3614b62..c32b33f 100644
--- a/security/apparmor/resource.c
+++ b/security/apparmor/resource.c
@@ -96,7 +96,7 @@ int aa_task_setrlimit(struct aa_label *label, struct task_struct *task,
 	int i, error = 0;
 
 	rcu_read_lock();
-	task_label = aa_get_label(aa_cred_label(__task_cred(task)));
+	task_label = aa_get_newest_cred_label(__task_cred(task));
 	rcu_read_unlock();
 
 	/* TODO: extend resource control to handle other (non current)
-- 
1.8.3.2





More information about the kernel-team mailing list