# \[PATCH 3/6\] exec: Check for a pending fatal signal instead of core\_state

- Subsystems: EXEC \& BINFMT API, ELF, FILESYSTEMS (VFS and infrastructure), THE REST
- Maintainers: Kees Cook, Alexander Viro, Christian Brauner, Linus Torvalds
- List: linux-api
- 2 messages, 2 authors
- Page: https://ratatoskr.run/linux-api/2021/09/1520121/t

## \[PATCH 3/6\] exec: Check for a pending fatal signal instead of core\_state

- From: Eric W. Biederman \<hidden\>
- Date: 2021-09-24 00:10:57 
- Message: https://ratatoskr.run/linux-api/2021/09/1520121.md

```text

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);
+		if (ret) {
 			up_write(&tsk->signal->exec_update_lock);
-			return -EINTR;
+			return ret;
 		}
 	}
 
-- 
2.20.1

```

## Re: \[PATCH 3/6\] exec: Check for a pending fatal signal instead of core\_state

- From: Kees Cook \<hidden\>
- Date: 2021-09-24 15:38:47 
- Message: https://ratatoskr.run/linux-api/2021/09/1521106.md

```text
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" <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
```