Skip to contentSkip to conversation

Patch "apparmor: fix oops, validate buffer size in apparmor_setprocattr()" has been added to the 4.6-stable tree

PATCH [email protected] 1 participant 1 message Jul 24, 2016

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

apparmor: fix oops, validate buffer size in apparmor_setprocattr()

to the 4.6-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
apparmor-fix-oops-validate-buffer-size-in-apparmor_setprocattr.patch
and it can be found in the queue-4.6 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 30a46a4647fd1df9cf52e43bf467f0d9265096ca Mon Sep 17 00:00:00 2001

From: Vegard Nossum <[email protected]>
Date: Thu, 7 Jul 2016 13:41:11 -0700
Subject: apparmor: fix oops, validate buffer size in apparmor_setprocattr()

From: Vegard Nossum <[email protected]>

commit 30a46a4647fd1df9cf52e43bf467f0d9265096ca upstream.

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 <[email protected]> modified for the case that apparmor provides null termination.

Fixes bb646cdb12e75d82258c2f2e7746d5952d3e321a
Cc Al Viro <[email protected]>
Cc John Johansen <[email protected]>
Cc Paul Moore <[email protected]>
Cc Stephen Smalley <[email protected]>
Cc Eric Paris <[email protected]>
Cc Casey Schaufler <[email protected]>
---
 security/apparmor/lsm.c |   36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)
security/apparmor/lsm.c +19 -17
@@ -523,34 +523,34 @@ static int apparmor_setprocattr(struct t
 {
 	struct common_audit_data sa;
 	struct apparmor_audit_data aad = {0,};
-	char *command, *args = value;
+	char *command, *largs = NULL, *args = value;
 	size_t arg_size;
 	int error;
 
 	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) {
@@ -576,10 +576,12 @@ static int apparmor_setprocattr(struct t
 			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:
@@ -588,9 +590,9 @@ fail:
 	aad.profile = aa_current_profile();
 	aad.op = OP_SETPROCATTR;
 	aad.info = name;
-	aad.error = -EINVAL;
+	aad.error = error = -EINVAL;
 	aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL);
-	return -EINVAL;
+	goto out;
 }
 
 static int apparmor_task_setrlimit(struct task_struct *task,

Patches currently in stable-queue which might be from [email protected] are

queue-4.6/apparmor-fix-oops-validate-buffer-size-in-apparmor_setprocattr.patch