public-mirrors/linux
1
0
Fork 0
mirror of git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2025-09-18 22:14:16 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

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

Browse source 
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@oracle.com>
modified for the case that apparmor provides null termination.

Fixes: bb646cdb12
Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: John Johansen <john.johansen@canonical.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Eric Paris <eparis@parisplace.org>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: stable@kernel.org
Signed-off-by: John Johansen <john.johansen@canonical.com>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
This commit is contained in:
Vegard Nossum 2016-07-07 13:41:11 -07:00 • committed by James Morris
parent ac904ae6e6
commit 30a46a4647
1 changed files with 19 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
security/apparmor/lsm.c
View file

@ -500,34 +500,34 @@ static int apparmor_setprocattr(struct task_struct *task, char *name,
{ {
struct common_audit_data sa; struct common_audit_data sa;
struct apparmor_audit_data aad = {0,}; struct apparmor_audit_data aad = {0,};
char *command, *args = value; char *command, *largs = NULL, *args = value;
size_t arg_size; size_t arg_size;
int error; int error;
if (size == 0) if (size == 0)
return -EINVAL; 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 */ /* task can only write its own attributes */
if (current != task) if (current != task)
return -EACCES; 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); args = strim(args);
command = strsep(&args, " "); command = strsep(&args, " ");
if (!args) if (!args)
return -EINVAL; goto out;
args = skip_spaces(args); args = skip_spaces(args);
if (!*args) if (!*args)
return -EINVAL; goto out;
arg_size = size - (args - (char *) value); arg_size = size - (args - (char *) value);
if (strcmp(name, "current") == 0) { if (strcmp(name, "current") == 0) {
@ -553,10 +553,12 @@ static int apparmor_setprocattr(struct task_struct *task, char *name,
goto fail; goto fail;
} else } else
/* only support the "current" and "exec" process attributes */ /* only support the "current" and "exec" process attributes */
return -EINVAL; goto fail;
if (!error) if (!error)
error = size; error = size;
out:
kfree(largs);
return error; return error;
fail: fail:
@ -565,9 +567,9 @@ fail:
aad.profile = aa_current_profile(); aad.profile = aa_current_profile();
aad.op = OP_SETPROCATTR; aad.op = OP_SETPROCATTR;
aad.info = name; aad.info = name;
aad.error = -EINVAL; aad.error = error = -EINVAL;
aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL); aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL);
return -EINVAL; goto out;
} }
static int apparmor_task_setrlimit(struct task_struct *task, static int apparmor_task_setrlimit(struct task_struct *task,