On Thu, Sep 23, 2021 at 07:10:43PM -0500, Eric W. Biederman wrote:
>
> Prevent exec continuing when a fatal signal is pending by replacing
> mmap_read_lock with mmap_read_lock_killable. This is always the right
> thing to do as userspace will never observe an exec complete when
> there is a fatal signal pending.
>
> With that change it becomes unnecessary to explicitly test for a core
> dump in progress. In coredump_wait zap_threads arranges under
> mmap_write_lock for all tasks that use a mm to also have SIGKILL
> pending, which means mmap_read_lock_killable will always return -EINTR
> when old_mm->core_state is present.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
> ---
> fs/exec.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index a098c133d8d7..b6079f1a098e 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -987,16 +987,14 @@ static int exec_mmap(struct mm_struct *mm)
>
> if (old_mm) {
> /*
> - * Make sure that if there is a core dump in progress
> - * for the old mm, we get out and die instead of going
> - * through with the exec. We must hold mmap_lock around
> - * checking core_state and changing tsk->mm.
> + * If there is a pending fatal signal perhaps a signal
> + * whose default action is to create a coredump get
> + * out and die instead of going through with the exec.
> */
> - mmap_read_lock(old_mm);
> - if (unlikely(old_mm->core_state)) {
> - mmap_read_unlock(old_mm);
> + ret = mmap_read_lock_killable(old_mm);
This appears to ultimately call into rwsem_down_read_slowpath(), which
checks signal_pending_state() (and returns the EINTR from before),
so this looks right to me.
Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
-Kees
> + if (ret) {
> up_write(&tsk->signal->exec_update_lock);
> - return -EINTR;
> + return ret;
> }
> }
>
> --
> 2.20.1
>
--
Kees Cook
