From 17322cc3f9ba578f20b5c09fb1630bd234040008 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 15:59:34 -0800
Subject: apparmor: fix auditing of domain transition failures due to
 incomplete policy

When policy specifies a transition to a profile that is not currently
loaded, it result in exec being denied.  However the failure is not being
audited correctly because the audit code is treating this as an allowed
permission and thus not reporting it.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
---
 security/apparmor/domain.c | 2 ++
 1 file changed, 2 insertions(+)

(limited to 'security')

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 859abdaac1ea..7bc85c7f4573 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -443,6 +443,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 			} else {
 				error = -ENOENT;
 				info = "profile not found";
+				/* remove MAY_EXEC to audit as failure */
+				perms.allow &= ~MAY_EXEC;
 			}
 		}
 	} else if (COMPLAIN_MODE(profile)) {
-- 
cgit v1.2.3


From 04266236b1c3030bb7f75472ac85a8b78fcfb284 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:00:34 -0800
Subject: apparmor: Remove -W1 warnings

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
---
 security/apparmor/domain.c | 2 --
 security/apparmor/lsm.c    | 4 ----
 2 files changed, 6 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 7bc85c7f4573..7a78e814f0d4 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -752,7 +752,6 @@ int aa_change_profile(const char *ns_name, const char *hname, bool onexec,
 		      bool permtest)
 {
 	const struct cred *cred;
-	struct aa_task_cxt *cxt;
 	struct aa_profile *profile, *target = NULL;
 	struct aa_namespace *ns = NULL;
 	struct file_perms perms = {};
@@ -772,7 +771,6 @@ int aa_change_profile(const char *ns_name, const char *hname, bool onexec,
 	}
 
 	cred = get_current_cred();
-	cxt = cred->security;
 	profile = aa_cred_profile(cred);
 
 	/*
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index b21830eced41..0f61dadca9e6 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -469,7 +469,6 @@ static int apparmor_file_lock(struct file *file, unsigned int cmd)
 static int common_mmap(int op, struct file *file, unsigned long prot,
 		       unsigned long flags)
 {
-	struct dentry *dentry;
 	int mask = 0;
 
 	if (!file || !file->f_security)
@@ -486,7 +485,6 @@ static int common_mmap(int op, struct file *file, unsigned long prot,
 	if (prot & PROT_EXEC)
 		mask |= AA_EXEC_MMAP;
 
-	dentry = file->f_path.dentry;
 	return common_file_perm(op, file, mask);
 }
 
@@ -507,11 +505,9 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
 				char **value)
 {
 	int error = -ENOENT;
-	struct aa_profile *profile;
 	/* released below */
 	const struct cred *cred = get_task_cred(task);
 	struct aa_task_cxt *cxt = cred->security;
-	profile = aa_cred_profile(cred);
 
 	if (strcmp(name, "current") == 0)
 		error = aa_getprocattr(aa_newest_version(cxt->profile),
-- 
cgit v1.2.3


From 50c5ecd5d8ffb0e549676b8fd9781e3b2fd751a0 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:01:34 -0800
Subject: apparmor: refactor profile mode macros

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
---
 security/apparmor/include/policy.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index bda4569fdd83..95979c431e26 100644
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -32,13 +32,13 @@
 extern const char *const profile_mode_names[];
 #define APPARMOR_NAMES_MAX_INDEX 3
 
-#define COMPLAIN_MODE(_profile)	\
-	((aa_g_profile_mode == APPARMOR_COMPLAIN) || \
-	 ((_profile)->mode == APPARMOR_COMPLAIN))
+#define PROFILE_MODE(_profile, _mode)		\
+	((aa_g_profile_mode == (_mode)) ||	\
+	 ((_profile)->mode == (_mode)))
 
-#define KILL_MODE(_profile) \
-	((aa_g_profile_mode == APPARMOR_KILL) || \
-	 ((_profile)->mode == APPARMOR_KILL))
+#define COMPLAIN_MODE(_profile)	PROFILE_MODE((_profile), APPARMOR_COMPLAIN)
+
+#define KILL_MODE(_profile) PROFILE_MODE((_profile), APPARMOR_KILL)
 
 #define PROFILE_IS_HAT(_profile) ((_profile)->flags & PFLAG_HAT)
 
-- 
cgit v1.2.3


From e573cc30bb36df23fb49a29d96e6c6333d17f59c Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:02:34 -0800
Subject: apparmor: fix error code to failure message mapping for name lookup

-ESTALE used to be incorrectly used to indicate a disconnected path, when
name lookup failed.  This was fixed in commit e1b0e444 to correctly return
-EACCESS, but the error to failure message mapping was not correctly updated
to reflect this change.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
---
 security/apparmor/path.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'security')

diff --git a/security/apparmor/path.c b/security/apparmor/path.c
index e91ffee80162..35b394a75d76 100644
--- a/security/apparmor/path.c
+++ b/security/apparmor/path.c
@@ -174,7 +174,7 @@ static int get_name_to_buffer(struct path *path, int flags, char *buffer,
 	if (info && error) {
 		if (error == -ENOENT)
 			*info = "Failed name lookup - deleted entry";
-		else if (error == -ESTALE)
+		else if (error == -EACCES)
 			*info = "Failed name lookup - disconnected path";
 		else if (error == -ENAMETOOLONG)
 			*info = "Failed name lookup - name too long";
-- 
cgit v1.2.3


From 3cfcc19e0b5390c04cb5bfa4e8fde39395410e61 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:03:34 -0800
Subject: apparmor: add utility function to get an arbitrary tasks profile.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
---
 security/apparmor/context.c         | 17 +++++++++++++++
 security/apparmor/domain.c          | 10 +++------
 security/apparmor/include/context.h | 41 ++++++++++++++++++++++---------------
 security/apparmor/ipc.c             | 13 ++++--------
 4 files changed, 49 insertions(+), 32 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/context.c b/security/apparmor/context.c
index 8a9b5027c813..611e6ce70b03 100644
--- a/security/apparmor/context.c
+++ b/security/apparmor/context.c
@@ -68,6 +68,23 @@ void aa_dup_task_context(struct aa_task_cxt *new, const struct aa_task_cxt *old)
 	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)
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 7a78e814f0d4..fb47d5b71ea6 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 a9cbee4d9e48..1e9443a58877 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
@@ -113,6 +98,30 @@ static inline struct aa_profile *aa_cred_profile(const struct cred *cred)
 	return aa_newest_version(cxt->profile);
 }
 
+/**
+ * __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)
+{
+	return !unconfined(__aa_task_profile(task));
+}
+
 /**
  * __aa_current_profile - find the current tasks confining profile
  *
diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
index cf1071b14232..c51d2266587e 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;
 }
-- 
cgit v1.2.3


From 0ca554b9fca425eb58325a36290deef698cef34b Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:04:34 -0800
Subject: apparmor: add kvzalloc to handle zeroing for kvmalloc

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
---
 security/apparmor/include/apparmor.h | 12 +++++++++++-
 security/apparmor/lib.c              | 14 +++++++++-----
 security/apparmor/match.c            |  4 ++--
 3 files changed, 22 insertions(+), 8 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h
index 40aedd9f73ea..1ba2ca56a6ef 100644
--- a/security/apparmor/include/apparmor.h
+++ b/security/apparmor/include/apparmor.h
@@ -15,6 +15,7 @@
 #ifndef __APPARMOR_H
 #define __APPARMOR_H
 
+#include <linux/slab.h>
 #include <linux/fs.h>
 
 #include "match.h"
@@ -64,9 +65,18 @@ extern int apparmor_initialized __initdata;
 /* fn's in lib */
 char *aa_split_fqname(char *args, char **ns_name);
 void aa_info_message(const char *str);
-void *kvmalloc(size_t size);
+void *__aa_kvmalloc(size_t size, gfp_t flags);
 void kvfree(void *buffer);
 
+static inline void *kvmalloc(size_t size)
+{
+	return __aa_kvmalloc(size, 0);
+}
+
+static inline void *kvzalloc(size_t size)
+{
+	return __aa_kvmalloc(size, __GFP_ZERO);
+}
 
 /**
  * aa_strneq - compare null terminated @str to a non null terminated substring
diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index 7430298116d6..d6e1f2148398 100644
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -75,15 +75,16 @@ void aa_info_message(const char *str)
 }
 
 /**
- * kvmalloc - do allocation preferring kmalloc but falling back to vmalloc
- * @size: size of allocation
+ * __aa_kvmalloc - do allocation preferring kmalloc but falling back to vmalloc
+ * @size: how many bytes of memory are required
+ * @flags: the type of memory to allocate (see kmalloc).
  *
  * Return: allocated buffer or NULL if failed
  *
  * It is possible that policy being loaded from the user is larger than
  * what can be allocated by kmalloc, in those cases fall back to vmalloc.
  */
-void *kvmalloc(size_t size)
+void *__aa_kvmalloc(size_t size, gfp_t flags)
 {
 	void *buffer = NULL;
 
@@ -92,14 +93,17 @@ void *kvmalloc(size_t size)
 
 	/* do not attempt kmalloc if we need more than 16 pages at once */
 	if (size <= (16*PAGE_SIZE))
-		buffer = kmalloc(size, GFP_NOIO | __GFP_NOWARN);
+		buffer = kmalloc(size, flags | GFP_NOIO | __GFP_NOWARN);
 	if (!buffer) {
 		/* see kvfree for why size must be at least work_struct size
 		 * when allocated via vmalloc
 		 */
 		if (size < sizeof(struct work_struct))
 			size = sizeof(struct work_struct);
-		buffer = vmalloc(size);
+		if (flags & __GFP_ZERO)
+			buffer = vzalloc(size);
+		else
+			buffer = vmalloc(size);
 	}
 	return buffer;
 }
diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index 90971a8c3789..dfd25a9c9a69 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -30,7 +30,7 @@
  *
  * Returns: pointer to table else NULL on failure
  *
- * NOTE: must be freed by kvfree (not kmalloc)
+ * NOTE: must be freed by kvfree (not kfree)
  */
 static struct table_header *unpack_table(char *blob, size_t bsize)
 {
@@ -57,7 +57,7 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
 	if (bsize < tsize)
 		goto out;
 
-	table = kvmalloc(tsize);
+	table = kvzalloc(tsize);
 	if (table) {
 		*table = th;
 		if (th.td_flags == YYTD_DATA8)
-- 
cgit v1.2.3


From 7a2871b566f34d980556072943295efd107eb53c Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:05:34 -0800
Subject: apparmor: use common fn to clear task_context for domain transitions

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
---
 security/apparmor/context.c         | 17 ++++++-----------
 security/apparmor/domain.c          |  6 +-----
 security/apparmor/include/context.h | 13 +++++++++++++
 3 files changed, 20 insertions(+), 16 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/context.c b/security/apparmor/context.c
index 611e6ce70b03..3f911afa2bb9 100644
--- a/security/apparmor/context.c
+++ b/security/apparmor/context.c
@@ -105,16 +105,12 @@ int aa_replace_current_profile(struct aa_profile *profile)
 		return -ENOMEM;
 
 	cxt = new->security;
-	if (unconfined(profile) || (cxt->profile->ns != profile->ns)) {
+	if (unconfined(profile) || (cxt->profile->ns != profile->ns))
 		/* if switching to unconfined or a different profile namespace
 		 * clear out context state
 		 */
-		aa_put_profile(cxt->previous);
-		aa_put_profile(cxt->onexec);
-		cxt->previous = NULL;
-		cxt->onexec = NULL;
-		cxt->token = 0;
-	}
+		aa_clear_task_cxt_trans(cxt);
+
 	/* be careful switching cxt->profile, when racing replacement it
 	 * is possible that cxt->profile->replacedby is the reference keeping
 	 * @profile valid, so make sure to get its reference before dropping
@@ -222,11 +218,10 @@ int aa_restore_previous_profile(u64 token)
 		aa_get_profile(cxt->profile);
 		aa_put_profile(cxt->previous);
 	}
-	/* clear exec && prev information when restoring to previous context */
+	/* ref has been transfered so avoid putting ref in clear_task_cxt */
 	cxt->previous = NULL;
-	cxt->token = 0;
-	aa_put_profile(cxt->onexec);
-	cxt->onexec = NULL;
+	/* clear exec && prev information when restoring to previous context */
+	aa_clear_task_cxt_trans(cxt);
 
 	commit_creds(new);
 	return 0;
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index fb47d5b71ea6..07fcb09b990f 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -512,11 +512,7 @@ x_clear:
 	cxt->profile = new_profile;
 
 	/* clear out all temporary/transitional state from the context */
-	aa_put_profile(cxt->previous);
-	aa_put_profile(cxt->onexec);
-	cxt->previous = NULL;
-	cxt->onexec = NULL;
-	cxt->token = 0;
+	aa_clear_task_cxt_trans(cxt);
 
 audit:
 	error = aa_audit_file(profile, &perms, GFP_KERNEL, OP_EXEC, MAY_EXEC,
diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h
index 1e9443a58877..4cecad313227 100644
--- a/security/apparmor/include/context.h
+++ b/security/apparmor/include/context.h
@@ -160,4 +160,17 @@ static inline struct aa_profile *aa_current_profile(void)
 	return profile;
 }
 
+/**
+ * aa_clear_task_cxt_trans - clear transition tracking info from the cxt
+ * @cxt: task context to clear (NOT NULL)
+ */
+static inline void aa_clear_task_cxt_trans(struct aa_task_cxt *cxt)
+{
+	aa_put_profile(cxt->previous);
+	aa_put_profile(cxt->onexec);
+	cxt->previous = NULL;
+	cxt->onexec = NULL;
+	cxt->token = 0;
+}
+
 #endif /* __AA_CONTEXT_H */
-- 
cgit v1.2.3


From 4b7c331fc2eceaa4da5ded41c0b2eca3fd924444 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:06:34 -0800
Subject: apparmor: remove "permipc" command

The "permipc" command is unused and unfinished, remove it.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
---
 security/apparmor/include/procattr.h | 1 -
 security/apparmor/lsm.c              | 2 --
 security/apparmor/procattr.c         | 6 ------
 3 files changed, 9 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/include/procattr.h b/security/apparmor/include/procattr.h
index 544aa6b766a4..6bd5f33d9533 100644
--- a/security/apparmor/include/procattr.h
+++ b/security/apparmor/include/procattr.h
@@ -21,6 +21,5 @@
 int aa_getprocattr(struct aa_profile *profile, char **string);
 int aa_setprocattr_changehat(char *args, size_t size, int test);
 int aa_setprocattr_changeprofile(char *fqname, bool onexec, int test);
-int aa_setprocattr_permipc(char *fqname);
 
 #endif /* __AA_PROCATTR_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 0f61dadca9e6..ed7e3aadba3a 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -572,8 +572,6 @@ static int apparmor_setprocattr(struct task_struct *task, char *name,
 		} else if (strcmp(command, "permprofile") == 0) {
 			error = aa_setprocattr_changeprofile(args, !AA_ONEXEC,
 							     AA_DO_TEST);
-		} else if (strcmp(command, "permipc") == 0) {
-			error = aa_setprocattr_permipc(args);
 		} else {
 			struct common_audit_data sa;
 			struct apparmor_audit_data aad = {0,};
diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c
index 1b41c542d376..6c9390179b89 100644
--- a/security/apparmor/procattr.c
+++ b/security/apparmor/procattr.c
@@ -163,9 +163,3 @@ int aa_setprocattr_changeprofile(char *fqname, bool onexec, int test)
 	name = aa_split_fqname(fqname, &ns_name);
 	return aa_change_profile(ns_name, name, onexec, test);
 }
-
-int aa_setprocattr_permipc(char *fqname)
-{
-	/* TODO: add ipc permission querying */
-	return -ENOTSUPP;
-}
-- 
cgit v1.2.3


From cf47aede3b9e197d3b4a028e2157bf7736665ac4 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:07:34 -0800
Subject: apparmor: relax the restrictions on setting rlimits

Instead of limiting the setting of the processes limits to current,
relax this to tasks confined by the same profile, as the apparmor
controls for rlimits are at a profile level granularity.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
---
 security/apparmor/resource.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/resource.c b/security/apparmor/resource.c
index e1f3d7ef2c54..748bf0ca6c9f 100644
--- a/security/apparmor/resource.c
+++ b/security/apparmor/resource.c
@@ -15,6 +15,7 @@
 #include <linux/audit.h>
 
 #include "include/audit.h"
+#include "include/context.h"
 #include "include/resource.h"
 #include "include/policy.h"
 
@@ -90,17 +91,25 @@ int aa_map_resource(int resource)
 int aa_task_setrlimit(struct aa_profile *profile, struct task_struct *task,
 		      unsigned int resource, struct rlimit *new_rlim)
 {
+	struct aa_profile *task_profile;
 	int error = 0;
 
+	rcu_read_lock();
+	task_profile = aa_get_profile(aa_cred_profile(__task_cred(task)));
+	rcu_read_unlock();
+
 	/* TODO: extend resource control to handle other (non current)
-	 * processes.  AppArmor rules currently have the implicit assumption
-	 * that the task is setting the resource of the current process
+	 * profiles.  AppArmor rules currently have the implicit assumption
+	 * that the task is setting the resource of a task confined with
+	 * the same profile.
 	 */
-	if ((task != current->group_leader) ||
+	if (profile != task_profile ||
 	    (profile->rlimits.mask & (1 << resource) &&
 	     new_rlim->rlim_max > profile->rlimits.limits[resource].rlim_max))
 		error = -EACCES;
 
+	aa_put_profile(task_profile);
+
 	return audit_resource(profile, resource, new_rlim->rlim_max, error);
 }
 
-- 
cgit v1.2.3


From 8e4ff109d0d2194d98e9e16325bb4102f6463b43 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:08:34 -0800
Subject: apparmor: misc cleanup of match

tidying up comments, includes and defines

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
---
 security/apparmor/include/match.h | 19 +++++++++++++------
 security/apparmor/match.c         |  3 +--
 2 files changed, 14 insertions(+), 8 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
index 775843e7f984..bbbf56f5ba78 100644
--- a/security/apparmor/include/match.h
+++ b/security/apparmor/include/match.h
@@ -4,7 +4,7 @@
  * This file contains AppArmor policy dfa matching engine definitions.
  *
  * Copyright (C) 1998-2008 Novell/SUSE
- * Copyright 2009-2010 Canonical Ltd.
+ * Copyright 2009-2012 Canonical Ltd.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -16,7 +16,6 @@
 #define __AA_MATCH_H
 
 #include <linux/kref.h>
-#include <linux/workqueue.h>
 
 #define DFA_NOMATCH			0
 #define DFA_START			1
@@ -29,12 +28,20 @@
  * file format (--tables-file option; see Table File Format in the flex
  * info pages and the flex sources for documentation). The magic number
  * used in the header is 0x1B5E783D instead of 0xF13C57B1 though, because
- * the YY_ID_CHK (check) and YY_ID_DEF (default) tables are used
- * slightly differently (see the apparmor-parser package).
+ * new tables have been defined and others YY_ID_CHK (check) and YY_ID_DEF
+ * (default) tables are used slightly differently (see the apparmor-parser
+ * package).
+ *
+ *
+ * The data in the packed dfa is stored in network byte order, and the tables
+ * are arranged for flexibility.  We convert the table data to host native
+ * byte order.
+ *
+ * The dfa begins with a table set header, and is followed by the actual
+ * tables.
  */
 
 #define YYTH_MAGIC	0x1B5E783D
-#define YYTH_DEF_RECURSE 0x1			/* DEF Table is recursive */
 
 struct table_set_header {
 	u32 th_magic;		/* YYTH_MAGIC */
@@ -63,7 +70,7 @@ struct table_set_header {
 #define YYTD_DATA32	4
 #define YYTD_DATA64	8
 
-/* Each ACCEPT2 table gets 6 dedicated flags, YYTD_DATAX define the
+/* ACCEPT & ACCEPT2 tables gets 6 dedicated flags, YYTD_DATAX define the
  * first flags
  */
 #define ACCEPT1_FLAGS(X) ((X) & 0x3f)
diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index dfd25a9c9a69..1ff823031c73 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -4,7 +4,7 @@
  * This file contains AppArmor dfa based regular expression matching engine
  *
  * Copyright (C) 1998-2008 Novell/SUSE
- * Copyright 2009-2010 Canonical Ltd.
+ * Copyright 2009-2012 Canonical Ltd.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -137,7 +137,6 @@ static int verify_dfa(struct aa_dfa *dfa, int flags)
 		for (i = 0; i < state_count; i++) {
 			if (DEFAULT_TABLE(dfa)[i] >= state_count)
 				goto out;
-			/* TODO: do check that DEF state recursion terminates */
 			if (BASE_TABLE(dfa)[i] + 255 >= trans_count) {
 				printk(KERN_ERR "AppArmor DFA next/check upper "
 				       "bounds error\n");
-- 
cgit v1.2.3


From 180a6f5965a49535a7704c07691a6d1209904971 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:09:34 -0800
Subject: apparmor: move perm defines into policy_unpack

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
---
 security/apparmor/include/match.h | 2 --
 security/apparmor/policy_unpack.c | 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
index bbbf56f5ba78..001c43aa0406 100644
--- a/security/apparmor/include/match.h
+++ b/security/apparmor/include/match.h
@@ -20,8 +20,6 @@
 #define DFA_NOMATCH			0
 #define DFA_START			1
 
-#define DFA_VALID_PERM_MASK		0xffffffff
-#define DFA_VALID_PERM2_MASK		0xffffffff
 
 /**
  * The format used for transition tables is based on the GNU flex table
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 329b1fd30749..ca48a7d8d5b3 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -290,6 +290,9 @@ static int unpack_strdup(struct aa_ext *e, char **string, const char *name)
 	return res;
 }
 
+#define DFA_VALID_PERM_MASK		0xffffffff
+#define DFA_VALID_PERM2_MASK		0xffffffff
+
 /**
  * verify_accept - verify the accept tables of a dfa
  * @dfa: dfa to verify accept tables of (NOT NULL)
-- 
cgit v1.2.3


From a4987857d2c958b93b2faafe0811eea1a63ff59a Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:10:34 -0800
Subject: apparmor: remove sid from profiles

The sid is not going to be a direct property of a profile anymore, instead
it will be directly related to the label, and the profile will pickup
a label back reference.

For null-profiles replace the use of sid with a per namespace unique
id.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
---
 security/apparmor/include/policy.h |  4 ++--
 security/apparmor/include/sid.h    |  4 +++-
 security/apparmor/policy.c         | 23 ++++++-----------------
 security/apparmor/policy_unpack.c  |  1 -
 4 files changed, 11 insertions(+), 21 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index 95979c431e26..b25491a3046a 100644
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -105,6 +105,7 @@ struct aa_ns_acct {
  * @acct: accounting for the namespace
  * @unconfined: special unconfined profile for the namespace
  * @sub_ns: list of namespaces under the current namespace.
+ * @uniq_null: uniq value used for null learning profiles
  *
  * An aa_namespace defines the set profiles that are searched to determine
  * which profile to attach to a task.  Profiles can not be shared between
@@ -127,6 +128,7 @@ struct aa_namespace {
 	struct aa_ns_acct acct;
 	struct aa_profile *unconfined;
 	struct list_head sub_ns;
+	atomic_t uniq_null;
 };
 
 /* struct aa_policydb - match engine for a policy
@@ -148,7 +150,6 @@ struct aa_policydb {
  * @rename: optional profile name that this profile renamed
  * @xmatch: optional extended matching for unconfined executables names
  * @xmatch_len: xmatch prefix len, used to determine xmatch priority
- * @sid: the unique security id number of this profile
  * @audit: the auditing mode of the profile
  * @mode: the enforcement mode of the profile
  * @flags: flags controlling profile behavior
@@ -184,7 +185,6 @@ struct aa_profile {
 
 	struct aa_dfa *xmatch;
 	int xmatch_len;
-	u32 sid;
 	enum audit_mode audit;
 	enum profile_mode mode;
 	u32 flags;
diff --git a/security/apparmor/include/sid.h b/security/apparmor/include/sid.h
index 020db35c3010..513ca0e48965 100644
--- a/security/apparmor/include/sid.h
+++ b/security/apparmor/include/sid.h
@@ -16,7 +16,9 @@
 
 #include <linux/types.h>
 
-struct aa_profile;
+/* sid value that will not be allocated */
+#define AA_SID_INVALID 0
+#define AA_SID_ALLOC AA_SID_INVALID
 
 u32 aa_alloc_sid(void);
 void aa_free_sid(u32 sid);
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 813200384d97..13fc9efddd5d 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -87,7 +87,6 @@
 #include "include/policy.h"
 #include "include/policy_unpack.h"
 #include "include/resource.h"
-#include "include/sid.h"
 
 
 /* root profile namespace */
@@ -292,7 +291,6 @@ static struct aa_namespace *alloc_namespace(const char *prefix,
 	if (!ns->unconfined)
 		goto fail_unconfined;
 
-	ns->unconfined->sid = aa_alloc_sid();
 	ns->unconfined->flags = PFLAG_UNCONFINED | PFLAG_IX_ON_NAME_ERROR |
 	    PFLAG_IMMUTABLE;
 
@@ -303,6 +301,8 @@ static struct aa_namespace *alloc_namespace(const char *prefix,
 	 */
 	ns->unconfined->ns = aa_get_namespace(ns);
 
+	atomic_set(&ns->uniq_null, 0);
+
 	return ns;
 
 fail_unconfined:
@@ -497,7 +497,6 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new)
 	/* released when @new is freed */
 	new->parent = aa_get_profile(old->parent);
 	new->ns = aa_get_namespace(old->ns);
-	new->sid = old->sid;
 	__list_add_profile(&policy->profiles, new);
 	/* inherit children */
 	list_for_each_entry_safe(child, tmp, &old->base.profiles, base.list) {
@@ -665,7 +664,7 @@ struct aa_profile *aa_alloc_profile(const char *hname)
  * @hat: true if the null- learning profile is a hat
  *
  * Create a null- complain mode profile used in learning mode.  The name of
- * the profile is unique and follows the format of parent//null-sid.
+ * the profile is unique and follows the format of parent//null-<uniq>.
  *
  * null profiles are added to the profile list but the list does not
  * hold a count on them so that they are automatically released when
@@ -677,20 +676,19 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat)
 {
 	struct aa_profile *profile = NULL;
 	char *name;
-	u32 sid = aa_alloc_sid();
+	int uniq = atomic_inc_return(&parent->ns->uniq_null);
 
 	/* freed below */
 	name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, GFP_KERNEL);
 	if (!name)
 		goto fail;
-	sprintf(name, "%s//null-%x", parent->base.hname, sid);
+	sprintf(name, "%s//null-%x", parent->base.hname, uniq);
 
 	profile = aa_alloc_profile(name);
 	kfree(name);
 	if (!profile)
 		goto fail;
 
-	profile->sid = sid;
 	profile->mode = APPARMOR_COMPLAIN;
 	profile->flags = PFLAG_NULL;
 	if (hat)
@@ -708,7 +706,6 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat)
 	return profile;
 
 fail:
-	aa_free_sid(sid);
 	return NULL;
 }
 
@@ -749,7 +746,6 @@ static void free_profile(struct aa_profile *profile)
 	aa_free_cap_rules(&profile->caps);
 	aa_free_rlimit_rules(&profile->rlimits);
 
-	aa_free_sid(profile->sid);
 	aa_put_dfa(profile->xmatch);
 	aa_put_dfa(profile->policy.dfa);
 
@@ -972,7 +968,6 @@ static void __add_new_profile(struct aa_namespace *ns, struct aa_policy *policy,
 		profile->parent = aa_get_profile((struct aa_profile *) policy);
 	__list_add_profile(&policy->profiles, profile);
 	/* released on free_profile */
-	profile->sid = aa_alloc_sid();
 	profile->ns = aa_get_namespace(ns);
 }
 
@@ -1110,14 +1105,8 @@ audit:
 	if (!error) {
 		if (rename_profile)
 			__replace_profile(rename_profile, new_profile);
-		if (old_profile) {
-			/* when there are both rename and old profiles
-			 * inherit old profiles sid
-			 */
-			if (rename_profile)
-				aa_free_sid(new_profile->sid);
+		if (old_profile)
 			__replace_profile(old_profile, new_profile);
-		}
 		if (!(old_profile || rename_profile))
 			__add_new_profile(ns, policy, new_profile);
 	}
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index ca48a7d8d5b3..6dac7d77cb4d 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -27,7 +27,6 @@
 #include "include/match.h"
 #include "include/policy.h"
 #include "include/policy_unpack.h"
-#include "include/sid.h"
 
 /*
  * The AppArmor interface treats data as a type byte followed by the
-- 
cgit v1.2.3


From 4da05cc08da3f2058cecbe42ed9f4803d669730a Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:11:34 -0800
Subject: apparmor: move the free_profile fn ahead of aa_alloc_profile

Move the free_profile fn ahead of aa_alloc_profile so it can be used
in aa_alloc_profile without a forward declaration.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
---
 security/apparmor/policy.c | 150 ++++++++++++++++++++++-----------------------
 1 file changed, 75 insertions(+), 75 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 13fc9efddd5d..f4ee72b44de4 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -634,81 +634,6 @@ void __init aa_free_root_ns(void)
 	 aa_put_namespace(ns);
 }
 
-/**
- * aa_alloc_profile - allocate, initialize and return a new profile
- * @hname: name of the profile  (NOT NULL)
- *
- * Returns: refcount profile or NULL on failure
- */
-struct aa_profile *aa_alloc_profile(const char *hname)
-{
-	struct aa_profile *profile;
-
-	/* freed by free_profile - usually through aa_put_profile */
-	profile = kzalloc(sizeof(*profile), GFP_KERNEL);
-	if (!profile)
-		return NULL;
-
-	if (!policy_init(&profile->base, NULL, hname)) {
-		kzfree(profile);
-		return NULL;
-	}
-
-	/* refcount released by caller */
-	return profile;
-}
-
-/**
- * aa_new_null_profile - create a new null-X learning profile
- * @parent: profile that caused this profile to be created (NOT NULL)
- * @hat: true if the null- learning profile is a hat
- *
- * Create a null- complain mode profile used in learning mode.  The name of
- * the profile is unique and follows the format of parent//null-<uniq>.
- *
- * null profiles are added to the profile list but the list does not
- * hold a count on them so that they are automatically released when
- * not in use.
- *
- * Returns: new refcounted profile else NULL on failure
- */
-struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat)
-{
-	struct aa_profile *profile = NULL;
-	char *name;
-	int uniq = atomic_inc_return(&parent->ns->uniq_null);
-
-	/* freed below */
-	name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, GFP_KERNEL);
-	if (!name)
-		goto fail;
-	sprintf(name, "%s//null-%x", parent->base.hname, uniq);
-
-	profile = aa_alloc_profile(name);
-	kfree(name);
-	if (!profile)
-		goto fail;
-
-	profile->mode = APPARMOR_COMPLAIN;
-	profile->flags = PFLAG_NULL;
-	if (hat)
-		profile->flags |= PFLAG_HAT;
-
-	/* released on free_profile */
-	profile->parent = aa_get_profile(parent);
-	profile->ns = aa_get_namespace(parent->ns);
-
-	write_lock(&profile->ns->lock);
-	__list_add_profile(&parent->base.profiles, profile);
-	write_unlock(&profile->ns->lock);
-
-	/* refcount released by caller */
-	return profile;
-
-fail:
-	return NULL;
-}
-
 /**
  * free_profile - free a profile
  * @profile: the profile to free  (MAYBE NULL)
@@ -786,6 +711,81 @@ void aa_free_profile_kref(struct kref *kref)
 	free_profile(p);
 }
 
+/**
+ * aa_alloc_profile - allocate, initialize and return a new profile
+ * @hname: name of the profile  (NOT NULL)
+ *
+ * Returns: refcount profile or NULL on failure
+ */
+struct aa_profile *aa_alloc_profile(const char *hname)
+{
+	struct aa_profile *profile;
+
+	/* freed by free_profile - usually through aa_put_profile */
+	profile = kzalloc(sizeof(*profile), GFP_KERNEL);
+	if (!profile)
+		return NULL;
+
+	if (!policy_init(&profile->base, NULL, hname)) {
+		kzfree(profile);
+		return NULL;
+	}
+
+	/* refcount released by caller */
+	return profile;
+}
+
+/**
+ * aa_new_null_profile - create a new null-X learning profile
+ * @parent: profile that caused this profile to be created (NOT NULL)
+ * @hat: true if the null- learning profile is a hat
+ *
+ * Create a null- complain mode profile used in learning mode.  The name of
+ * the profile is unique and follows the format of parent//null-<uniq>.
+ *
+ * null profiles are added to the profile list but the list does not
+ * hold a count on them so that they are automatically released when
+ * not in use.
+ *
+ * Returns: new refcounted profile else NULL on failure
+ */
+struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat)
+{
+	struct aa_profile *profile = NULL;
+	char *name;
+	int uniq = atomic_inc_return(&parent->ns->uniq_null);
+
+	/* freed below */
+	name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, GFP_KERNEL);
+	if (!name)
+		goto fail;
+	sprintf(name, "%s//null-%x", parent->base.hname, uniq);
+
+	profile = aa_alloc_profile(name);
+	kfree(name);
+	if (!profile)
+		goto fail;
+
+	profile->mode = APPARMOR_COMPLAIN;
+	profile->flags = PFLAG_NULL;
+	if (hat)
+		profile->flags |= PFLAG_HAT;
+
+	/* released on free_profile */
+	profile->parent = aa_get_profile(parent);
+	profile->ns = aa_get_namespace(parent->ns);
+
+	write_lock(&profile->ns->lock);
+	__list_add_profile(&parent->base.profiles, profile);
+	write_unlock(&profile->ns->lock);
+
+	/* refcount released by caller */
+	return profile;
+
+fail:
+	return NULL;
+}
+
 /* TODO: profile accounting - setup in remove */
 
 /**
-- 
cgit v1.2.3


From ed686308c6837ff67f56e4115d0fd6bdc65a4313 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:12:34 -0800
Subject: apparmor: reserve and mask off the top 8 bits of the base field

The top 8 bits of the base field have never been used, in fact can't
be used, by the current 'dfa16' format.  However they will be used in the
future as flags, so mask them off when using base as an index value.

Note: the use of the top 8 bits, without masking is trapped by the verify
      checks that base entries are within the size bounds.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
---
 security/apparmor/match.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index 1ff823031c73..727eb4200d5c 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -23,6 +23,8 @@
 #include "include/apparmor.h"
 #include "include/match.h"
 
+#define base_idx(X) ((X) & 0xffffff)
+
 /**
  * unpack_table - unpack a dfa table (one of accept, default, base, next check)
  * @blob: data to unpack (NOT NULL)
@@ -137,7 +139,7 @@ static int verify_dfa(struct aa_dfa *dfa, int flags)
 		for (i = 0; i < state_count; i++) {
 			if (DEFAULT_TABLE(dfa)[i] >= state_count)
 				goto out;
-			if (BASE_TABLE(dfa)[i] + 255 >= trans_count) {
+			if (base_idx(BASE_TABLE(dfa)[i]) + 255 >= trans_count) {
 				printk(KERN_ERR "AppArmor DFA next/check upper "
 				       "bounds error\n");
 				goto out;
@@ -313,7 +315,7 @@ unsigned int aa_dfa_match_len(struct aa_dfa *dfa, unsigned int start,
 		u8 *equiv = EQUIV_TABLE(dfa);
 		/* default is direct to next state */
 		for (; len; len--) {
-			pos = base[state] + equiv[(u8) *str++];
+			pos = base_idx(base[state]) + equiv[(u8) *str++];
 			if (check[pos] == state)
 				state = next[pos];
 			else
@@ -322,7 +324,7 @@ unsigned int aa_dfa_match_len(struct aa_dfa *dfa, unsigned int start,
 	} else {
 		/* default is direct to next state */
 		for (; len; len--) {
-			pos = base[state] + (u8) *str++;
+			pos = base_idx(base[state]) + (u8) *str++;
 			if (check[pos] == state)
 				state = next[pos];
 			else
@@ -363,7 +365,7 @@ unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start,
 		u8 *equiv = EQUIV_TABLE(dfa);
 		/* default is direct to next state */
 		while (*str) {
-			pos = base[state] + equiv[(u8) *str++];
+			pos = base_idx(base[state]) + equiv[(u8) *str++];
 			if (check[pos] == state)
 				state = next[pos];
 			else
@@ -372,7 +374,7 @@ unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start,
 	} else {
 		/* default is direct to next state */
 		while (*str) {
-			pos = base[state] + (u8) *str++;
+			pos = base_idx(base[state]) + (u8) *str++;
 			if (check[pos] == state)
 				state = next[pos];
 			else
@@ -408,14 +410,14 @@ unsigned int aa_dfa_next(struct aa_dfa *dfa, unsigned int state,
 		u8 *equiv = EQUIV_TABLE(dfa);
 		/* default is direct to next state */
 
-		pos = base[state] + equiv[(u8) c];
+		pos = base_idx(base[state]) + equiv[(u8) c];
 		if (check[pos] == state)
 			state = next[pos];
 		else
 			state = def[state];
 	} else {
 		/* default is direct to next state */
-		pos = base[state] + (u8) c;
+		pos = base_idx(base[state]) + (u8) c;
 		if (check[pos] == state)
 			state = next[pos];
 		else
-- 
cgit v1.2.3


From b492d50bf597b87ab7ea1e738ec837f74b11594e Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 18 Feb 2013 16:13:34 -0800
Subject: apparmor: fix the audit type table

The audit type table is missing a comma so that KILLED comes out as
KILLEDAUTO.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
---
 security/apparmor/audit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'security')

diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 3ae28db5a64f..031d2d9dd695 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -88,7 +88,7 @@ static const char *const aa_audit_type[] = {
 	"HINT",
 	"STATUS",
 	"ERROR",
-	"KILLED"
+	"KILLED",
 	"AUTO"
 };
 
-- 
cgit v1.2.3


From 41d1b3e868c263e8b43dd5903a70633e05ae58a6 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Thu, 21 Feb 2013 01:14:17 -0800
Subject: apparmor: Fix smatch warning in aa_remove_profiles

smatch reports
  error: potential NULL dereference 'ns'.

this can not actually occur because it relies on aa_split_fqname setting
both ns_name and name as null but ns_name will actually always have a
value in this case.

so remove the unnecessary if (ns_name) conditional that is resulting
in the false positive further down.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/policy.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index f4ee72b44de4..0f345c4dee5f 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -1156,14 +1156,12 @@ ssize_t aa_remove_profiles(char *fqname, size_t size)
 	if (fqname[0] == ':') {
 		char *ns_name;
 		name = aa_split_fqname(fqname, &ns_name);
-		if (ns_name) {
-			/* released below */
-			ns = aa_find_namespace(root, ns_name);
-			if (!ns) {
-				info = "namespace does not exist";
-				error = -ENOENT;
-				goto fail;
-			}
+		/* released below */
+		ns = aa_find_namespace(root, ns_name);
+		if (!ns) {
+			info = "namespace does not exist";
+			error = -ENOENT;
+			goto fail;
 		}
 	} else
 		/* released below */
-- 
cgit v1.2.3


From 53fe8b9961716033571d9799005bfdbbafa5162c Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Thu, 21 Feb 2013 13:25:44 -0800
Subject: apparmor: fix sparse warnings

Fix a couple of warning reported by sparse

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/include/file.h | 14 +++++++-------
 security/apparmor/lsm.c          |  8 ++++----
 2 files changed, 11 insertions(+), 11 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h
index 967b2deda376..2c922b86bd44 100644
--- a/security/apparmor/include/file.h
+++ b/security/apparmor/include/file.h
@@ -186,11 +186,6 @@ static inline void aa_free_file_rules(struct aa_file_rules *rules)
 	aa_free_domain_entries(&rules->trans);
 }
 
-#define ACC_FMODE(x) (("\000\004\002\006"[(x)&O_ACCMODE]) | (((x) << 1) & 0x40))
-
-/* from namei.c */
-#define MAP_OPEN_FLAGS(x) ((((x) + 1) & O_ACCMODE) ? (x) + 1 : (x))
-
 /**
  * aa_map_file_perms - map file flags to AppArmor permissions
  * @file: open file to map flags to AppArmor permissions
@@ -199,8 +194,13 @@ static inline void aa_free_file_rules(struct aa_file_rules *rules)
  */
 static inline u32 aa_map_file_to_perms(struct file *file)
 {
-	int flags = MAP_OPEN_FLAGS(file->f_flags);
-	u32 perms = ACC_FMODE(file->f_mode);
+	int flags = file->f_flags;
+	u32 perms = 0;
+
+	if (file->f_mode & FMODE_WRITE)
+		perms |= MAY_WRITE;
+	if (file->f_mode & FMODE_READ)
+		perms |= MAY_READ;
 
 	if ((flags & O_APPEND) && (perms & MAY_WRITE))
 		perms = (perms & ~MAY_WRITE) | MAY_APPEND;
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index ed7e3aadba3a..10843aa5a368 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -909,8 +909,11 @@ static int __init apparmor_init(void)
 
 	error = register_security(&apparmor_ops);
 	if (error) {
+		struct cred *cred = (struct cred *)current->real_cred;
+		aa_free_task_context(cred->security);
+		cred->security = NULL;
 		AA_ERROR("Unable to register AppArmor\n");
-		goto set_init_cxt_out;
+		goto register_security_out;
 	}
 
 	/* Report that AppArmor successfully initialized */
@@ -924,9 +927,6 @@ static int __init apparmor_init(void)
 
 	return error;
 
-set_init_cxt_out:
-	aa_free_task_context(current->real_cred->security);
-
 register_security_out:
 	aa_free_root_ns();
 
-- 
cgit v1.2.3


From 214beacaa7b669473bc963af719fa359a8312ea4 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Wed, 27 Feb 2013 03:43:40 -0800
Subject: apparmor: localize getting the security context to a few macros

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
---
 security/apparmor/context.c         | 10 +++++-----
 security/apparmor/domain.c          |  6 +++---
 security/apparmor/include/context.h |  7 +++++--
 security/apparmor/lsm.c             | 22 +++++++++++-----------
 4 files changed, 24 insertions(+), 21 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/context.c b/security/apparmor/context.c
index 3f911afa2bb9..d5af1d15f26d 100644
--- a/security/apparmor/context.c
+++ b/security/apparmor/context.c
@@ -93,7 +93,7 @@ struct aa_profile *aa_get_task_profile(struct task_struct *task)
  */
 int aa_replace_current_profile(struct aa_profile *profile)
 {
-	struct aa_task_cxt *cxt = current_cred()->security;
+	struct aa_task_cxt *cxt = current_cxt();
 	struct cred *new;
 	BUG_ON(!profile);
 
@@ -104,7 +104,7 @@ int aa_replace_current_profile(struct aa_profile *profile)
 	if (!new)
 		return -ENOMEM;
 
-	cxt = new->security;
+	cxt = cred_cxt(new);
 	if (unconfined(profile) || (cxt->profile->ns != profile->ns))
 		/* if switching to unconfined or a different profile namespace
 		 * clear out context state
@@ -136,7 +136,7 @@ int aa_set_current_onexec(struct aa_profile *profile)
 	if (!new)
 		return -ENOMEM;
 
-	cxt = new->security;
+	cxt = cred_cxt(new);
 	aa_get_profile(profile);
 	aa_put_profile(cxt->onexec);
 	cxt->onexec = profile;
@@ -163,7 +163,7 @@ int aa_set_current_hat(struct aa_profile *profile, u64 token)
 		return -ENOMEM;
 	BUG_ON(!profile);
 
-	cxt = new->security;
+	cxt = cred_cxt(new);
 	if (!cxt->previous) {
 		/* transfer refcount */
 		cxt->previous = cxt->profile;
@@ -200,7 +200,7 @@ int aa_restore_previous_profile(u64 token)
 	if (!new)
 		return -ENOMEM;
 
-	cxt = new->security;
+	cxt = cred_cxt(new);
 	if (cxt->token != token) {
 		abort_creds(new);
 		return -EACCES;
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 07fcb09b990f..01b7bd669a88 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -356,7 +356,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 	if (bprm->cred_prepared)
 		return 0;
 
-	cxt = bprm->cred->security;
+	cxt = cred_cxt(bprm->cred);
 	BUG_ON(!cxt);
 
 	profile = aa_get_profile(aa_newest_version(cxt->profile));
@@ -551,7 +551,7 @@ 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_cxt *new_cxt = bprm->cred->security;
+	struct aa_task_cxt *new_cxt = cred_cxt(bprm->cred);
 
 	/* bail out if unconfined or not changing profile */
 	if ((new_cxt->profile == profile) ||
@@ -628,7 +628,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest)
 
 	/* released below */
 	cred = get_current_cred();
-	cxt = cred->security;
+	cxt = cred_cxt(cred);
 	profile = aa_cred_profile(cred);
 	previous_profile = cxt->previous;
 
diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h
index 4cecad313227..d44ba5802e3d 100644
--- a/security/apparmor/include/context.h
+++ b/security/apparmor/include/context.h
@@ -21,6 +21,9 @@
 
 #include "policy.h"
 
+#define cred_cxt(X) (X)->security
+#define current_cxt() cred_cxt(current_cred())
+
 /* struct aa_file_cxt - the AppArmor context the file was opened in
  * @perms: the permission the file was opened with
  *
@@ -93,7 +96,7 @@ 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_cxt *cxt = cred->security;
+	struct aa_task_cxt *cxt = cred_cxt(cred);
 	BUG_ON(!cxt || !cxt->profile);
 	return aa_newest_version(cxt->profile);
 }
@@ -145,7 +148,7 @@ static inline struct aa_profile *__aa_current_profile(void)
  */
 static inline struct aa_profile *aa_current_profile(void)
 {
-	const struct aa_task_cxt *cxt = current_cred()->security;
+	const struct aa_task_cxt *cxt = current_cxt();
 	struct aa_profile *profile;
 	BUG_ON(!cxt || !cxt->profile);
 
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 10843aa5a368..2027fdf2060b 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -48,8 +48,8 @@ int apparmor_initialized __initdata;
  */
 static void apparmor_cred_free(struct cred *cred)
 {
-	aa_free_task_context(cred->security);
-	cred->security = NULL;
+	aa_free_task_context(cred_cxt(cred));
+	cred_cxt(cred) = NULL;
 }
 
 /*
@@ -62,7 +62,7 @@ static int apparmor_cred_alloc_blank(struct cred *cred, gfp_t gfp)
 	if (!cxt)
 		return -ENOMEM;
 
-	cred->security = cxt;
+	cred_cxt(cred) = cxt;
 	return 0;
 }
 
@@ -77,8 +77,8 @@ static int apparmor_cred_prepare(struct cred *new, const struct cred *old,
 	if (!cxt)
 		return -ENOMEM;
 
-	aa_dup_task_context(cxt, old->security);
-	new->security = cxt;
+	aa_dup_task_context(cxt, cred_cxt(old));
+	cred_cxt(new) = cxt;
 	return 0;
 }
 
@@ -87,8 +87,8 @@ 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_cxt *old_cxt = old->security;
-	struct aa_task_cxt *new_cxt = new->security;
+	const struct aa_task_cxt *old_cxt = cred_cxt(old);
+	struct aa_task_cxt *new_cxt = cred_cxt(new);
 
 	aa_dup_task_context(new_cxt, old_cxt);
 }
@@ -507,7 +507,7 @@ 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_cxt *cxt = cred->security;
+	struct aa_task_cxt *cxt = cred_cxt(cred);
 
 	if (strcmp(name, "current") == 0)
 		error = aa_getprocattr(aa_newest_version(cxt->profile),
@@ -880,7 +880,7 @@ static int __init set_init_cxt(void)
 		return -ENOMEM;
 
 	cxt->profile = aa_get_profile(root_ns->unconfined);
-	cred->security = cxt;
+	cred_cxt(cred) = cxt;
 
 	return 0;
 }
@@ -910,8 +910,8 @@ static int __init apparmor_init(void)
 	error = register_security(&apparmor_ops);
 	if (error) {
 		struct cred *cred = (struct cred *)current->real_cred;
-		aa_free_task_context(cred->security);
-		cred->security = NULL;
+		aa_free_task_context(cred_cxt(cred));
+		cred_cxt(cred) = NULL;
 		AA_ERROR("Unable to register AppArmor\n");
 		goto register_security_out;
 	}
-- 
cgit v1.2.3


From 3eea57c26e49a5add4c053a031cc2a1977b7c48e Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Wed, 27 Feb 2013 03:44:40 -0800
Subject: apparmor: fix setprocattr arg processing for onexec

the exec file isn't processing its command arg. It should only set be
responding to a command of exec.

Also cleanup setprocattr some more while we are at it.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/lsm.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 2027fdf2060b..2e2a0dd4a73f 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -529,6 +529,8 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
 static int apparmor_setprocattr(struct task_struct *task, char *name,
 				void *value, size_t size)
 {
+	struct common_audit_data sa;
+	struct apparmor_audit_data aad = {0,};
 	char *command, *args = value;
 	size_t arg_size;
 	int error;
@@ -572,28 +574,31 @@ static int apparmor_setprocattr(struct task_struct *task, char *name,
 		} else if (strcmp(command, "permprofile") == 0) {
 			error = aa_setprocattr_changeprofile(args, !AA_ONEXEC,
 							     AA_DO_TEST);
-		} else {
-			struct common_audit_data sa;
-			struct apparmor_audit_data aad = {0,};
-			sa.type = LSM_AUDIT_DATA_NONE;
-			sa.aad = &aad;
-			aad.op = OP_SETPROCATTR;
-			aad.info = name;
-			aad.error = -EINVAL;
-			return aa_audit(AUDIT_APPARMOR_DENIED,
-					__aa_current_profile(), GFP_KERNEL,
-					&sa, NULL);
-		}
+		} else
+			goto fail;
 	} else if (strcmp(name, "exec") == 0) {
-		error = aa_setprocattr_changeprofile(args, AA_ONEXEC,
-						     !AA_DO_TEST);
-	} else {
+		if (strcmp(command, "exec") == 0)
+			error = aa_setprocattr_changeprofile(args, AA_ONEXEC,
+							     !AA_DO_TEST);
+		else
+			goto fail;
+	} else
 		/* only support the "current" and "exec" process attributes */
 		return -EINVAL;
-	}
+
 	if (!error)
 		error = size;
 	return error;
+
+fail:
+	sa.type = LSM_AUDIT_DATA_NONE;
+	sa.aad = &aad;
+	aad.profile = aa_current_profile();
+	aad.op = OP_SETPROCATTR;
+	aad.info = name;
+	aad.error = -EINVAL;
+	aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL);
+	return -EINVAL;
 }
 
 static int apparmor_task_setrlimit(struct task_struct *task,
-- 
cgit v1.2.3


From 2654bfbc2bd0e1e64f0b257c21da23f6cec32c6c Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Wed, 27 Feb 2013 03:45:05 -0800
Subject: apparmor: fix fully qualified name parsing

currently apparmor name parsing is only correctly handling
:<NS>:<profile>

but
:<NS>://<profile>

is also a valid form and what is exported to userspace.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/lib.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

(limited to 'security')

diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index d6e1f2148398..d40bc592180d 100644
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -45,8 +45,10 @@ char *aa_split_fqname(char *fqname, char **ns_name)
 		*ns_name = skip_spaces(&name[1]);
 		if (split) {
 			/* overwrite ':' with \0 */
-			*split = 0;
-			name = skip_spaces(split + 1);
+			*split++ = 0;
+			if (strncmp(split, "//", 2) == 0)
+				split += 2;
+			name = skip_spaces(split);
 		} else
 			/* a ns name without a following profile is allowed */
 			name = NULL;
-- 
cgit v1.2.3