Skip to content

Commit af7bb0d

Browse files
oleg-nesterovbrauner
authored andcommitted
exec: fix the racy usage of fs_struct->in_exec
check_unsafe_exec() sets fs->in_exec under cred_guard_mutex, then execve() paths clear fs->in_exec lockless. This is fine if exec succeeds, but if it fails we have the following race: T1 sets fs->in_exec = 1, fails, drops cred_guard_mutex T2 sets fs->in_exec = 1 T1 clears fs->in_exec T2 continues with fs->in_exec == 0 Change fs/exec.c to clear fs->in_exec with cred_guard_mutex held. Reported-by: syzbot+1c486d0b62032c82a968@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/67dc67f0.050a0220.25ae54.001f.GAE@google.com/ Cc: stable@vger.kernel.org Signed-off-by: Oleg Nesterov <oleg@redhat.com> Link: https://lore.kernel.org/r/20250324160003.GA8878@redhat.com Signed-off-by: Christian Brauner <brauner@kernel.org>
1 parent 8661bb9 commit af7bb0d

File tree

1 file changed

+9
-6
lines changed

1 file changed

+9
-6
lines changed

fs/exec.c

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,13 +1227,12 @@ int begin_new_exec(struct linux_binprm * bprm)
12271227
*/
12281228
bprm->point_of_no_return = true;
12291229

1230-
/*
1231-
* Make this the only thread in the thread group.
1232-
*/
1230+
/* Make this the only thread in the thread group */
12331231
retval = de_thread(me);
12341232
if (retval)
12351233
goto out;
1236-
1234+
/* see the comment in check_unsafe_exec() */
1235+
current->fs->in_exec = 0;
12371236
/*
12381237
* Cancel any io_uring activity across execve
12391238
*/
@@ -1495,6 +1494,8 @@ static void free_bprm(struct linux_binprm *bprm)
14951494
}
14961495
free_arg_pages(bprm);
14971496
if (bprm->cred) {
1497+
/* in case exec fails before de_thread() succeeds */
1498+
current->fs->in_exec = 0;
14981499
mutex_unlock(&current->signal->cred_guard_mutex);
14991500
abort_creds(bprm->cred);
15001501
}
@@ -1616,6 +1617,10 @@ static void check_unsafe_exec(struct linux_binprm *bprm)
16161617
* suid exec because the differently privileged task
16171618
* will be able to manipulate the current directory, etc.
16181619
* It would be nice to force an unshare instead...
1620+
*
1621+
* Otherwise we set fs->in_exec = 1 to deny clone(CLONE_FS)
1622+
* from another sub-thread until de_thread() succeeds, this
1623+
* state is protected by cred_guard_mutex we hold.
16191624
*/
16201625
n_fs = 1;
16211626
spin_lock(&p->fs->lock);
@@ -1860,7 +1865,6 @@ static int bprm_execve(struct linux_binprm *bprm)
18601865

18611866
sched_mm_cid_after_execve(current);
18621867
/* execve succeeded */
1863-
current->fs->in_exec = 0;
18641868
current->in_execve = 0;
18651869
rseq_execve(current);
18661870
user_events_execve(current);
@@ -1879,7 +1883,6 @@ static int bprm_execve(struct linux_binprm *bprm)
18791883
force_fatal_sig(SIGSEGV);
18801884

18811885
sched_mm_cid_after_execve(current);
1882-
current->fs->in_exec = 0;
18831886
current->in_execve = 0;
18841887

18851888
return retval;

0 commit comments

Comments
 (0)