[pve-devel] [PATCH v2 kernel 1/3] Fix CVE-2016-6187: AppArmor oops in apparmor_setprocattr

[Wolfgang Bumiller]

---
 ...x-oops-validate-buffer-size-in-apparmor_s.patch | 120 +++++++++++++++++++++
 Makefile                                           |   1 +
 2 files changed, 121 insertions(+)
 create mode 100644 CVE-2016-6187-apparmor-fix-oops-validate-buffer-size-in-apparmor_s.patch

diff --git a/CVE-2016-6187-apparmor-fix-oops-validate-buffer-size-in-apparmor_s.patch b/CVE-2016-6187-apparmor-fix-oops-validate-buffer-size-in-apparmor_s.patch
new file mode 100644
index 0000000..001da87
--- /dev/null
+++ b/CVE-2016-6187-apparmor-fix-oops-validate-buffer-size-in-apparmor_s.patch
@@ -0,0 +1,120 @@
+From 30a46a4647fd1df9cf52e43bf467f0d9265096ca Mon Sep 17 00:00:00 2001
+From: Vegard Nossum <vegard.nossum at oracle.com>
+Date: Thu, 7 Jul 2016 13:41:11 -0700
+Subject: [PATCH] apparmor: fix oops, validate buffer size in
+ apparmor_setprocattr()
+
+When proc_pid_attr_write() was changed to use memdup_user apparmor's
+(interface violating) assumption that the setprocattr buffer was always
+a single page was violated.
+
+The size test is not strictly speaking needed as proc_pid_attr_write()
+will reject anything larger, but for the sake of robustness we can keep
+it in.
+
+SMACK and SELinux look safe to me, but somebody else should probably
+have a look just in case.
+
+Based on original patch from Vegard Nossum <vegard.nossum at oracle.com>
+modified for the case that apparmor provides null termination.
+
+Fixes: bb646cdb12e75d82258c2f2e7746d5952d3e321a
+Reported-by: Vegard Nossum <vegard.nossum at oracle.com>
+Cc: Al Viro <viro at zeniv.linux.org.uk>
+Cc: John Johansen <john.johansen at canonical.com>
+Cc: Paul Moore <paul at paul-moore.com>
+Cc: Stephen Smalley <sds at tycho.nsa.gov>
+Cc: Eric Paris <eparis at parisplace.org>
+Cc: Casey Schaufler <casey at schaufler-ca.com>
+Cc: stable at kernel.org
+Signed-off-by: John Johansen <john.johansen at canonical.com>
+Reviewed-by: Tyler Hicks <tyhicks at canonical.com>
+Signed-off-by: James Morris <james.l.morris at oracle.com>
+---
+ security/apparmor/lsm.c | 36 +++++++++++++++++++-----------------
+ 1 file changed, 19 insertions(+), 17 deletions(-)
+
+diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
+index 2660fbc..7798e16 100644
+--- a/security/apparmor/lsm.c
++++ b/security/apparmor/lsm.c
+@@ -500,35 +500,35 @@ static int apparmor_setprocattr(struct task_struct *task, char *name,
+ static int apparmor_setprocattr(struct task_struct *task, char *name,
+ 				void *value, size_t size)
+ {
+-	char *command, *args = value;
++	char *command, *largs = NULL, *args = value;
+ 	size_t arg_size;
+ 	int error;
+ 	DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_SETPROCATTR);
+ 
+ 	if (size == 0)
+ 		return -EINVAL;
+-	/* args points to a PAGE_SIZE buffer, AppArmor requires that
+-	 * the buffer must be null terminated or have size <= PAGE_SIZE -1
+-	 * so that AppArmor can null terminate them
+-	 */
+-	if (args[size - 1] != '\0') {
+-		if (size == PAGE_SIZE)
+-			return -EINVAL;
+-		args[size] = '\0';
+-	}
+-
+ 	/* task can only write its own attributes */
+ 	if (current != task)
+ 		return -EACCES;
+ 
+-	args = value;
++	/* AppArmor requires that the buffer must be null terminated atm */
++	if (args[size - 1] != '\0') {
++		/* null terminate */
++		largs = args = kmalloc(size + 1, GFP_KERNEL);
++		if (!args)
++			return -ENOMEM;
++		memcpy(args, value, size);
++		args[size] = '\0';
++	}
++
++	error = -EINVAL;
+ 	args = strim(args);
+ 	command = strsep(&args, " ");
+ 	if (!args)
+-		return -EINVAL;
++		goto out;
+ 	args = skip_spaces(args);
+ 	if (!*args)
+-		return -EINVAL;
++		goto out;
+ 
+ 	arg_size = size - (args - (char *) value);
+ 	if (strcmp(name, "current") == 0) {
+@@ -553,10 +553,12 @@ static int apparmor_setprocattr(struct task_struct *task, char *name,
+ 			goto fail;
+ 	} else
+ 		/* only support the "current" and "exec" process attributes */
+-		return -EINVAL;
++		goto fail;
+ 
+ 	if (!error)
+ 		error = size;
++out:
++	kfree(largs);
+ 	return error;
+ 
+ fail:
+@@ -565,10 +567,10 @@ fail:
+ fail:
+ 	aad(&sa)->label = aa_begin_current_label(DO_UPDATE);
+ 	aad(&sa)->info = name;
+-	aad(&sa)->error = -EINVAL;
++	aad(&sa)->error = error = -EINVAL;
+ 	aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL);
+ 	aa_end_current_label(aad(&sa)->label);
+-	return -EINVAL;
++	goto out;
+ }
+ 
+ /**
+-- 
+2.1.4
+
diff --git a/Makefile b/Makefile
index 5c3df76..1496d37 100644
--- a/Makefile
+++ b/Makefile
@@ -255,6 +255,7 @@ ${KERNEL_SRC}/README ${KERNEL_CFG_ORG}: ${KERNELSRCTAR}
 	cd ${KERNEL_SRC}; patch -p1 < ../981-1-PCI-Reverse-standard-ACS-vs-device-specific-ACS-enabling.patch
 	cd ${KERNEL_SRC}; patch -p1 < ../981-2-PCI-Quirk-PCH-root-port-ACS-for-Sunrise-Point.patch
 	cd ${KERNEL_SRC}; patch -p1 < ../kvm-dynamic-halt-polling-disable-default.patch
+	cd ${KERNEL_SRC}; patch -p1 < ../CVE-2016-6187-apparmor-fix-oops-validate-buffer-size-in-apparmor_s.patch
 	sed -i ${KERNEL_SRC}/Makefile -e 's/^EXTRAVERSION.*$$/EXTRAVERSION=${EXTRAVERSION}/'
 	touch $@
 
-- 
2.1.4