浏览代码

Kernel: Fix Process use-after-free in Thread finalization

We leak a ref() onto every user process when constructing them,
either via Process::create_user_process(), or via Process::sys$fork().

This ref() is balanced by a corresponding unref() in
Thread::WaitBlockCondition::finalize().

Since kernel processes don't have a leaked ref() on them, this led to
an extra Process::unref() on kernel processes during finalization.
This happened during every boot, with the `init_stage2` process.

Found by turning off kfree() scrubbing. :^)
Andreas Kling 4 年之前
父节点
当前提交
859e5741ff
共有 3 个文件被更改,包括 14 次插入6 次删除
  1. 4 0
      Kernel/Process.cpp
  2. 3 2
      Kernel/Syscalls/fork.cpp
  3. 7 4
      Kernel/ThreadBlockers.cpp

+ 4 - 0
Kernel/Process.cpp

@@ -181,6 +181,10 @@ RefPtr<Process> Process::create_user_process(RefPtr<Thread>& first_thread, const
 
     register_new(*process);
     error = 0;
+
+    // NOTE: All user processes have a leaked ref on them. It's balanced by Thread::WaitBlockCondition::finalize().
+    (void)process.leak_ref();
+
     return process;
 }
 

+ 3 - 2
Kernel/Syscalls/fork.cpp

@@ -119,9 +119,10 @@ KResultOr<FlatPtr> Process::sys$fork(RegisterState& regs)
     child_first_thread->set_state(Thread::State::Runnable);
 
     auto child_pid = child->pid().value();
-    // We need to leak one reference so we don't destroy the Process,
-    // which will be dropped by Process::reap
+
+    // NOTE: All user processes have a leaked ref on them. It's balanced by Thread::WaitBlockCondition::finalize().
     (void)child.leak_ref();
+
     return child_pid;
 }
 

+ 7 - 4
Kernel/ThreadBlockers.cpp

@@ -580,10 +580,13 @@ void Thread::WaitBlockCondition::finalize()
     // Clear the list of threads here so we can drop the references to them
     m_processes.clear();
 
-    // No more waiters, drop the last reference immediately. This may
-    // cause us to be destructed ourselves!
-    VERIFY(m_process.ref_count() > 0);
-    m_process.unref();
+    // NOTE: Kernel processes don't have a leaked ref on them.
+    if (!is_kernel_mode()) {
+        // No more waiters, drop the last reference immediately. This may
+        // cause us to be destructed ourselves!
+        VERIFY(m_process.ref_count() > 0);
+        m_process.unref();
+    }
 }
 
 Thread::WaitBlocker::WaitBlocker(int wait_options, idtype_t id_type, pid_t id, KResultOr<siginfo_t>& result)