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.
This commit is contained in:
Andreas Kling 2022-01-27 11:18:25 +01:00
parent b7d316d291
commit 31c1094577
Notes: sideshowbarker 2024-07-17 21:16:31 +09:00
2 changed files with 5 additions and 4 deletions

View File

@ -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;
}

View File

@ -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();