From 278de07ef84a4356a3b166dc17751abc03abca67 Mon Sep 17 00:00:00 2001
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 Jul 2019 20:27:32 +0200
Subject: apparmor: Replace two seq_printf() calls by seq_puts() in
 aa_label_seq_xprint()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two strings which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function “seq_puts”.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/label.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/apparmor/label.c b/security/apparmor/label.c
index 470693239e64..bb34094421c4 100644
--- a/security/apparmor/label.c
+++ b/security/apparmor/label.c
@@ -1749,13 +1749,13 @@ void aa_label_seq_xprint(struct seq_file *f, struct aa_ns *ns,
 			AA_DEBUG("label print error");
 			return;
 		}
-		seq_printf(f, "%s", str);
+		seq_puts(f, str);
 		kfree(str);
 	} else if (display_mode(ns, label, flags))
 		seq_printf(f, "%s (%s)", label->hname,
 			   label_modename(ns, label, flags));
 	else
-		seq_printf(f, "%s", label->hname);
+		seq_puts(f, label->hname);
 }
 
 void aa_label_xprintk(struct aa_ns *ns, struct aa_label *label, int flags,
-- 
cgit v1.2.3-59-g8ed1b


From e4f4e6ba5eaadb839d17cfe5235cff149a44b36a Mon Sep 17 00:00:00 2001
From: Vasyl Gomonovych <gomonovych@gmail.com>
Date: Fri, 26 Jul 2019 15:32:10 +0200
Subject: AppArmor: Remove semicolon

Remove unneeded semicolon

Signed-off-by: Vasyl Gomonovych <gomonovych@gmail.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/path.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/apparmor/path.c b/security/apparmor/path.c
index c6da542de27b..b02dfdbff7cd 100644
--- a/security/apparmor/path.c
+++ b/security/apparmor/path.c
@@ -142,7 +142,7 @@ static int d_namespace_path(const struct path *path, char *buf, char **name,
 			error = PTR_ERR(res);
 			*name = buf;
 			goto out;
-		};
+		}
 	} else if (!our_mnt(path->mnt))
 		connected = 0;
 
-- 
cgit v1.2.3-59-g8ed1b


From c659696964a7530ddd9ae075919b44f263fba05c Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Sat, 31 Aug 2019 15:55:06 -0700
Subject: apparmor: add a valid state flags check

Add a check to ensure only known state flags are set on each
state in the dfa.

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

diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
index e23f4aadc1ff..f280b046361e 100644
--- a/security/apparmor/include/match.h
+++ b/security/apparmor/include/match.h
@@ -181,5 +181,9 @@ static inline void aa_put_dfa(struct aa_dfa *dfa)
 
 #define MATCH_FLAG_DIFF_ENCODE 0x80000000
 #define MARK_DIFF_ENCODE 0x40000000
+#define MATCH_FLAG_OOB_TRANSITION 0x20000000
+#define MATCH_FLAGS_MASK 0xff000000
+#define MATCH_FLAGS_VALID MATCH_FLAG_DIFF_ENCODE
+#define MATCH_FLAGS_INVALID (MATCH_FLAGS_MASK & ~MATCH_FLAGS_VALID)
 
 #endif /* __AA_MATCH_H */
diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index 525ce22dc0e9..b477352305ed 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -202,6 +202,10 @@ static int verify_dfa(struct aa_dfa *dfa)
 		if (!(BASE_TABLE(dfa)[i] & MATCH_FLAG_DIFF_ENCODE) &&
 		    (DEFAULT_TABLE(dfa)[i] >= state_count))
 			goto out;
+		if (BASE_TABLE(dfa)[i] & MATCH_FLAGS_INVALID) {
+			pr_err("AppArmor DFA state with invalid match flags");
+			goto out;
+		}
 		if (base_idx(BASE_TABLE(dfa)[i]) + 255 >= trans_count) {
 			pr_err("AppArmor DFA next/check upper bounds error\n");
 			goto out;
-- 
cgit v1.2.3-59-g8ed1b


From dae6029325a4744e639eb048c13f53c24320aeda Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Sat, 31 Aug 2019 15:55:45 -0700
Subject: apparmor: add consistency check between state and dfa diff encode
 flags

Check that a states diff encode flag is only set if diff encode is
enabled in the dfa header.

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

diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index b477352305ed..651dbb6e38b8 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -206,6 +206,12 @@ static int verify_dfa(struct aa_dfa *dfa)
 			pr_err("AppArmor DFA state with invalid match flags");
 			goto out;
 		}
+		if ((BASE_TABLE(dfa)[i] & MATCH_FLAG_DIFF_ENCODE)) {
+			if (!(dfa->flags & YYTH_FLAG_DIFF_ENCODE)) {
+				pr_err("AppArmor DFA diff encoded transition state without header flag");
+				goto out;
+			}
+		}
 		if (base_idx(BASE_TABLE(dfa)[i]) + 255 >= trans_count) {
 			pr_err("AppArmor DFA next/check upper bounds error\n");
 			goto out;
-- 
cgit v1.2.3-59-g8ed1b


From 6413f852ce086c0f95817012c08d481ce24d8b1a Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 4 Feb 2019 05:23:14 -0800
Subject: apparmor: add proc subdir to attrs

This patch provides a /proc/<pid>/attr/apparmor/
subdirectory. Enabling userspace to use the apparmor attributes
without having to worry about collisions with selinux or smack on
interface files in /proc/<pid>/attr.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 fs/proc/base.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index ebea9501afb8..7bc192465e39 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2645,6 +2645,15 @@ static const struct pid_entry smack_attr_dir_stuff[] = {
 LSM_DIR_OPS(smack);
 #endif
 
+#ifdef CONFIG_SECURITY_APPARMOR
+static const struct pid_entry apparmor_attr_dir_stuff[] = {
+	ATTR("apparmor", "current",	0666),
+	ATTR("apparmor", "prev",	0444),
+	ATTR("apparmor", "exec",	0666),
+};
+LSM_DIR_OPS(apparmor);
+#endif
+
 static const struct pid_entry attr_dir_stuff[] = {
 	ATTR(NULL, "current",		0666),
 	ATTR(NULL, "prev",		0444),
@@ -2656,6 +2665,10 @@ static const struct pid_entry attr_dir_stuff[] = {
 	DIR("smack",			0555,
 	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
 #endif
+#ifdef CONFIG_SECURITY_APPARMOR
+	DIR("apparmor",			0555,
+	    proc_apparmor_attr_dir_inode_ops, proc_apparmor_attr_dir_ops),
+#endif
 };
 
 static int proc_attr_dir_readdir(struct file *file, struct dir_context *ctx)
-- 
cgit v1.2.3-59-g8ed1b


From a68d59ff4d67ec182926aaa82daaa66b3d465c9a Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Tue, 24 Sep 2019 09:46:33 -0700
Subject: apparmor: remove useless aafs_create_symlink

commit 1180b4c757aa ("apparmor: fix dangling symlinks to policy
rawdata after replacement") reworked how the rawdata symlink is
handled but failedto remove aafs_create_symlink which was reduced to a
useles stub.

Fixes: 1180b4c757aa ("apparmor: fix dangling symlinks to policy rawdata after replacement")
Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/apparmorfs.c | 50 ++++++++----------------------------------
 1 file changed, 9 insertions(+), 41 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 47aff8700547..be6dc548d307 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -340,38 +340,6 @@ static struct dentry *aafs_create_dir(const char *name, struct dentry *parent)
 			   NULL);
 }
 
-/**
- * aafs_create_symlink - create a symlink in the apparmorfs filesystem
- * @name: name of dentry to create
- * @parent: parent directory for this dentry
- * @target: if symlink, symlink target string
- * @private: private data
- * @iops: struct of inode_operations that should be used
- *
- * If @target parameter is %NULL, then the @iops parameter needs to be
- * setup to handle .readlink and .get_link inode_operations.
- */
-static struct dentry *aafs_create_symlink(const char *name,
-					  struct dentry *parent,
-					  const char *target,
-					  void *private,
-					  const struct inode_operations *iops)
-{
-	struct dentry *dent;
-	char *link = NULL;
-
-	if (target) {
-		if (!link)
-			return ERR_PTR(-ENOMEM);
-	}
-	dent = aafs_create(name, S_IFLNK | 0444, parent, private, link, NULL,
-			   iops);
-	if (IS_ERR(dent))
-		kfree(link);
-
-	return dent;
-}
-
 /**
  * aafs_remove - removes a file or directory from the apparmorfs filesystem
  *
@@ -1762,25 +1730,25 @@ int __aafs_profile_mkdir(struct aa_profile *profile, struct dentry *parent)
 	}
 
 	if (profile->rawdata) {
-		dent = aafs_create_symlink("raw_sha1", dir, NULL,
-					   profile->label.proxy,
-					   &rawdata_link_sha1_iops);
+		dent = aafs_create("raw_sha1", S_IFLNK | 0444, dir,
+				   profile->label.proxy, NULL, NULL,
+				   &rawdata_link_sha1_iops);
 		if (IS_ERR(dent))
 			goto fail;
 		aa_get_proxy(profile->label.proxy);
 		profile->dents[AAFS_PROF_RAW_HASH] = dent;
 
-		dent = aafs_create_symlink("raw_abi", dir, NULL,
-					   profile->label.proxy,
-					   &rawdata_link_abi_iops);
+		dent = aafs_create("raw_abi", S_IFLNK | 0444, dir,
+				   profile->label.proxy, NULL, NULL,
+				   &rawdata_link_abi_iops);
 		if (IS_ERR(dent))
 			goto fail;
 		aa_get_proxy(profile->label.proxy);
 		profile->dents[AAFS_PROF_RAW_ABI] = dent;
 
-		dent = aafs_create_symlink("raw_data", dir, NULL,
-					   profile->label.proxy,
-					   &rawdata_link_data_iops);
+		dent = aafs_create("raw_data", S_IFLNK | 0444, dir,
+				   profile->label.proxy, NULL, NULL,
+				   &rawdata_link_data_iops);
 		if (IS_ERR(dent))
 			goto fail;
 		aa_get_proxy(profile->label.proxy);
-- 
cgit v1.2.3-59-g8ed1b


From 3ed4aaa94fc07db3cd0c91be95e3e1b9782a2710 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Wed, 25 Sep 2019 08:02:48 -0700
Subject: apparmor: fix nnp subset test for unconfined

The subset test is not taking into account the unconfined exception
which will cause profile transitions in the stacked confinement
case to fail when no_new_privs is applied.

This fixes a regression introduced in the fix for
https://bugs.launchpad.net/bugs/1839037

BugLink: https://bugs.launchpad.net/bugs/1844186
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/domain.c        |  9 +++++----
 security/apparmor/include/label.h |  1 +
 security/apparmor/label.c         | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 6ceb74e0f789..f73ba303ba24 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -929,7 +929,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 	 * aways results in a further reduction of permissions.
 	 */
 	if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
-	    !unconfined(label) && !aa_label_is_subset(new, ctx->nnp)) {
+	    !unconfined(label) &&
+	    !aa_label_is_unconfined_subset(new, ctx->nnp)) {
 		error = -EPERM;
 		info = "no new privs";
 		goto audit;
@@ -1207,7 +1208,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags)
 		 * reduce restrictions.
 		 */
 		if (task_no_new_privs(current) && !unconfined(label) &&
-		    !aa_label_is_subset(new, ctx->nnp)) {
+		    !aa_label_is_unconfined_subset(new, ctx->nnp)) {
 			/* not an apparmor denial per se, so don't log it */
 			AA_DEBUG("no_new_privs - change_hat denied");
 			error = -EPERM;
@@ -1228,7 +1229,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags)
 		 * reduce restrictions.
 		 */
 		if (task_no_new_privs(current) && !unconfined(label) &&
-		    !aa_label_is_subset(previous, ctx->nnp)) {
+		    !aa_label_is_unconfined_subset(previous, ctx->nnp)) {
 			/* not an apparmor denial per se, so don't log it */
 			AA_DEBUG("no_new_privs - change_hat denied");
 			error = -EPERM;
@@ -1424,7 +1425,7 @@ check:
 		 * reduce restrictions.
 		 */
 		if (task_no_new_privs(current) && !unconfined(label) &&
-		    !aa_label_is_subset(new, ctx->nnp)) {
+		    !aa_label_is_unconfined_subset(new, ctx->nnp)) {
 			/* not an apparmor denial per se, so don't log it */
 			AA_DEBUG("no_new_privs - change_hat denied");
 			error = -EPERM;
diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h
index 47942c4ba7ca..255764ab06e2 100644
--- a/security/apparmor/include/label.h
+++ b/security/apparmor/include/label.h
@@ -281,6 +281,7 @@ bool aa_label_init(struct aa_label *label, int size, gfp_t gfp);
 struct aa_label *aa_label_alloc(int size, struct aa_proxy *proxy, gfp_t gfp);
 
 bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub);
+bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub);
 struct aa_profile *__aa_label_next_not_in_set(struct label_it *I,
 					     struct aa_label *set,
 					     struct aa_label *sub);
diff --git a/security/apparmor/label.c b/security/apparmor/label.c
index bb34094421c4..ba3987242282 100644
--- a/security/apparmor/label.c
+++ b/security/apparmor/label.c
@@ -550,6 +550,39 @@ bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub)
 	return __aa_label_next_not_in_set(&i, set, sub) == NULL;
 }
 
+/**
+ * aa_label_is_unconfined_subset - test if @sub is a subset of @set
+ * @set: label to test against
+ * @sub: label to test if is subset of @set
+ *
+ * This checks for subset but taking into account unconfined. IF
+ * @sub contains an unconfined profile that does not have a matching
+ * unconfined in @set then this will not cause the test to fail.
+ * Conversely we don't care about an unconfined in @set that is not in
+ * @sub
+ *
+ * Returns: true if @sub is special_subset of @set
+ *     else false
+ */
+bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub)
+{
+	struct label_it i = { };
+	struct aa_profile *p;
+
+	AA_BUG(!set);
+	AA_BUG(!sub);
+
+	if (sub == set)
+		return true;
+
+	do {
+		p = __aa_label_next_not_in_set(&i, set, sub);
+		if (p && !profile_unconfined(p))
+			break;
+	} while (p);
+
+	return p == NULL;
+}
 
 
 /**
-- 
cgit v1.2.3-59-g8ed1b


From f05841a940df995b784b5e3ec6f76141e8337245 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Thu, 19 Dec 2019 15:55:39 -0800
Subject: apparmor: fail unpack if profile mode is unknown

Profile unpack should fail if the profile mode is not a mode that the
kernel understands.

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

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 80364310fb1e..e4e329d69527 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -748,10 +748,14 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
 		goto fail;
 	if (tmp == PACKED_MODE_COMPLAIN || (e->version & FORCE_COMPLAIN_FLAG))
 		profile->mode = APPARMOR_COMPLAIN;
+	else if (tmp == PACKED_MODE_ENFORCE)
+		profile->mode = APPARMOR_ENFORCE;
 	else if (tmp == PACKED_MODE_KILL)
 		profile->mode = APPARMOR_KILL;
 	else if (tmp == PACKED_MODE_UNCONFINED)
 		profile->mode = APPARMOR_UNCONFINED;
+	else
+		goto fail;
 	if (!unpack_u32(e, &tmp, NULL))
 		goto fail;
 	if (tmp)
-- 
cgit v1.2.3-59-g8ed1b


From 0df34a645bae00c86f383fb063cd3840862837bf Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Tue, 30 Jul 2019 02:42:13 -0700
Subject: apparmor: add outofband transition and use it in xattr match

There are cases where the a special out of band transition that can
not be triggered by input is useful in separating match conditions
in the dfa encoding.

The null_transition is currently used as an out of band transition
for match conditions that can not contain a \0 in their input
but apparmor needs an out of band transition for cases where
the match condition is allowed to contain any input character.

Achieve this by allowing for an explicit transition out of input
range that can only be triggered by code.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/apparmorfs.c    |  4 +++-
 security/apparmor/domain.c        | 13 ++++++++----
 security/apparmor/include/match.h |  9 +++++++-
 security/apparmor/match.c         | 43 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index be6dc548d307..d7a179cdaa1e 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -591,7 +591,7 @@ static __poll_t ns_revision_poll(struct file *file, poll_table *pt)
 
 void __aa_bump_ns_revision(struct aa_ns *ns)
 {
-	WRITE_ONCE(ns->revision, ns->revision + 1);
+	WRITE_ONCE(ns->revision, READ_ONCE(ns->revision) + 1);
 	wake_up_interruptible(&ns->wait);
 }
 
@@ -2331,6 +2331,8 @@ static struct aa_sfs_entry aa_sfs_entry_versions[] = {
 static struct aa_sfs_entry aa_sfs_entry_policy[] = {
 	AA_SFS_DIR("versions",			aa_sfs_entry_versions),
 	AA_SFS_FILE_BOOLEAN("set_load",		1),
+	/* number of out of band transitions supported */
+	AA_SFS_FILE_U64("outofband",		MAX_OOB_SUPPORTED),
 	{ }
 };
 
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index f73ba303ba24..0a91d5f7d0e9 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -320,8 +320,7 @@ static int aa_xattrs_match(const struct linux_binprm *bprm,
 	might_sleep();
 
 	/* transition from exec match to xattr set */
-	state = aa_dfa_null_transition(profile->xmatch, state);
-
+	state = aa_dfa_outofband_transition(profile->xmatch, state);
 	d = bprm->file->f_path.dentry;
 
 	for (i = 0; i < profile->xattr_count; i++) {
@@ -330,7 +329,13 @@ static int aa_xattrs_match(const struct linux_binprm *bprm,
 		if (size >= 0) {
 			u32 perm;
 
-			/* Check the xattr value, not just presence */
+			/*
+			 * Check the xattr presence before value. This ensure
+			 * that not present xattr can be distinguished from a 0
+			 * length value or rule that matches any value
+			 */
+			state = aa_dfa_null_transition(profile->xmatch, state);
+			/* Check xattr value */
 			state = aa_dfa_match_len(profile->xmatch, state, value,
 						 size);
 			perm = dfa_user_allow(profile->xmatch, state);
@@ -340,7 +345,7 @@ static int aa_xattrs_match(const struct linux_binprm *bprm,
 			}
 		}
 		/* transition to next element */
-		state = aa_dfa_null_transition(profile->xmatch, state);
+		state = aa_dfa_outofband_transition(profile->xmatch, state);
 		if (size < 0) {
 			/*
 			 * No xattr match, so verify if transition to
diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
index f280b046361e..884489590588 100644
--- a/security/apparmor/include/match.h
+++ b/security/apparmor/include/match.h
@@ -37,6 +37,10 @@
 
 #define YYTH_MAGIC	0x1B5E783D
 #define YYTH_FLAG_DIFF_ENCODE	1
+#define YYTH_FLAG_OOB_TRANS	2
+#define YYTH_FLAGS (YYTH_FLAG_DIFF_ENCODE | YYTH_FLAG_OOB_TRANS)
+
+#define MAX_OOB_SUPPORTED	1
 
 struct table_set_header {
 	u32 th_magic;		/* YYTH_MAGIC */
@@ -94,6 +98,7 @@ struct table_header {
 struct aa_dfa {
 	struct kref count;
 	u16 flags;
+	u32 max_oob;
 	struct table_header *tables[YYTD_ID_TSIZE];
 };
 
@@ -127,6 +132,8 @@ unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start,
 			  const char *str);
 unsigned int aa_dfa_next(struct aa_dfa *dfa, unsigned int state,
 			 const char c);
+unsigned int aa_dfa_outofband_transition(struct aa_dfa *dfa,
+					 unsigned int state);
 unsigned int aa_dfa_match_until(struct aa_dfa *dfa, unsigned int start,
 				const char *str, const char **retpos);
 unsigned int aa_dfa_matchn_until(struct aa_dfa *dfa, unsigned int start,
@@ -183,7 +190,7 @@ static inline void aa_put_dfa(struct aa_dfa *dfa)
 #define MARK_DIFF_ENCODE 0x40000000
 #define MATCH_FLAG_OOB_TRANSITION 0x20000000
 #define MATCH_FLAGS_MASK 0xff000000
-#define MATCH_FLAGS_VALID MATCH_FLAG_DIFF_ENCODE
+#define MATCH_FLAGS_VALID (MATCH_FLAG_DIFF_ENCODE | MATCH_FLAG_OOB_TRANSITION)
 #define MATCH_FLAGS_INVALID (MATCH_FLAGS_MASK & ~MATCH_FLAGS_VALID)
 
 #endif /* __AA_MATCH_H */
diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index 651dbb6e38b8..e605b7d53fb4 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -212,6 +212,16 @@ static int verify_dfa(struct aa_dfa *dfa)
 				goto out;
 			}
 		}
+		if ((BASE_TABLE(dfa)[i] & MATCH_FLAG_OOB_TRANSITION)) {
+			if (base_idx(BASE_TABLE(dfa)[i]) < dfa->max_oob) {
+				pr_err("AppArmor DFA out of bad transition out of range");
+				goto out;
+			}
+			if (!(dfa->flags & YYTH_FLAG_OOB_TRANS)) {
+				pr_err("AppArmor DFA out of bad transition state without header flag");
+				goto out;
+			}
+		}
 		if (base_idx(BASE_TABLE(dfa)[i]) + 255 >= trans_count) {
 			pr_err("AppArmor DFA next/check upper bounds error\n");
 			goto out;
@@ -314,9 +324,23 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags)
 		goto fail;
 
 	dfa->flags = ntohs(*(__be16 *) (data + 12));
-	if (dfa->flags != 0 && dfa->flags != YYTH_FLAG_DIFF_ENCODE)
+	if (dfa->flags & ~(YYTH_FLAGS))
 		goto fail;
 
+	/*
+	 * TODO: needed for dfa to support more than 1 oob
+	 * if (dfa->flags & YYTH_FLAGS_OOB_TRANS) {
+	 *	if (hsize < 16 + 4)
+	 *		goto fail;
+	 *	dfa->max_oob = ntol(*(__be32 *) (data + 16));
+	 *	if (dfa->max <= MAX_OOB_SUPPORTED) {
+	 *		pr_err("AppArmor DFA OOB greater than supported\n");
+	 *		goto fail;
+	 *	}
+	 * }
+	 */
+	dfa->max_oob = 1;
+
 	data += hsize;
 	size -= hsize;
 
@@ -505,6 +529,23 @@ unsigned int aa_dfa_next(struct aa_dfa *dfa, unsigned int state,
 	return state;
 }
 
+unsigned int aa_dfa_outofband_transition(struct aa_dfa *dfa, unsigned int state)
+{
+	u16 *def = DEFAULT_TABLE(dfa);
+	u32 *base = BASE_TABLE(dfa);
+	u16 *next = NEXT_TABLE(dfa);
+	u16 *check = CHECK_TABLE(dfa);
+	u32 b = (base)[(state)];
+
+	if (!(b & MATCH_FLAG_OOB_TRANSITION))
+		return DFA_NOMATCH;
+
+	/* No Equivalence class remapping for outofband transitions */
+	match_char(state, def, base, next, check, -1);
+
+	return state;
+}
+
 /**
  * aa_dfa_match_until - traverse @dfa until accept state or end of input
  * @dfa: the dfa to match @str against  (NOT NULL)
-- 
cgit v1.2.3-59-g8ed1b


From 01df52d726b5d55d2970f5c957f1961930acd5d6 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Thu, 29 Aug 2019 11:35:50 -0700
Subject: apparmor: remove duplicate check of xattrs on profile attachment.

The second check to ensure the xattrs are present and checked is
unneeded as this is already done in the profile attachment xmatch.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/domain.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 0a91d5f7d0e9..0926553ca86f 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -625,8 +625,6 @@ static struct aa_label *profile_transition(struct aa_profile *profile,
 					   bool *secure_exec)
 {
 	struct aa_label *new = NULL;
-	struct aa_profile *component;
-	struct label_it i;
 	const char *info = NULL, *name = NULL, *target = NULL;
 	unsigned int state = profile->file.start;
 	struct aa_perms perms = {};
@@ -675,21 +673,6 @@ static struct aa_label *profile_transition(struct aa_profile *profile,
 			info = "profile transition not found";
 			/* remove MAY_EXEC to audit as failure */
 			perms.allow &= ~MAY_EXEC;
-		} else {
-			/* verify that each component's xattr requirements are
-			 * met, and fail execution otherwise
-			 */
-			label_for_each(i, new, component) {
-				if (aa_xattrs_match(bprm, component, state) <
-				    0) {
-					error = -EACCES;
-					info = "required xattrs not present";
-					perms.allow &= ~MAY_EXEC;
-					aa_put_label(new);
-					new = NULL;
-					goto audit;
-				}
-			}
 		}
 	} else if (COMPLAIN_MODE(profile)) {
 		/* no exec permission - learning mode */
-- 
cgit v1.2.3-59-g8ed1b


From c27c6bd2c4d6b6bb779f9b722d5607993e1d5e5c Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Mon, 30 Mar 2020 23:37:54 -0700
Subject: apparmor: ensure that dfa state tables have entries

Currently it is possible to specify a state machine table with 0 length,
this is not valid as optional tables are specified by not defining
the table as present. Further this allows by-passing the base tables
range check against the next/check tables.

Fixes: d901d6a298dc ("apparmor: dfa split verification of table headers")
Reported-by: Mike Salvatore <mike.salvatore@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/match.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index e605b7d53fb4..3e9e1eaf990e 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -97,6 +97,9 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
 	      th.td_flags == YYTD_DATA8))
 		goto out;
 
+	/* if we have a table it must have some entries */
+	if (th.td_lolen == 0)
+		goto out;
 	tsize = table_size(th.td_lolen, th.td_flags);
 	if (bsize < tsize)
 		goto out;
@@ -198,6 +201,8 @@ static int verify_dfa(struct aa_dfa *dfa)
 
 	state_count = dfa->tables[YYTD_ID_BASE]->td_lolen;
 	trans_count = dfa->tables[YYTD_ID_NXT]->td_lolen;
+	if (state_count == 0)
+		goto out;
 	for (i = 0; i < state_count; i++) {
 		if (!(BASE_TABLE(dfa)[i] & MATCH_FLAG_DIFF_ENCODE) &&
 		    (DEFAULT_TABLE(dfa)[i] >= state_count))
-- 
cgit v1.2.3-59-g8ed1b


From fe9fd23e3b587c7ca9520717b213f88050c1d324 Mon Sep 17 00:00:00 2001
From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Date: Thu, 7 May 2020 13:43:56 -0500
Subject: apparmor: Replace zero-length array with flexible-array

The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:

struct foo {
        int stuff;
        struct boo array[];
};

By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.

Also, notice that, dynamic memory allocations won't be affected by
this change:

"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]

sizeof(flexible-array-member) triggers a warning because flexible array
members have incomplete type[1]. There are some instances of code in
which the sizeof operator is being incorrectly/erroneously applied to
zero-length arrays and the result is zero. Such instances may be hiding
some bugs. So, this work (flexible-array member conversions) will also
help to get completely rid of those sorts of issues.

This issue was found with the help of Coccinelle.

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/apparmorfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index d7a179cdaa1e..d65324415980 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -807,7 +807,7 @@ static ssize_t query_label(char *buf, size_t buf_len,
 struct multi_transaction {
 	struct kref count;
 	ssize_t size;
-	char data[0];
+	char data[];
 };
 
 #define MULTI_TRANSACTION_LIMIT (PAGE_SIZE - sizeof(struct multi_transaction))
-- 
cgit v1.2.3-59-g8ed1b


From c84b80cd41e05395655459ecc652fa5ee05c257e Mon Sep 17 00:00:00 2001
From: Mateusz Nosek <mateusznosek0@gmail.com>
Date: Tue, 3 Mar 2020 19:30:23 +0100
Subject: security/apparmor/label.c: Clean code by removing redundant
 instructions

Previously 'label->proxy->label' value checking
and conditional reassigning were done twice in the same function.
The second one is redundant and can be removed.

Signed-off-by: Mateusz Nosek <mateusznosek0@gmail.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/label.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/security/apparmor/label.c b/security/apparmor/label.c
index ba3987242282..676eebcbfd68 100644
--- a/security/apparmor/label.c
+++ b/security/apparmor/label.c
@@ -311,8 +311,6 @@ out:
 
 static void label_destroy(struct aa_label *label)
 {
-	struct aa_label *tmp;
-
 	AA_BUG(!label);
 
 	if (!label_isprofile(label)) {
@@ -333,10 +331,6 @@ static void label_destroy(struct aa_label *label)
 
 	aa_free_secid(label->secid);
 
-	tmp = rcu_dereference_protected(label->proxy->label, true);
-	if (tmp == label)
-		rcu_assign_pointer(label->proxy->label, NULL);
-
 	aa_put_proxy(label->proxy);
 	label->proxy = (struct aa_proxy *) PROXY_POISON + 1;
 }
-- 
cgit v1.2.3-59-g8ed1b


From e37986097ba63c94b1af9d5ad5486d120a809f72 Mon Sep 17 00:00:00 2001
From: Zou Wei <zou_wei@huawei.com>
Date: Tue, 28 Apr 2020 19:52:21 +0800
Subject: apparmor: Use true and false for bool variable

Fixes coccicheck warnings:

security/apparmor/file.c:162:9-10: WARNING: return of 0/1 in function 'is_deleted' with return type bool
security/apparmor/file.c:362:9-10: WARNING: return of 0/1 in function 'xindex_is_subset' with return type bool
security/apparmor/policy_unpack.c:246:9-10: WARNING: return of 0/1 in function 'unpack_X' with return type bool
security/apparmor/policy_unpack.c:292:9-10: WARNING: return of 0/1 in function 'unpack_nameX' with return type bool
security/apparmor/policy_unpack.c:646:8-9: WARNING: return of 0/1 in function 'unpack_rlimits' with return type bool
security/apparmor/policy_unpack.c:604:8-9: WARNING: return of 0/1 in function 'unpack_secmark' with return type bool
security/apparmor/policy_unpack.c:538:8-9: WARNING: return of 0/1 in function 'unpack_trans_table' with return type bool
security/apparmor/policy_unpack.c:327:9-10: WARNING: return of 0/1 in function 'unpack_u32' with return type bool
security/apparmor/policy_unpack.c:345:9-10: WARNING: return of 0/1 in function 'unpack_u64' with return type bool
security/apparmor/policy_unpack.c:309:9-10: WARNING: return of 0/1 in function 'unpack_u8' with return type bool
security/apparmor/policy_unpack.c:568:8-9: WARNING: return of 0/1 in function 'unpack_xattrs' with return type bool
security/apparmor/policy_unpack.c:1007:10-11: WARNING: return of 0/1 in function 'verify_dfa_xindex' with return type bool
security/apparmor/policy_unpack.c:997:9-10: WARNING: return of 0/1 in function 'verify_xindex' with return type bool

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zou Wei <zou_wei@huawei.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/file.c          | 12 ++++-----
 security/apparmor/policy_unpack.c | 54 +++++++++++++++++++--------------------
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index f1caf3674e1c..9a2d14b7c9f8 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -154,13 +154,13 @@ int aa_audit_file(struct aa_profile *profile, struct aa_perms *perms,
  * is_deleted - test if a file has been completely unlinked
  * @dentry: dentry of file to test for deletion  (NOT NULL)
  *
- * Returns: %1 if deleted else %0
+ * Returns: true if deleted else false
  */
 static inline bool is_deleted(struct dentry *dentry)
 {
 	if (d_unlinked(dentry) && d_backing_inode(dentry)->i_nlink == 0)
-		return 1;
-	return 0;
+		return true;
+	return false;
 }
 
 static int path_name(const char *op, struct aa_label *label,
@@ -353,15 +353,15 @@ int aa_path_perm(const char *op, struct aa_label *label,
  * this is done as part of the subset test, where a hardlink must have
  * a subset of permissions that the target has.
  *
- * Returns: %1 if subset else %0
+ * Returns: true if subset else false
  */
 static inline bool xindex_is_subset(u32 link, u32 target)
 {
 	if (((link & ~AA_X_UNSAFE) != (target & ~AA_X_UNSAFE)) ||
 	    ((link & AA_X_UNSAFE) && !(target & AA_X_UNSAFE)))
-		return 0;
+		return false;
 
-	return 1;
+	return true;
 }
 
 static int profile_path_link(struct aa_profile *profile,
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index e4e329d69527..d9ef9a99c26e 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -243,11 +243,11 @@ fail:
 static bool unpack_X(struct aa_ext *e, enum aa_code code)
 {
 	if (!inbounds(e, 1))
-		return 0;
+		return false;
 	if (*(u8 *) e->pos != code)
-		return 0;
+		return false;
 	e->pos++;
-	return 1;
+	return true;
 }
 
 /**
@@ -261,10 +261,10 @@ static bool unpack_X(struct aa_ext *e, enum aa_code code)
  * name element in the stream.  If @name is NULL any name element will be
  * skipped and only the typecode will be tested.
  *
- * Returns 1 on success (both type code and name tests match) and the read
+ * Returns true on success (both type code and name tests match) and the read
  * head is advanced past the headers
  *
- * Returns: 0 if either match fails, the read head does not move
+ * Returns: false if either match fails, the read head does not move
  */
 static bool unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name)
 {
@@ -289,11 +289,11 @@ static bool unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name)
 
 	/* now check if type code matches */
 	if (unpack_X(e, code))
-		return 1;
+		return true;
 
 fail:
 	e->pos = pos;
-	return 0;
+	return false;
 }
 
 static bool unpack_u8(struct aa_ext *e, u8 *data, const char *name)
@@ -306,12 +306,12 @@ static bool unpack_u8(struct aa_ext *e, u8 *data, const char *name)
 		if (data)
 			*data = get_unaligned((u8 *)e->pos);
 		e->pos += sizeof(u8);
-		return 1;
+		return true;
 	}
 
 fail:
 	e->pos = pos;
-	return 0;
+	return false;
 }
 
 static bool unpack_u32(struct aa_ext *e, u32 *data, const char *name)
@@ -324,12 +324,12 @@ static bool unpack_u32(struct aa_ext *e, u32 *data, const char *name)
 		if (data)
 			*data = le32_to_cpu(get_unaligned((__le32 *) e->pos));
 		e->pos += sizeof(u32);
-		return 1;
+		return true;
 	}
 
 fail:
 	e->pos = pos;
-	return 0;
+	return false;
 }
 
 static bool unpack_u64(struct aa_ext *e, u64 *data, const char *name)
@@ -342,12 +342,12 @@ static bool unpack_u64(struct aa_ext *e, u64 *data, const char *name)
 		if (data)
 			*data = le64_to_cpu(get_unaligned((__le64 *) e->pos));
 		e->pos += sizeof(u64);
-		return 1;
+		return true;
 	}
 
 fail:
 	e->pos = pos;
-	return 0;
+	return false;
 }
 
 static size_t unpack_array(struct aa_ext *e, const char *name)
@@ -472,7 +472,7 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e)
  * @e: serialized data extent information  (NOT NULL)
  * @profile: profile to add the accept table to (NOT NULL)
  *
- * Returns: 1 if table successfully unpacked
+ * Returns: true if table successfully unpacked
  */
 static bool unpack_trans_table(struct aa_ext *e, struct aa_profile *profile)
 {
@@ -535,12 +535,12 @@ static bool unpack_trans_table(struct aa_ext *e, struct aa_profile *profile)
 		if (!unpack_nameX(e, AA_STRUCTEND, NULL))
 			goto fail;
 	}
-	return 1;
+	return true;
 
 fail:
 	aa_free_domain_entries(&profile->file.trans);
 	e->pos = saved_pos;
-	return 0;
+	return false;
 }
 
 static bool unpack_xattrs(struct aa_ext *e, struct aa_profile *profile)
@@ -565,11 +565,11 @@ static bool unpack_xattrs(struct aa_ext *e, struct aa_profile *profile)
 			goto fail;
 	}
 
-	return 1;
+	return true;
 
 fail:
 	e->pos = pos;
-	return 0;
+	return false;
 }
 
 static bool unpack_secmark(struct aa_ext *e, struct aa_profile *profile)
@@ -601,7 +601,7 @@ static bool unpack_secmark(struct aa_ext *e, struct aa_profile *profile)
 			goto fail;
 	}
 
-	return 1;
+	return true;
 
 fail:
 	if (profile->secmark) {
@@ -613,7 +613,7 @@ fail:
 	}
 
 	e->pos = pos;
-	return 0;
+	return false;
 }
 
 static bool unpack_rlimits(struct aa_ext *e, struct aa_profile *profile)
@@ -643,11 +643,11 @@ static bool unpack_rlimits(struct aa_ext *e, struct aa_profile *profile)
 		if (!unpack_nameX(e, AA_STRUCTEND, NULL))
 			goto fail;
 	}
-	return 1;
+	return true;
 
 fail:
 	e->pos = pos;
-	return 0;
+	return false;
 }
 
 static u32 strhash(const void *data, u32 len, u32 seed)
@@ -994,8 +994,8 @@ static bool verify_xindex(int xindex, int table_size)
 	xtype = xindex & AA_X_TYPE_MASK;
 	index = xindex & AA_X_INDEX_MASK;
 	if (xtype == AA_X_TABLE && index >= table_size)
-		return 0;
-	return 1;
+		return false;
+	return true;
 }
 
 /* verify dfa xindexes are in range of transition tables */
@@ -1004,11 +1004,11 @@ static bool verify_dfa_xindex(struct aa_dfa *dfa, int table_size)
 	int i;
 	for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) {
 		if (!verify_xindex(dfa_user_xindex(dfa, i), table_size))
-			return 0;
+			return false;
 		if (!verify_xindex(dfa_other_xindex(dfa, i), table_size))
-			return 0;
+			return false;
 	}
-	return 1;
+	return true;
 }
 
 /**
-- 
cgit v1.2.3-59-g8ed1b


From 3b646abc5bc6c0df649daea4c2c976bd4d47e4c8 Mon Sep 17 00:00:00 2001
From: Mauricio Faria de Oliveira <mfo@canonical.com>
Date: Tue, 2 Jun 2020 18:15:16 -0300
Subject: apparmor: check/put label on apparmor_sk_clone_security()

Currently apparmor_sk_clone_security() does not check for existing
label/peer in the 'new' struct sock; it just overwrites it, if any
(with another reference to the label of the source sock.)

    static void apparmor_sk_clone_security(const struct sock *sk,
                                           struct sock *newsk)
    {
            struct aa_sk_ctx *ctx = SK_CTX(sk);
            struct aa_sk_ctx *new = SK_CTX(newsk);

            new->label = aa_get_label(ctx->label);
            new->peer = aa_get_label(ctx->peer);
    }

This might leak label references, which might overflow under load.
Thus, check for and put labels, to prevent such errors.

Note this is similarly done on:

    static int apparmor_socket_post_create(struct socket *sock, ...)
    ...
            if (sock->sk) {
                    struct aa_sk_ctx *ctx = SK_CTX(sock->sk);

                    aa_put_label(ctx->label);
                    ctx->label = aa_get_label(label);
            }
    ...

Context:
-------

The label reference count leak is observed if apparmor_sock_graft()
is called previously: this sets the 'ctx->label' field by getting
a reference to the current label (later overwritten, without put.)

    static void apparmor_sock_graft(struct sock *sk, ...)
    {
            struct aa_sk_ctx *ctx = SK_CTX(sk);

            if (!ctx->label)
                    ctx->label = aa_get_current_label();
    }

And that is the case on crypto/af_alg.c:af_alg_accept():

    int af_alg_accept(struct sock *sk, struct socket *newsock, ...)
    ...
            struct sock *sk2;
            ...
            sk2 = sk_alloc(...);
            ...
            security_sock_graft(sk2, newsock);
            security_sk_clone(sk, sk2);
    ...

Apparently both calls are done on their own right, especially for
other LSMs, being introduced in 2010/2014, before apparmor socket
mediation in 2017 (see commits [1,2,3,4]).

So, it looks OK there! Let's fix the reference leak in apparmor.

Test-case:
---------

Exercise that code path enough to overflow label reference count.

    $ cat aa-refcnt-af_alg.c
    #include <stdio.h>
    #include <string.h>
    #include <unistd.h>
    #include <sys/socket.h>
    #include <linux/if_alg.h>

    int main() {
            int sockfd;
            struct sockaddr_alg sa;

            /* Setup the crypto API socket */
            sockfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
            if (sockfd < 0) {
                    perror("socket");
                    return 1;
            }

            memset(&sa, 0, sizeof(sa));
            sa.salg_family = AF_ALG;
            strcpy((char *) sa.salg_type, "rng");
            strcpy((char *) sa.salg_name, "stdrng");

            if (bind(sockfd, (struct sockaddr *) &sa, sizeof(sa)) < 0) {
                    perror("bind");
                    return 1;
            }

            /* Accept a "connection" and close it; repeat. */
            while (!close(accept(sockfd, NULL, 0)));

            return 0;
    }

    $ gcc -o aa-refcnt-af_alg aa-refcnt-af_alg.c

    $ ./aa-refcnt-af_alg
    <a few hours later>

    [ 9928.475953] refcount_t overflow at apparmor_sk_clone_security+0x37/0x70 in aa-refcnt-af_alg[1322], uid/euid: 1000/1000
    ...
    [ 9928.507443] RIP: 0010:apparmor_sk_clone_security+0x37/0x70
    ...
    [ 9928.514286]  security_sk_clone+0x33/0x50
    [ 9928.514807]  af_alg_accept+0x81/0x1c0 [af_alg]
    [ 9928.516091]  alg_accept+0x15/0x20 [af_alg]
    [ 9928.516682]  SYSC_accept4+0xff/0x210
    [ 9928.519609]  SyS_accept+0x10/0x20
    [ 9928.520190]  do_syscall_64+0x73/0x130
    [ 9928.520808]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2

Note that other messages may be seen, not just overflow, depending on
the value being incremented by kref_get(); on another run:

    [ 7273.182666] refcount_t: saturated; leaking memory.
    ...
    [ 7273.185789] refcount_t: underflow; use-after-free.

Kprobes:
-------

Using kprobe events to monitor sk -> sk_security -> label -> count (kref):

Original v5.7 (one reference leak every iteration)

 ... (af_alg_accept+0x0/0x1c0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd2
 ... (af_alg_release_parent+0x0/0xd0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd4
 ... (af_alg_accept+0x0/0x1c0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd3
 ... (af_alg_release_parent+0x0/0xd0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd5
 ... (af_alg_accept+0x0/0x1c0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd4
 ... (af_alg_release_parent+0x0/0xd0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd6

Patched v5.7 (zero reference leak per iteration)

 ... (af_alg_accept+0x0/0x1c0) label=0xffff9ff376c25eb0 label_refcnt=0x593
 ... (af_alg_release_parent+0x0/0xd0) label=0xffff9ff376c25eb0 label_refcnt=0x594
 ... (af_alg_accept+0x0/0x1c0) label=0xffff9ff376c25eb0 label_refcnt=0x593
 ... (af_alg_release_parent+0x0/0xd0) label=0xffff9ff376c25eb0 label_refcnt=0x594
 ... (af_alg_accept+0x0/0x1c0) label=0xffff9ff376c25eb0 label_refcnt=0x593
 ... (af_alg_release_parent+0x0/0xd0) label=0xffff9ff376c25eb0 label_refcnt=0x594

Commits:
-------

[1] commit 507cad355fc9 ("crypto: af_alg - Make sure sk_security is initialized on accept()ed sockets")
[2] commit 4c63f83c2c2e ("crypto: af_alg - properly label AF_ALG socket")
[3] commit 2acce6aa9f65 ("Networking") a.k.a ("crypto: af_alg - Avoid sock_graft call warning)
[4] commit 56974a6fcfef ("apparmor: add base infastructure for socket mediation")

Fixes: 56974a6fcfef ("apparmor: add base infastructure for socket mediation")
Reported-by: Brian Moyles <bmoyles@netflix.com>
Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/lsm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index b621ad74f54a..66a8504c8bea 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -804,7 +804,12 @@ static void apparmor_sk_clone_security(const struct sock *sk,
 	struct aa_sk_ctx *ctx = SK_CTX(sk);
 	struct aa_sk_ctx *new = SK_CTX(newsk);
 
+	if (new->label)
+		aa_put_label(new->label);
 	new->label = aa_get_label(ctx->label);
+
+	if (new->peer)
+		aa_put_label(new->peer);
 	new->peer = aa_get_label(ctx->peer);
 }
 
-- 
cgit v1.2.3-59-g8ed1b


From dd2569fbb053719f7df7ef8fdbb45cf47156a701 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Fri, 5 Jun 2020 18:12:21 -0700
Subject: apparmor: fix introspection of of task mode for unconfined tasks

Fix two issues with introspecting the task mode.

1. If a task is attached to a unconfined profile that is not the
   ns->unconfined profile then. Mode the mode is always reported
   as -

      $ ps -Z
      LABEL                               PID TTY          TIME CMD
      unconfined                         1287 pts/0    00:00:01 bash
      test (-)                           1892 pts/0    00:00:00 ps

   instead of the correct value of (unconfined) as shown below

      $ ps -Z
      LABEL                               PID TTY          TIME CMD
      unconfined                         2483 pts/0    00:00:01 bash
      test (unconfined)                  3591 pts/0    00:00:00 ps

2. if a task is confined by a stack of profiles that are unconfined
   the output of label mode is again the incorrect value of (-) like
   above, instead of (unconfined). This is because the visibile
   profile count increment is skipped by the special casing of
   unconfined.

Fixes: f1bd904175e8 ("apparmor: add the base fns() for domain labels")
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/label.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/apparmor/label.c b/security/apparmor/label.c
index 676eebcbfd68..23f7a193df4f 100644
--- a/security/apparmor/label.c
+++ b/security/apparmor/label.c
@@ -1558,13 +1558,13 @@ static const char *label_modename(struct aa_ns *ns, struct aa_label *label,
 
 	label_for_each(i, label, profile) {
 		if (aa_ns_visible(ns, profile->ns, flags & FLAG_VIEW_SUBNS)) {
-			if (profile->mode == APPARMOR_UNCONFINED)
+			count++;
+			if (profile == profile->ns->unconfined)
 				/* special case unconfined so stacks with
 				 * unconfined don't report as mixed. ie.
 				 * profile_foo//&:ns1:unconfined (mixed)
 				 */
 				continue;
-			count++;
 			if (mode == -1)
 				mode = profile->mode;
 			else if (mode != profile->mode)
-- 
cgit v1.2.3-59-g8ed1b


From 3622ad25d4d68fcbdef3bc084b5916873e785344 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Sun, 7 Jun 2020 04:10:33 -0700
Subject: apparmor: Fix memory leak of profile proxy

When the proxy isn't replaced and the profile is removed, the proxy
is being leaked resulting in a kmemleak check message of

unreferenced object 0xffff888077a3a490 (size 16):
  comm "apparmor_parser", pid 128041, jiffies 4322684109 (age 1097.028s)
  hex dump (first 16 bytes):
    03 00 00 00 00 00 00 00 b0 92 fd 4b 81 88 ff ff  ...........K....
  backtrace:
    [<0000000084d5daf2>] aa_alloc_proxy+0x58/0xe0
    [<00000000ecc0e21a>] aa_alloc_profile+0x159/0x1a0
    [<000000004cc9ce15>] unpack_profile+0x275/0x1c40
    [<000000007332b3ca>] aa_unpack+0x1e7/0x7e0
    [<00000000e25e31bd>] aa_replace_profiles+0x18a/0x1d10
    [<00000000350d9415>] policy_update+0x237/0x650
    [<000000003fbf934e>] profile_load+0x122/0x160
    [<0000000047f7b781>] vfs_write+0x139/0x290
    [<000000008ad12358>] ksys_write+0xcd/0x170
    [<000000001a9daa7b>] do_syscall_64+0x70/0x310
    [<00000000b9efb0cf>] entry_SYSCALL_64_after_hwframe+0x49/0xb3

Make sure to cleanup the profile's embedded label which will result
on the proxy being properly freed.

Fixes: 637f688dc3dc ("apparmor: switch from profiles to using labels on contexts")
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/include/label.h |  1 +
 security/apparmor/label.c         | 13 +++++++------
 security/apparmor/policy.c        |  1 +
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h
index 255764ab06e2..1e90384b1523 100644
--- a/security/apparmor/include/label.h
+++ b/security/apparmor/include/label.h
@@ -275,6 +275,7 @@ void aa_labelset_destroy(struct aa_labelset *ls);
 void aa_labelset_init(struct aa_labelset *ls);
 void __aa_labelset_update_subtree(struct aa_ns *ns);
 
+void aa_label_destroy(struct aa_label *label);
 void aa_label_free(struct aa_label *label);
 void aa_label_kref(struct kref *kref);
 bool aa_label_init(struct aa_label *label, int size, gfp_t gfp);
diff --git a/security/apparmor/label.c b/security/apparmor/label.c
index 23f7a193df4f..e68bcedca976 100644
--- a/security/apparmor/label.c
+++ b/security/apparmor/label.c
@@ -309,7 +309,7 @@ out:
 }
 
 
-static void label_destroy(struct aa_label *label)
+void aa_label_destroy(struct aa_label *label)
 {
 	AA_BUG(!label);
 
@@ -326,12 +326,13 @@ static void label_destroy(struct aa_label *label)
 		}
 	}
 
-	if (rcu_dereference_protected(label->proxy->label, true) == label)
-		rcu_assign_pointer(label->proxy->label, NULL);
-
+	if (label->proxy) {
+		if (rcu_dereference_protected(label->proxy->label, true) == label)
+			rcu_assign_pointer(label->proxy->label, NULL);
+		aa_put_proxy(label->proxy);
+	}
 	aa_free_secid(label->secid);
 
-	aa_put_proxy(label->proxy);
 	label->proxy = (struct aa_proxy *) PROXY_POISON + 1;
 }
 
@@ -340,7 +341,7 @@ void aa_label_free(struct aa_label *label)
 	if (!label)
 		return;
 
-	label_destroy(label);
+	aa_label_destroy(label);
 	kfree(label);
 }
 
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 269f2f53c0b1..af4f50fda9e3 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -242,6 +242,7 @@ void aa_free_profile(struct aa_profile *profile)
 
 	kzfree(profile->hash);
 	aa_put_loaddata(profile->rawdata);
+	aa_label_destroy(&profile->label);
 
 	kzfree(profile);
 }
-- 
cgit v1.2.3-59-g8ed1b