Patch "apparmor: fix oops, validate buffer size in apparmor_setprocattr()" has been added to the 4.6-stable tree
Subsystems (3)
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
30a46a4647fd1df9cf52e43bf467f0d9265096caMon 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.
--- 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