On 08/05/2014 07:23 AM, Miklos Szeredi wrote:
> On Mon, Jul 14, 2014 at 11:44 AM, Miklos Szeredi <[email protected]> wrote:
>> Hi John,
>>
>> On Sun, Jul 13, 2014 at 1:18 AM, John Johansen
>> <[email protected]> wrote:
>>> No, this needs to be fixed, and has been in later dev trees, but they
>>> won't apply. What kernels versions are we looking at so I can provide
>>> a backport patch.
> 
> I can do the backport as well, but haven't been able to track down the
> dev tree you speak of.
> 
> Could you please help?
> 
Hey Miklos sorry, I have been meaning to get back to this. Attached is the
patch but I haven't build tested it, nor tested.

In the dev tree it is put together a little differently and over multiple
patches that need to be cleaned up and folded together. You can poke at
  git://kernel.ubuntu.com/jj/ubuntu-utopic.git

>From b4069c98ec767dbbba468e8600ed6fdae2c808b0 Mon Sep 17 00:00:00 2001
From: John Johansen <[email protected]>
Date: Tue, 5 Aug 2014 09:48:54 -0700
Subject: [PATCH] apparmor: Fix: replacement not being applied due to cred not
 being updated

The task cred can only be updated by the task it self. So profile
replacement is achieved by the task recognizing its profile is no longer
valid and updating its own cred with the new profile.

However this can not be done in all hooks because 'updating' a cred does
a GFP_KERNEL allocation. For these paths fallback grabbing a reference
to the new profile if the current cred is stale.

Signed-off-by: John Johansen <[email protected]>
---
 security/apparmor/include/context.h | 31 +++++++++++++++++++++++++++++++
 security/apparmor/lsm.c             | 10 ++++++----
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h
index 6bf6579..6875a0d 100644
--- a/security/apparmor/include/context.h
+++ b/security/apparmor/include/context.h
@@ -139,6 +139,37 @@ static inline struct aa_profile *__aa_current_profile(void)
 }
 
 /**
+ * aa_begin_current_profile - find newest version of the current tasks profile
+ *
+ * Returns: newest version of confining profile (NOT NULL)
+ *
+ * This fn will not update the tasks cred, so it is safe inside of locks
+ *
+ * The returned reference must be put with aa_end_current_profile()
+ */
+static inline struct aa_profile *aa_begin_current_profile(void)
+{
+	struct aa_profile *p = __aa_current_profile();
+
+	if (PROFILE_INVALID(p))
+		l = aa_get_newest_profile(p);
+	return l;
+}
+
+/**
+ * aa_end_current_label - put a reference found with aa_begin_current_label
+ * @profile: profile reference to put
+ *
+ * Should only be used with a reference obtained with aa_begin_current_profile
+ * and never used in situations where the task cred may be updated
+ */
+static inline void aa_end_current_profile(struct aa_label *profile)
+{
+	if (profile != __aa_current_profile())
+		aa_put_profile(profile);
+}
+
+/**
  * aa_current_profile - find the current tasks confining profile and do updates
  *
  * Returns: up to date confining profile or the ns unconfined profile (NOT NULL)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 9981000..42bdf73 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -165,10 +165,10 @@ static int common_perm(int op, struct path *path, u32 mask,
 	struct aa_profile *profile;
 	int error = 0;
 
-	profile = __aa_current_profile();
+	profile = aa_begin_current_profile();
 	if (!unconfined(profile))
 		error = aa_path_perm(op, profile, path, 0, mask, cond);
-
+	aa_end_current_profile(profile);
 	return error;
 }
 
@@ -435,7 +435,7 @@ static int common_file_perm(int op, struct file *file, u32 mask)
 	    !mediated_filesystem(file_inode(file)))
 		return 0;
 
-	profile = __aa_current_profile();
+	profile = aa_begin_current_profile();
 
 	/* revalidate access, if task is unconfined, or the cached cred
 	 * doesn't match or if the request is for more permissions than
@@ -448,6 +448,7 @@ static int common_file_perm(int op, struct file *file, u32 mask)
 	    ((fprofile != profile) || (mask & ~fcxt->allow)))
 		error = aa_file_perm(op, profile, file, mask);
 
+	aa_end_current_profile(profile);
 	return error;
 }
 
@@ -606,12 +607,13 @@ fail:
 static int apparmor_task_setrlimit(struct task_struct *task,
 		unsigned int resource, struct rlimit *new_rlim)
 {
-	struct aa_profile *profile = __aa_current_profile();
+	struct aa_profile *profile = aa_begin_current_profile();
 	int error = 0;
 
 	if (!unconfined(profile))
 		error = aa_task_setrlimit(profile, task, resource, new_rlim);
 
+	aa_end_current_profile(profile);
 	return error;
 }
 
-- 
2.0.1

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

Reply via email to