Commit 11bbd8b4 authored by Michal Koutný's avatar Michal Koutný Committed by Linus Torvalds

prctl_set_mm: refactor checks from validate_prctl_map

Despite comment of validate_prctl_map claims there are no capability
checks, it is not completely true since commit 4d28df61 ("prctl: Allow
local CAP_SYS_ADMIN changing exe_file").  Extract the check out of the
function and make the function perform purely arithmetic checks.

This patch should not change any behavior, it is mere refactoring for
following patch.

[akpm@linux-foundation.org: coding style fixes]
Link: http://lkml.kernel.org/r/20190502125203.24014-2-mkoutny@suse.comSigned-off-by: default avatarMichal Koutný <mkoutny@suse.com>
Reviewed-by: default avatarKirill Tkhai <ktkhai@virtuozzo.com>
Reviewed-by: default avatarCyrill Gorcunov <gorcunov@gmail.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Cc: Mateusz Guzik <mguzik@redhat.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Yang Shi <yang.shi@linux.alibaba.com>
Cc: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 8856ae4d
...@@ -1882,13 +1882,14 @@ static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd) ...@@ -1882,13 +1882,14 @@ static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
} }
/* /*
* Check arithmetic relations of passed addresses.
*
* WARNING: we don't require any capability here so be very careful * WARNING: we don't require any capability here so be very careful
* in what is allowed for modification from userspace. * in what is allowed for modification from userspace.
*/ */
static int validate_prctl_map(struct prctl_mm_map *prctl_map) static int validate_prctl_map_addr(struct prctl_mm_map *prctl_map)
{ {
unsigned long mmap_max_addr = TASK_SIZE; unsigned long mmap_max_addr = TASK_SIZE;
struct mm_struct *mm = current->mm;
int error = -EINVAL, i; int error = -EINVAL, i;
static const unsigned char offsets[] = { static const unsigned char offsets[] = {
...@@ -1949,24 +1950,6 @@ static int validate_prctl_map(struct prctl_mm_map *prctl_map) ...@@ -1949,24 +1950,6 @@ static int validate_prctl_map(struct prctl_mm_map *prctl_map)
prctl_map->start_data)) prctl_map->start_data))
goto out; goto out;
/*
* Someone is trying to cheat the auxv vector.
*/
if (prctl_map->auxv_size) {
if (!prctl_map->auxv || prctl_map->auxv_size > sizeof(mm->saved_auxv))
goto out;
}
/*
* Finally, make sure the caller has the rights to
* change /proc/pid/exe link: only local sys admin should
* be allowed to.
*/
if (prctl_map->exe_fd != (u32)-1) {
if (!ns_capable(current_user_ns(), CAP_SYS_ADMIN))
goto out;
}
error = 0; error = 0;
out: out:
return error; return error;
...@@ -1993,11 +1976,18 @@ static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data ...@@ -1993,11 +1976,18 @@ static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data
if (copy_from_user(&prctl_map, addr, sizeof(prctl_map))) if (copy_from_user(&prctl_map, addr, sizeof(prctl_map)))
return -EFAULT; return -EFAULT;
error = validate_prctl_map(&prctl_map); error = validate_prctl_map_addr(&prctl_map);
if (error) if (error)
return error; return error;
if (prctl_map.auxv_size) { if (prctl_map.auxv_size) {
/*
* Someone is trying to cheat the auxv vector.
*/
if (!prctl_map.auxv ||
prctl_map.auxv_size > sizeof(mm->saved_auxv))
return -EINVAL;
memset(user_auxv, 0, sizeof(user_auxv)); memset(user_auxv, 0, sizeof(user_auxv));
if (copy_from_user(user_auxv, if (copy_from_user(user_auxv,
(const void __user *)prctl_map.auxv, (const void __user *)prctl_map.auxv,
...@@ -2010,6 +2000,14 @@ static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data ...@@ -2010,6 +2000,14 @@ static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data
} }
if (prctl_map.exe_fd != (u32)-1) { if (prctl_map.exe_fd != (u32)-1) {
/*
* Make sure the caller has the rights to
* change /proc/pid/exe link: only local sys admin should
* be allowed to.
*/
if (!ns_capable(current_user_ns(), CAP_SYS_ADMIN))
return -EINVAL;
error = prctl_set_mm_exe_file(mm, prctl_map.exe_fd); error = prctl_set_mm_exe_file(mm, prctl_map.exe_fd);
if (error) if (error)
return error; return error;
...@@ -2097,7 +2095,11 @@ static int prctl_set_mm(int opt, unsigned long addr, ...@@ -2097,7 +2095,11 @@ static int prctl_set_mm(int opt, unsigned long addr,
unsigned long arg4, unsigned long arg5) unsigned long arg4, unsigned long arg5)
{ {
struct mm_struct *mm = current->mm; struct mm_struct *mm = current->mm;
struct prctl_mm_map prctl_map; struct prctl_mm_map prctl_map = {
.auxv = NULL,
.auxv_size = 0,
.exe_fd = -1,
};
struct vm_area_struct *vma; struct vm_area_struct *vma;
int error; int error;
...@@ -2139,9 +2141,6 @@ static int prctl_set_mm(int opt, unsigned long addr, ...@@ -2139,9 +2141,6 @@ static int prctl_set_mm(int opt, unsigned long addr,
prctl_map.arg_end = mm->arg_end; prctl_map.arg_end = mm->arg_end;
prctl_map.env_start = mm->env_start; prctl_map.env_start = mm->env_start;
prctl_map.env_end = mm->env_end; prctl_map.env_end = mm->env_end;
prctl_map.auxv = NULL;
prctl_map.auxv_size = 0;
prctl_map.exe_fd = -1;
switch (opt) { switch (opt) {
case PR_SET_MM_START_CODE: case PR_SET_MM_START_CODE:
...@@ -2181,7 +2180,7 @@ static int prctl_set_mm(int opt, unsigned long addr, ...@@ -2181,7 +2180,7 @@ static int prctl_set_mm(int opt, unsigned long addr,
goto out; goto out;
} }
error = validate_prctl_map(&prctl_map); error = validate_prctl_map_addr(&prctl_map);
if (error) if (error)
goto out; goto out;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment