From df9e4d2c4a53503a97fc08eeebdc04e3c11b4618 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sun, 3 May 2020 07:15:28 -0500 Subject: [PATCH] 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 Reviewed-by: Greg Ungerer Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 133 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 68 insertions(+), 65 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 8c3abafb9bb1..0eff20558735 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1359,6 +1359,73 @@ int flush_old_exec(struct linux_binprm * bprm) * undergoing exec(2). */ do_close_on_exec(me->files); + + /* + * Once here, prepare_binrpm() will not be called any more, so + * the final state of setuid/setgid/fscaps can be merged into the + * secureexec flag. + */ + bprm->secureexec |= bprm->cap_elevated; + + if (bprm->secureexec) { + /* Make sure parent cannot signal privileged process. */ + me->pdeath_signal = 0; + + /* + * For secureexec, reset the stack limit to sane default to + * avoid bad behavior from the prior rlimits. This has to + * happen before arch_pick_mmap_layout(), which examines + * RLIMIT_STACK, but after the point of no return to avoid + * needing to clean up the change on failure. + */ + if (bprm->rlim_stack.rlim_cur > _STK_LIM) + bprm->rlim_stack.rlim_cur = _STK_LIM; + } + + me->sas_ss_sp = me->sas_ss_size = 0; + + /* + * Figure out dumpability. Note that this checking only of current + * is wrong, but userspace depends on it. This should be testing + * bprm->secureexec instead. + */ + if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP || + !(uid_eq(current_euid(), current_uid()) && + gid_eq(current_egid(), current_gid()))) + set_dumpable(current->mm, suid_dumpable); + else + set_dumpable(current->mm, SUID_DUMP_USER); + + perf_event_exec(); + __set_task_comm(me, kbasename(bprm->filename), true); + + /* 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); + flush_signal_handlers(me, 0); + + /* + * install the new credentials for this executable + */ + security_bprm_committing_creds(bprm); + + commit_creds(bprm->cred); + bprm->cred = NULL; + + /* + * Disable monitoring for regular users + * when executing setuid binaries. Must + * wait until new credentials are committed + * by commit_creds() above + */ + if (get_dumpable(me->mm) != SUID_DUMP_USER) + perf_event_exit_task(me); + /* + * cred_guard_mutex must be held at least to this point to prevent + * ptrace_attach() from altering our determination of the task's + * credentials; any time after this it may be unlocked. + */ + security_bprm_committed_creds(bprm); return 0; out_unlock: @@ -1391,82 +1458,18 @@ 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; - /* - * Once here, prepare_binrpm() will not be called any more, so - * the final state of setuid/setgid/fscaps can be merged into the - * secureexec flag. - */ - bprm->secureexec |= bprm->cap_elevated; - - if (bprm->secureexec) { - /* Make sure parent cannot signal privileged process. */ - me->pdeath_signal = 0; - - /* - * For secureexec, reset the stack limit to sane default to - * avoid bad behavior from the prior rlimits. This has to - * happen before arch_pick_mmap_layout(), which examines - * RLIMIT_STACK, but after the point of no return to avoid - * needing to clean up the change on failure. - */ - if (bprm->rlim_stack.rlim_cur > _STK_LIM) - 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; - - /* - * Figure out dumpability. Note that this checking only of current - * is wrong, but userspace depends on it. This should be testing - * bprm->secureexec instead. - */ - if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP || - !(uid_eq(current_euid(), current_uid()) && - gid_eq(current_egid(), current_gid()))) - set_dumpable(current->mm, suid_dumpable); - 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); - flush_signal_handlers(me, 0); - - /* - * install the new credentials for this executable - */ - security_bprm_committing_creds(bprm); - - commit_creds(bprm->cred); - bprm->cred = NULL; - - /* - * Disable monitoring for regular users - * when executing setuid binaries. Must - * wait until new credentials are committed - * by commit_creds() above - */ - if (get_dumpable(me->mm) != SUID_DUMP_USER) - perf_event_exit_task(me); - /* - * cred_guard_mutex must be held at least to this point to prevent - * ptrace_attach() from altering our determination of the task's - * credentials; any time after this it may be unlocked. - */ - security_bprm_committed_creds(bprm); mutex_unlock(&me->signal->exec_update_mutex); mutex_unlock(&me->signal->cred_guard_mutex); }