Kernel: Make sure we can allocate kernel stack before creating thread

Wrap thread creation in a Thread::try_create() helper that first
allocates a kernel stack region. If that allocation fails, we propagate
an ENOMEM error to the caller.

This avoids the situation where a thread is half-constructed, without a
valid kernel stack, and avoids having to do messy cleanup in that case.

Kernel/Process.cpp

@@ -346,14 +346,11 @@ Process::Process(RefPtr<Thread>& first_thread, const String& name, uid_t uid, gi
         first_thread = Thread::current()->clone(*this);
     } else {
         // NOTE: This non-forked code path is only taken when the kernel creates a process "manually" (at boot.)
-        first_thread = adopt(*new Thread(*this));
+        auto thread_or_error = Thread::try_create(*this);
+        ASSERT(!thread_or_error.is_error());
+        first_thread = thread_or_error.release_value();
         first_thread->detach();
     }
-
-    if (first_thread && !first_thread->was_created()) {
-        // We couldn't entirely create or clone this thread, abort
-        first_thread = nullptr;
-    }
 }
 
 Process::~Process()
@@ -810,12 +807,11 @@ RefPtr<Thread> Process::create_kernel_thread(void (*entry)(void*), void* entry_d
 
     // FIXME: Do something with guard pages?
 
-    auto thread = adopt(*new Thread(*this));
-    if (!thread->was_created()) {
-        // Could not fully create this thread
+    auto thread_or_error = Thread::try_create(*this);
+    if (thread_or_error.is_error())
         return {};
-    }
 
+    auto& thread = thread_or_error.value();
     thread->set_name(name);
     thread->set_affinity(affinity);
     thread->set_priority(priority);

Kernel/Syscalls/thread.cpp

@@ -60,11 +60,11 @@ int Process::sys$create_thread(void* (*entry)(void*), Userspace<const Syscall::S
 
     // FIXME: Do something with guard pages?
 
-    auto thread = adopt(*new Thread(*this));
-    if (!thread->was_created()) {
-        // Could not fully create a thread
-        return -ENOMEM;
-    }
+    auto thread_or_error = Thread::try_create(*this);
+    if (thread_or_error.is_error())
+        return thread_or_error.error();
+
+    auto& thread = thread_or_error.value();
 
     // We know this thread is not the main_thread,
     // So give it a unique name until the user calls $set_thread_name on it

Kernel/Thread.cpp

@@ -53,20 +53,29 @@ void Thread::initialize()
     g_tid_map = new HashMap<ThreadID, Thread*>();
 }
 
-Thread::Thread(NonnullRefPtr<Process> process)
+KResultOr<NonnullRefPtr<Thread>> Thread::try_create(NonnullRefPtr<Process> process)
+{
+    auto kernel_stack_region = MM.allocate_kernel_region(default_kernel_stack_size, {}, Region::Access::Read | Region::Access::Write, false, AllocationStrategy::AllocateNow);
+    if (!kernel_stack_region)
+        return ENOMEM;
+    return adopt(*new Thread(move(process), kernel_stack_region.release_nonnull()));
+}
+
+Thread::Thread(NonnullRefPtr<Process> process, NonnullOwnPtr<Region> kernel_stack_region)
     : m_process(move(process))
+    , m_kernel_stack_region(move(kernel_stack_region))
     , m_name(m_process->name())
 {
     bool is_first_thread = m_process->add_thread(*this);
-    ArmedScopeGuard guard([&]() {
-        drop_thread_count(is_first_thread);
-    });
     if (is_first_thread) {
         // First thread gets TID == PID
         m_tid = m_process->pid().value();
     } else {
         m_tid = Process::allocate_pid().value();
     }
+
+    m_kernel_stack_region->set_name(String::formatted("Kernel stack (thread {})", m_tid.value()));
+
     {
         ScopedSpinLock lock(g_tid_map_lock);
         auto result = g_tid_map->set(m_tid, this);
@@ -126,7 +135,6 @@ Thread::Thread(NonnullRefPtr<Process> process)
     // The finalizer is responsible for dropping this reference once this
     // thread is ready to be cleaned up.
     ref();
-    guard.disarm();
 }
 
 Thread::~Thread()
@@ -881,11 +889,10 @@ RegisterState& Thread::get_register_dump_from_stack()
 
 RefPtr<Thread> Thread::clone(Process& process)
 {
-    auto clone = adopt(*new Thread(process));
-    if (!clone->was_created()) {
-        // We failed to clone this thread
+    auto thread_or_error = Thread::try_create(process);
+    if (thread_or_error.is_error())
         return {};
-    }
+    auto& clone = thread_or_error.value();
     memcpy(clone->m_signal_action_data, m_signal_action_data, sizeof(m_signal_action_data));
     clone->m_signal_mask = m_signal_mask;
     memcpy(clone->m_fpu_state, m_fpu_state, sizeof(FPUState));

Kernel/Thread.h

@@ -99,7 +99,7 @@ public:
 
     static void initialize();
 
-    explicit Thread(NonnullRefPtr<Process>);
+    static KResultOr<NonnullRefPtr<Thread>> try_create(NonnullRefPtr<Process>);
     ~Thread();
 
     static RefPtr<Thread> from_tid(ThreadID);
@@ -1157,15 +1157,15 @@ public:
     }
 #endif
 
-    bool was_created() const
+    bool is_handling_page_fault() const
     {
-        return m_kernel_stack_region;
+        return m_handling_page_fault;
     }
-
-    bool is_handling_page_fault() const { return m_handling_page_fault; }
     void set_handling_page_fault(bool b) { m_handling_page_fault = b; }
 
 private:
+    Thread(NonnullRefPtr<Process>, NonnullOwnPtr<Region> kernel_stack_region);
+
     IntrusiveListNode m_process_thread_list_node;
     int m_runnable_priority { -1 };