Skip to content

Commit

Permalink
sched_setattr: switch to copy_struct_from_user()
Browse files Browse the repository at this point in the history
Switch sched_setattr() syscall from it's own copying struct sched_attr
from userspace to the new dedicated copy_struct_from_user() helper.

The change is very straightforward, and helps unify the syscall
interface for struct-from-userspace syscalls. Ideally we could also
unify sched_getattr(2)-style syscalls as well, but unfortunately the
correct semantics for such syscalls are much less clear (see [1] for
more detail). In future we could come up with a more sane idea for how
the syscall interface should look.

[1]: commit 1251201 ("sched/core: Fix uclamp ABI bug, clean up and
     robustify sched_read_attr() ABI logic and code")

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
[christian.brauner@ubuntu.com: improve commit message]
Link: https://lore.kernel.org/r/20191001011055.19283-4-cyphar@cyphar.com
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
  • Loading branch information
Aleksa Sarai authored and Christian Brauner committed Oct 1, 2019
1 parent f14c234 commit dff3a85
Showing 1 changed file with 7 additions and 36 deletions.
43 changes: 7 additions & 36 deletions kernel/sched/core.c
Original file line number Diff line number Diff line change
@@ -5106,55 +5106,26 @@ static int sched_copy_attr(struct sched_attr __user *uattr, struct sched_attr *a
u32 size;
int ret;

if (!access_ok(uattr, SCHED_ATTR_SIZE_VER0))
return -EFAULT;

/* Zero the full structure, so that a short copy will be nice: */
memset(attr, 0, sizeof(*attr));

ret = get_user(size, &uattr->size);
if (ret)
return ret;

/* Bail out on silly large: */
if (size > PAGE_SIZE)
goto err_size;

/* ABI compatibility quirk: */
if (!size)
size = SCHED_ATTR_SIZE_VER0;

if (size < SCHED_ATTR_SIZE_VER0)
if (size < SCHED_ATTR_SIZE_VER0 || size > PAGE_SIZE)
goto err_size;

/*
* If we're handed a bigger struct than we know of,
* ensure all the unknown bits are 0 - i.e. new
* user-space does not rely on any kernel feature
* extensions we dont know about yet.
*/
if (size > sizeof(*attr)) {
unsigned char __user *addr;
unsigned char __user *end;
unsigned char val;

addr = (void __user *)uattr + sizeof(*attr);
end = (void __user *)uattr + size;

for (; addr < end; addr++) {
ret = get_user(val, addr);
if (ret)
return ret;
if (val)
goto err_size;
}
size = sizeof(*attr);
ret = copy_struct_from_user(attr, sizeof(*attr), uattr, size);
if (ret) {
if (ret == -E2BIG)
goto err_size;
return ret;
}

ret = copy_from_user(attr, uattr, size);
if (ret)
return -EFAULT;

if ((attr->sched_flags & SCHED_FLAG_UTIL_CLAMP) &&
size < SCHED_ATTR_SIZE_VER1)
return -EINVAL;
@@ -5354,7 +5325,7 @@ sched_attr_copy_to_user(struct sched_attr __user *uattr,
* sys_sched_getattr - similar to sched_getparam, but with sched_attr
* @pid: the pid in question.
* @uattr: structure containing the extended parameters.
* @usize: sizeof(attr) that user-space knows about, for forwards and backwards compatibility.
* @usize: sizeof(attr) for fwd/bwd comp.
* @flags: for future extension.
*/
SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,

0 comments on commit dff3a85

Please sign in to comment.