exec: Move most of setup_new_exec into flush_old_exec

The current idiom for the callers is:

flush_old_exec(bprm);
set_personality(...);
setup_new_exec(bprm);

In 2010 Linus split flush_old_exec into flush_old_exec and
setup_new_exec.  With the intention that setup_new_exec be what is
called after the processes new personality is set.

Move the code that doesn't depend upon the personality from
setup_new_exec into flush_old_exec.  This is to facilitate future
changes by having as much code together in one function as possible.

To see why it is safe to move this code please note that effectively
this change moves the personality setting in the binfmt and the following
three lines of code after everything except unlocking the mutexes:
	arch_pick_mmap_layout
	arch_setup_new_exec
	mm->task_size = TASK_SIZE

The function arch_pick_mmap_layout at most sets:
	mm->get_unmapped_area
	mm->mmap_base
	mm->mmap_legacy_base
	mm->mmap_compat_base
	mm->mmap_compat_legacy_base
which nothing in flush_old_exec or setup_new_exec depends on.

The function arch_setup_new_exec only sets architecture specific
state and the rest of the functions only deal in state that applies
to all architectures.

The last line just sets mm->task_size and again nothing in flush_old_exec
or setup_new_exec depend on task_size.

Ref: 221af7f87b97 ("Split 'flush_old_exec' into two functions")
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Greg Ungerer <gerg@linux-m68k.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

diff --git a/fs/exec.c b/fs/exec.c
index 8c3abafb9bb18369ff0cb0cb0ba7163a258317f3..0eff205587350c7f215ce1775901f94a64b8f67e 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1359,39 +1359,7 @@ int flush_old_exec(struct linux_binprm * bprm)
         * undergoing exec(2).
         */
        do_close_on_exec(me->files);
-       return 0;
-
-out_unlock:
-       mutex_unlock(&me->signal->exec_update_mutex);
-out:
-       return retval;
-}
-EXPORT_SYMBOL(flush_old_exec);
-
-void would_dump(struct linux_binprm *bprm, struct file *file)
-{
-       struct inode *inode = file_inode(file);
-       if (inode_permission(inode, MAY_READ) < 0) {
-               struct user_namespace *old, *user_ns;
-               bprm->interp_flags |= BINPRM_FLAGS_ENFORCE_NONDUMP;
-
-               /* Ensure mm->user_ns contains the executable */
-               user_ns = old = bprm->mm->user_ns;
-               while ((user_ns != &init_user_ns) &&
-                      !privileged_wrt_inode_uidgid(user_ns, inode))
-                       user_ns = user_ns->parent;
 
-               if (old != user_ns) {
-                       bprm->mm->user_ns = get_user_ns(user_ns);
-                       put_user_ns(old);
-               }
-       }
-}
-EXPORT_SYMBOL(would_dump);
-
-void setup_new_exec(struct linux_binprm * bprm)
-{
-       struct task_struct *me = current;
        /*
         * Once here, prepare_binrpm() will not be called any more, so
         * the final state of setuid/setgid/fscaps can be merged into the
@@ -1414,8 +1382,6 @@ void setup_new_exec(struct linux_binprm * bprm)
                        bprm->rlim_stack.rlim_cur = _STK_LIM;
        }
 
-       arch_pick_mmap_layout(me->mm, &bprm->rlim_stack);
-
        me->sas_ss_sp = me->sas_ss_size = 0;
 
        /*
@@ -1430,16 +1396,9 @@ void setup_new_exec(struct linux_binprm * bprm)
        else
                set_dumpable(current->mm, SUID_DUMP_USER);
 
-       arch_setup_new_exec();
        perf_event_exec();
        __set_task_comm(me, kbasename(bprm->filename), true);
 
-       /* Set the new mm task size. We have to do that late because it may
-        * depend on TIF_32BIT which is only updated in flush_thread() on
-        * some architectures like powerpc
-        */
-       me->mm->task_size = TASK_SIZE;
-
        /* An exec changes our domain. We are no longer part of the thread
           group */
        WRITE_ONCE(me->self_exec_id, me->self_exec_id + 1);
@@ -1467,6 +1426,50 @@ void setup_new_exec(struct linux_binprm * bprm)
         * credentials; any time after this it may be unlocked.
         */
        security_bprm_committed_creds(bprm);
+       return 0;
+
+out_unlock:
+       mutex_unlock(&me->signal->exec_update_mutex);
+out:
+       return retval;
+}
+EXPORT_SYMBOL(flush_old_exec);
+
+void would_dump(struct linux_binprm *bprm, struct file *file)
+{
+       struct inode *inode = file_inode(file);
+       if (inode_permission(inode, MAY_READ) < 0) {
+               struct user_namespace *old, *user_ns;
+               bprm->interp_flags |= BINPRM_FLAGS_ENFORCE_NONDUMP;
+
+               /* Ensure mm->user_ns contains the executable */
+               user_ns = old = bprm->mm->user_ns;
+               while ((user_ns != &init_user_ns) &&
+                      !privileged_wrt_inode_uidgid(user_ns, inode))
+                       user_ns = user_ns->parent;
+
+               if (old != user_ns) {
+                       bprm->mm->user_ns = get_user_ns(user_ns);
+                       put_user_ns(old);
+               }
+       }
+}
+EXPORT_SYMBOL(would_dump);
+
+void setup_new_exec(struct linux_binprm * bprm)
+{
+       /* Setup things that can depend upon the personality */
+       struct task_struct *me = current;
+
+       arch_pick_mmap_layout(me->mm, &bprm->rlim_stack);
+
+       arch_setup_new_exec();
+
+       /* Set the new mm task size. We have to do that late because it may
+        * depend on TIF_32BIT which is only updated in flush_thread() on
+        * some architectures like powerpc
+        */
+       me->mm->task_size = TASK_SIZE;
        mutex_unlock(&me->signal->exec_update_mutex);
        mutex_unlock(&me->signal->cred_guard_mutex);
 }