Re: [PATCH 2/3] exec: Make unlocking exec_update_mutex explict

Kees Cook
2020-04-07T16:02:37Z

On Mon, Apr 06, 2020 at 08:31:52PM -0500, Eric W. Biederman wrote:
>
> With install_exec_creds updated to follow immediately after
> setup_new_exec, the failure of unshare_sighand is the only
> code path where exec_update_mutex is held but not explicitly
> unlocked.
>
> Update that code path to explicitly unlock exec_update_mutex.
>
> Remove the unlocking of exec_update_mutex from free_bprm.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
> fs/exec.c | 6 +++---
> include/linux/binfmts.h | 3 +--
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index d55710a36056..28c87020da9b 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1318,7 +1318,7 @@ int flush_old_exec(struct linux_binprm * bprm)
> */
> retval = unshare_sighand(me);
> if (retval)
> - goto out;
> + goto out_unlock;
>
> set_fs(USER_DS);
> me->flags &= ~(PF_RANDOMIZE | PF_FORKNOEXEC | PF_KTHREAD |
> @@ -1335,6 +1335,8 @@ int flush_old_exec(struct linux_binprm * bprm)
> do_close_on_exec(me->files);
> return 0;
>
> +out_unlock:
> + mutex_unlock(&me->signal->exec_update_mutex);
> out:
> return retval;
> }
> @@ -1451,8 +1453,6 @@ static void free_bprm(struct linux_binprm *bprm)
> {
> free_arg_pages(bprm);
> if (bprm->cred) {
> - if (bprm->called_exec_mmap)
> - mutex_unlock(&current->signal->exec_update_mutex);
> mutex_unlock(&current->signal->cred_guard_mutex);
> abort_creds(bprm->cred);
> }
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index a345d9fed3d8..6f564b9ad882 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -47,8 +47,7 @@ struct linux_binprm {
> secureexec:1,
> /*
> * Set by flush_old_exec, when exec_mmap has been called.
> - * This is past the point of no return, when the
> - * exec_update_mutex has been taken.
> + * This is past the point of no return.
> */
> called_exec_mmap:1;
> #ifdef __alpha__
> --
> 2.25.0
>

--
Kees Cook