From 31c109457729f6ebb7c590f4303f185f8611f147 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Thu, 27 Jan 2022 11:18:25 +0100
Subject: [PATCH] Kernel: Don't mess with thread state in Process::do_exec()

We were marking the execing thread as Runnable near the end of
Process::do_exec().

This was necessary for exec in processes that had never been scheduled
yet, which is a specific edge case that only applies to the very first
userspace process (normally SystemServer). At this point, such threads
are in the Invalid state.

In the common case (normal userspace-initiated exec), making the current
thread Runnable meant that we switched away from its current state:
Running. As the thread is indeed running, that's a bogus change!
This created a short time window in which the thread state was bogus,
and any attempt to block the thread would panic the kernel (due to a
bogus thread state in Thread::block() leading to VERIFY_NOT_REACHED().)

Fix this by not touching the thread state in Process::do_exec()
and instead make the first userspace thread Runnable directly after
calling Process::exec() on it in try_create_userspace_process().

It's unfortunate that exec() can be called both on the current thread,
and on a new thread that has never been scheduled. It would be good to
not have the latter edge case, but fixing that will require larger
architectural changes outside the scope of this fix.
---
 Kernel/Process.cpp         | 5 +++++
 Kernel/Syscalls/execve.cpp | 4 ----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index 7191cd0611..7f59b5bd83 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -164,6 +164,11 @@ ErrorOr<NonnullRefPtr<Process>> Process::try_create_user_process(RefPtr<Thread>&
     // NOTE: All user processes have a leaked ref on them. It's balanced by Thread::WaitBlockerSet::finalize().
     process->ref();
 
+    {
+        SpinlockLocker lock(g_scheduler_lock);
+        new_main_thread->set_state(Thread::State::Runnable);
+    }
+
     return process;
 }
 
diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp
index 6696e9b149..14c04ee798 100644
--- a/Kernel/Syscalls/execve.cpp
+++ b/Kernel/Syscalls/execve.cpp
@@ -627,10 +627,6 @@ ErrorOr<void> Process::do_exec(NonnullRefPtr<OpenFileDescription> main_program_d
         PerformanceManager::add_process_exec_event(*this);
     }
 
-    {
-        SpinlockLocker lock(g_scheduler_lock);
-        new_main_thread->set_state(Thread::State::Runnable);
-    }
     u32 lock_count_to_restore;
     [[maybe_unused]] auto rc = big_lock().force_unlock_if_locked(lock_count_to_restore);
     VERIFY_INTERRUPTS_DISABLED();