[apparmor] [PATCH 05/15] apparmor: add utility function to get an arbitrary tasks profile.

[Seth Arnold]

I think I'd rather see this function a little differently:

+static inline bool __aa_task_is_confined(struct task_struct *task)
+{
+if (unconfined(__aa_task_profile(task)))
+return 0;
+
+return 1;
+}

It could be "return !unconfined(__aa_task_profile(task))". That'd make it clear the swapped true/false is intentional. (It'd also remove a conditional branch but one hopes the compiler already made this change.)
-----Original Message-----
From: John Johansen <john.johansen at canonical.com>
Sender: apparmor-bounces at lists.ubuntu.com
Date: Thu, 12 Jul 2012 12:05:01 
To: <apparmor at lists.ubuntu.com>
Subject: [apparmor] [PATCH 05/15] apparmor: add utility function to get an
	arbitrary tasks profile.

Signed-off-by: John Johansen <john.johansen at canonical.com>
---
 security/apparmor/context.c         |   17 ++++++++++++++
 security/apparmor/domain.c          |   10 +++-----
 security/apparmor/include/context.h |   44 ++++++++++++++++++++++-------------
 security/apparmor/ipc.c             |   13 ++++-------
 4 files changed, 52 insertions(+), 32 deletions(-)

diff --git a/security/apparmor/context.c b/security/apparmor/context.c
index 8a9b502..611e6ce 100644
--- a/security/apparmor/context.c
+++ b/security/apparmor/context.c
@@ -69,6 +69,23 @@ void aa_dup_task_context(struct aa_task_cxt *new, const struct aa_task_cxt *old)
 }
 
 /**
+ * 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)
  *
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index f5691bd..35e6605 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -62,17 +62,14 @@ static int may_change_ptraced_domain(struct task_struct *task,
 				     struct aa_profile *to_profile)
 {
 	struct task_struct *tracer;
-	const struct cred *cred = NULL;
 	struct aa_profile *tracerp = NULL;
 	int error = 0;
 
 	rcu_read_lock();
 	tracer = ptrace_parent(task);
-	if (tracer) {
+	if (tracer)
 		/* released below */
-		cred = get_task_cred(tracer);
-		tracerp = aa_cred_profile(cred);
-	}
+		tracerp = aa_get_task_profile(tracer);
 
 	/* not ptraced */
 	if (!tracer || unconfined(tracerp))
@@ -82,8 +79,7 @@ static int may_change_ptraced_domain(struct task_struct *task,
 
 out:
 	rcu_read_unlock();
-	if (cred)
-		put_cred(cred);
+	aa_put_profile(tracerp);
 
 	return error;
 }
diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h
index a9cbee4..f5fe462f 100644
--- a/security/apparmor/include/context.h
+++ b/security/apparmor/include/context.h
@@ -80,23 +80,8 @@ 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);
 int aa_restore_previous_profile(u64 cookie);
+struct aa_profile *aa_get_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)
-{
-	struct aa_task_cxt *cxt = __task_cred(task)->security;
-
-	BUG_ON(!cxt || !cxt->profile);
-	if (unconfined(aa_newest_version(cxt->profile)))
-		return 0;
-
-	return 1;
-}
 
 /**
  * aa_cred_profile - obtain cred's profiles
@@ -114,6 +99,33 @@ static inline struct aa_profile *aa_cred_profile(const struct cred *cred)
 }
 
 /**
+ * __aa_task_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)
+{
+	return aa_cred_profile(__task_cred(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)
+{
+	if (unconfined(__aa_task_profile(task)))
+		return 0;
+
+	return 1;
+}
+
+/**
  * __aa_current_profile - find the current tasks confining profile
  *
  * Returns: up to date confining profile or the ns unconfined profile (NOT NULL)
diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
index cf1071b..c51d226 100644
--- a/security/apparmor/ipc.c
+++ b/security/apparmor/ipc.c
@@ -95,23 +95,18 @@ int aa_ptrace(struct task_struct *tracer, struct task_struct *tracee,
 	 *       - tracer profile has CAP_SYS_PTRACE
 	 */
 
-	struct aa_profile *tracer_p;
-	/* cred released below */
-	const struct cred *cred = get_task_cred(tracer);
+	struct aa_profile *tracer_p = aa_get_task_profile(tracer);
 	int error = 0;
-	tracer_p = aa_cred_profile(cred);
 
 	if (!unconfined(tracer_p)) {
-		/* lcred released below */
-		const struct cred *lcred = get_task_cred(tracee);
-		struct aa_profile *tracee_p = aa_cred_profile(lcred);
+		struct aa_profile *tracee_p = aa_get_task_profile(tracee);
 
 		error = aa_may_ptrace(tracer, tracer_p, tracee_p, mode);
 		error = aa_audit_ptrace(tracer_p, tracee_p, error);
 
-		put_cred(lcred);
+		aa_put_profile(tracee_p);
 	}
-	put_cred(cred);
+	aa_put_profile(tracer_p);
 
 	return error;
 }
-- 
1.7.9.5


-- 
AppArmor mailing list
AppArmor at lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor