Oleg noticed that I'm doing a strange games with mm::mmap_sem
member -- I take it and keep busy whie validating data which
comes from user space but then I release it and copy auxv
data itself.

This means we've a small window where previously found VMAs might
be no longer valid thus move @auxv validation and copying at the
beginning of prctl_set_mm_map before we take the lock.

Signed-off-by: Cyrill Gorcunov <[email protected]>
CC: Oleg Nesterov <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Andrew Vagin <[email protected]>
Cc: Eric W. Biederman <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Serge Hallyn <[email protected]>
Cc: Pavel Emelyanov <[email protected]>
Cc: Vasiliy Kulikov <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Cc: Michael Kerrisk <[email protected]>
Cc: Julien Tinnes <[email protected]>
---
 kernel/sys.c |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

Index: linux-2.6.git/kernel/sys.c
===================================================================
--- linux-2.6.git.orig/kernel/sys.c
+++ linux-2.6.git/kernel/sys.c
@@ -1838,22 +1838,28 @@ static int prctl_set_mm_map(int opt, con
        if (copy_from_user(&prctl_map, addr, sizeof(prctl_map)))
                return -EFAULT;
 
-       down_read(&mm->mmap_sem);
-
-       if (validate_prctl_map_locked(&prctl_map))
-               goto out;
-
+       /*
+        * Validate and copy auxv early to be able to keep
+        * @mmap_sem taken until this function ends.
+        */
        if (prctl_map.auxv_size) {
-               up_read(&mm->mmap_sem);
+               if (!prctl_map.auxv ||
+                   prctl_map.auxv_size > sizeof(mm->saved_auxv))
+                       return -EINVAL;
+
                memset(user_auxv, 0, sizeof(user_auxv));
                error = copy_from_user(user_auxv,
                                       (const void __user *)prctl_map.auxv,
                                       prctl_map.auxv_size);
-               down_read(&mm->mmap_sem);
                if (error)
                        goto out;
        }
 
+       down_read(&mm->mmap_sem);
+
+       if (validate_prctl_map_locked(&prctl_map))
+               goto out;
+
        if (prctl_map.exe_fd != (u32)-1) {
                error = prctl_set_mm_exe_file_locked(mm, prctl_map.exe_fd);
                if (error)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to