prctl_set_mm: refactor checks from validate_prctl_map
authorMichal Koutný <mkoutny@suse.com>
Sat, 1 Jun 2019 05:30:16 +0000 (22:30 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Sat, 1 Jun 2019 22:51:31 +0000 (15:51 -0700)
Despite comment of validate_prctl_map claims there are no capability
checks, it is not completely true since commit 4d28df6152aa ("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.com
Signed-off-by: Michal Koutný <mkoutny@suse.com>
Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Reviewed-by: Cyrill 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: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
kernel/sys.c

index bdbfe8d..775bf8d 100644 (file)
@@ -1882,13 +1882,14 @@ exit_err:
 }
 
 /*
+ * Check arithmetic relations of passed addresses.
+ *
  * WARNING: we don't require any capability here so be very careful
  * 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;
-       struct mm_struct *mm = current->mm;
        int error = -EINVAL, i;
 
        static const unsigned char offsets[] = {
@@ -1949,24 +1950,6 @@ static int validate_prctl_map(struct prctl_mm_map *prctl_map)
                              prctl_map->start_data))
                        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;
 out:
        return error;
@@ -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)))
                return -EFAULT;
 
-       error = validate_prctl_map(&prctl_map);
+       error = validate_prctl_map_addr(&prctl_map);
        if (error)
                return error;
 
        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));
                if (copy_from_user(user_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
        }
 
        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);
                if (error)
                        return error;
@@ -2097,7 +2095,11 @@ static int prctl_set_mm(int opt, unsigned long addr,
                        unsigned long arg4, unsigned long arg5)
 {
        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;
        int error;
 
@@ -2139,9 +2141,6 @@ static int prctl_set_mm(int opt, unsigned long addr,
        prctl_map.arg_end       = mm->arg_end;
        prctl_map.env_start     = mm->env_start;
        prctl_map.env_end       = mm->env_end;
-       prctl_map.auxv          = NULL;
-       prctl_map.auxv_size     = 0;
-       prctl_map.exe_fd        = -1;
 
        switch (opt) {
        case PR_SET_MM_START_CODE:
@@ -2181,7 +2180,7 @@ static int prctl_set_mm(int opt, unsigned long addr,
                goto out;
        }
 
-       error = validate_prctl_map(&prctl_map);
+       error = validate_prctl_map_addr(&prctl_map);
        if (error)
                goto out;