[PATCH] exec: do_close_on_exec() before taking exec_update_lock

[PATCH] exec: do_close_on_exec() before taking exec_update_lock

[Jann Horn]

do_close_on_exec() currently happens while holding the exec_update_lock,
which is used in a lot of places that access process state to
synchronize access checks.
I recently added another such use of exec_update_lock, causing a
regression.

do_close_on_exec() can block waiting for a reply from a filesystem.
That means a hung filesystem can block codepaths that use
exec_update_lock; and it also means that a FUSE filesystem which
attempts to inspect the calling process can deadlock.

To avoid such problems, move do_close_on_exec() before the
exec_update_lock is taken, but after the FD table has been copied if
necessary.

I have looked through all the calls between the old and new position of
the do_close_on_exec() call; there seems to be no file descriptor table
access in between.

Reported-by: Benjamin Peterson <benjamin@locrian.net>
Closes: https://lore.kernel.org/r/f5e8166a-88be-46c5-8939-1e5227ffe4c2@app.fastmail.com
Fixes: 6650527444da ("proc: protect ptrace_may_access() with exec_update_lock (part 1)")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
 fs/exec.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 745f6eb5279e..b51e5d7e4536 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1164,6 +1164,20 @@ int begin_new_exec(struct linux_binprm * bprm)
 	if (retval)
 		goto out;
 
+	/*
+	 * We have to apply CLOEXEC before we change whether the process is
+	 * dumpable (in setup_new_exec) to avoid a race with a process in userspace
+	 * trying to access the should-be-closed file descriptors of a process
+	 * undergoing exec(2).
+	 *
+	 * This can block on filesystem ->flush() handlers, including waiting
+	 * for FUSE daemons, so do it before exec_mmap takes the
+	 * exec_update_lock.
+	 * This must happen after the point of no return, and after unsharing
+	 * the FD table.
+	 */
+	do_close_on_exec(me->files);
+
 	/*
 	 * Must be called _before_ exec_mmap() as bprm->mm is
 	 * not visible until then. Doing it here also ensures
@@ -1214,14 +1228,6 @@ int begin_new_exec(struct linux_binprm * bprm)
 
 	clear_syscall_work_syscall_user_dispatch(me);
 
-	/*
-	 * We have to apply CLOEXEC before we change whether the process is
-	 * dumpable (in setup_new_exec) to avoid a race with a process in userspace
-	 * trying to access the should-be-closed file descriptors of a process
-	 * undergoing exec(2).
-	 */
-	do_close_on_exec(me->files);
-
 	if (bprm->secureexec) {
 		/* Make sure parent cannot signal privileged process. */
 		me->pdeath_signal = 0;

---
base-commit: 73ae59e975966d24e32926247ddb45a537ebe184
change-id: 20260907-cloexec-before-exec-update-lock-972ad108510c

Best regards,
--  
Jann Horn <jannh@google.com>

Re: [PATCH] exec: do_close_on_exec() before taking exec_update_lock

[Jan Kara]

On Mon 07-09-26 23:26:32, Jann Horn wrote:
> do_close_on_exec() currently happens while holding the exec_update_lock,
> which is used in a lot of places that access process state to
> synchronize access checks.
> I recently added another such use of exec_update_lock, causing a
> regression.
> 
> do_close_on_exec() can block waiting for a reply from a filesystem.
> That means a hung filesystem can block codepaths that use
> exec_update_lock; and it also means that a FUSE filesystem which
> attempts to inspect the calling process can deadlock.
> 
> To avoid such problems, move do_close_on_exec() before the
> exec_update_lock is taken, but after the FD table has been copied if
> necessary.
> 
> I have looked through all the calls between the old and new position of
> the do_close_on_exec() call; there seems to be no file descriptor table
> access in between.
> 
> Reported-by: Benjamin Peterson <benjamin@locrian.net>
> Closes: https://lore.kernel.org/r/f5e8166a-88be-46c5-8939-1e5227ffe4c2@app.fastmail.com
> Fixes: 6650527444da ("proc: protect ptrace_may_access() with exec_update_lock (part 1)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/exec.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index 745f6eb5279e..b51e5d7e4536 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1164,6 +1164,20 @@ int begin_new_exec(struct linux_binprm * bprm)
>  	if (retval)
>  		goto out;
>  
> +	/*
> +	 * We have to apply CLOEXEC before we change whether the process is
> +	 * dumpable (in setup_new_exec) to avoid a race with a process in userspace
> +	 * trying to access the should-be-closed file descriptors of a process
> +	 * undergoing exec(2).
> +	 *
> +	 * This can block on filesystem ->flush() handlers, including waiting
> +	 * for FUSE daemons, so do it before exec_mmap takes the
> +	 * exec_update_lock.
> +	 * This must happen after the point of no return, and after unsharing
> +	 * the FD table.
> +	 */
> +	do_close_on_exec(me->files);
> +
>  	/*
>  	 * Must be called _before_ exec_mmap() as bprm->mm is
>  	 * not visible until then. Doing it here also ensures
> @@ -1214,14 +1228,6 @@ int begin_new_exec(struct linux_binprm * bprm)
>  
>  	clear_syscall_work_syscall_user_dispatch(me);
>  
> -	/*
> -	 * We have to apply CLOEXEC before we change whether the process is
> -	 * dumpable (in setup_new_exec) to avoid a race with a process in userspace
> -	 * trying to access the should-be-closed file descriptors of a process
> -	 * undergoing exec(2).
> -	 */
> -	do_close_on_exec(me->files);
> -
>  	if (bprm->secureexec) {
>  		/* Make sure parent cannot signal privileged process. */
>  		me->pdeath_signal = 0;
> 
> ---
> base-commit: 73ae59e975966d24e32926247ddb45a537ebe184
> change-id: 20260907-cloexec-before-exec-update-lock-972ad108510c
> 
> Best regards,
> --  
> Jann Horn <jannh@google.com>
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] exec: do_close_on_exec() before taking exec_update_lock

[Benjamin Peterson]

On Mon, Sep 7, 2026, at 14:26, Jann Horn wrote:
> do_close_on_exec() currently happens while holding the exec_update_lock,
> which is used in a lot of places that access process state to
> synchronize access checks.
> I recently added another such use of exec_update_lock, causing a
> regression.
>
> do_close_on_exec() can block waiting for a reply from a filesystem.
> That means a hung filesystem can block codepaths that use
> exec_update_lock; and it also means that a FUSE filesystem which
> attempts to inspect the calling process can deadlock.
>
> To avoid such problems, move do_close_on_exec() before the
> exec_update_lock is taken, but after the FD table has been copied if
> necessary.
>
> I have looked through all the calls between the old and new position of
> the do_close_on_exec() call; there seems to be no file descriptor table
> access in between.
>
> Reported-by: Benjamin Peterson <benjamin@locrian.net>
> Closes: 
> https://lore.kernel.org/r/f5e8166a-88be-46c5-8939-1e5227ffe4c2@app.fastmail.com
> Fixes: 6650527444da ("proc: protect ptrace_may_access() with 
> exec_update_lock (part 1)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>

Thanks. I confirmed this fixes my regression, so

Tested-by: Benjamin Peterson <benjamin@locrian.net>

Re: [PATCH] exec: do_close_on_exec() before taking exec_update_lock

[Christian Brauner]

On Mon, 07 Sep 2026 23:26:32 +0200, Jann Horn wrote:
> do_close_on_exec() currently happens while holding the exec_update_lock,
> which is used in a lot of places that access process state to
> synchronize access checks.
> I recently added another such use of exec_update_lock, causing a
> regression.
> 
> do_close_on_exec() can block waiting for a reply from a filesystem.
> That means a hung filesystem can block codepaths that use
> exec_update_lock; and it also means that a FUSE filesystem which
> attempts to inspect the calling process can deadlock.
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] exec: do_close_on_exec() before taking exec_update_lock
      https://git.kernel.org/vfs/vfs/c/e780259b54e6

Re: [PATCH] exec: do_close_on_exec() before taking exec_update_lock

[Eric W. Biederman]

Jann Horn <jannh@google.com> writes:

> do_close_on_exec() currently happens while holding the exec_update_lock,
> which is used in a lot of places that access process state to
> synchronize access checks.
> I recently added another such use of exec_update_lock, causing a
> regression.

A small nit.

It has always been a requirement that in exec_update_lock not be held
for writing over any userspace accesses.  Which is why it is taken in
right after exec_mmap is done updating userspace.

In the original version I missed that do_close_on_exec calls flush
which can block waiting on userspace (Is that just a fuse thing?).

So unless I am mistaken I don't think your change technically
created a new bug, so much as aggravated an existing bug.  It was
definitely a regression in user experience.

I am mentioning this just to make it clear what is going on.

> do_close_on_exec() can block waiting for a reply from a filesystem.
> That means a hung filesystem can block codepaths that use
> exec_update_lock; and it also means that a FUSE filesystem which
> attempts to inspect the calling process can deadlock.
>
> To avoid such problems, move do_close_on_exec() before the
> exec_update_lock is taken, but after the FD table has been copied if
> necessary.
>
> I have looked through all the calls between the old and new position of
> the do_close_on_exec() call; there seems to be no file descriptor table
> access in between.

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

> Reported-by: Benjamin Peterson <benjamin@locrian.net>
> Closes: https://lore.kernel.org/r/f5e8166a-88be-46c5-8939-1e5227ffe4c2@app.fastmail.com
> Fixes: 6650527444da ("proc: protect ptrace_may_access() with exec_update_lock (part 1)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
>  fs/exec.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 745f6eb5279e..b51e5d7e4536 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1164,6 +1164,20 @@ int begin_new_exec(struct linux_binprm * bprm)
>  	if (retval)
>  		goto out;
>  
> +	/*
> +	 * We have to apply CLOEXEC before we change whether the process is
> +	 * dumpable (in setup_new_exec) to avoid a race with a process in userspace
> +	 * trying to access the should-be-closed file descriptors of a process
> +	 * undergoing exec(2).
> +	 *
> +	 * This can block on filesystem ->flush() handlers, including waiting
> +	 * for FUSE daemons, so do it before exec_mmap takes the
> +	 * exec_update_lock.
> +	 * This must happen after the point of no return, and after unsharing
> +	 * the FD table.
> +	 */
> +	do_close_on_exec(me->files);
> +
>  	/*
>  	 * Must be called _before_ exec_mmap() as bprm->mm is
>  	 * not visible until then. Doing it here also ensures
> @@ -1214,14 +1228,6 @@ int begin_new_exec(struct linux_binprm * bprm)
>  
>  	clear_syscall_work_syscall_user_dispatch(me);
>  
> -	/*
> -	 * We have to apply CLOEXEC before we change whether the process is
> -	 * dumpable (in setup_new_exec) to avoid a race with a process in userspace
> -	 * trying to access the should-be-closed file descriptors of a process
> -	 * undergoing exec(2).
> -	 */
> -	do_close_on_exec(me->files);
> -
>  	if (bprm->secureexec) {
>  		/* Make sure parent cannot signal privileged process. */
>  		me->pdeath_signal = 0;
>
> ---
> base-commit: 73ae59e975966d24e32926247ddb45a537ebe184
> change-id: 20260907-cloexec-before-exec-update-lock-972ad108510c
>
> Best regards,
> --  
>
> Jann Horn <jannh@google.com>

Re: [PATCH] exec: do_close_on_exec() before taking exec_update_lock

[Jann Horn]

On Fri, Sep 11, 2026 at 1:30 PM Eric W. Biederman <ebiederm@xmission.com> wrote:
> It has always been a requirement that in exec_update_lock not be held
> for writing over any userspace accesses.  Which is why it is taken in
> right after exec_mmap is done updating userspace.
>
> In the original version I missed that do_close_on_exec calls flush
> which can block waiting on userspace (Is that just a fuse thing?).

It's particularly egregious in FUSE, but I think network filesystems
can also block on network I/O in there.

> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

Thanks for the review!