Re: [PATCH 3/6] exec: Check for a pending fatal signal instead of core_state
flat view
From: Kees Cook <hidden>
Date: 2021-09-24 15:38:47
Also in:
lkml
On Thu, Sep 23, 2021 at 07:10:43PM -0500, Eric W. Biederman wrote:
quoted hunk ↗ jump to hunk
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" <redacted> --- 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 <redacted> -Kees
+ if (ret) {
up_write(&tsk->signal->exec_update_lock);
- return -EINTR;
+ return ret;
}
}
--
2.20.1-- Kees Cook