From 81a6976e9039fc85818e4389fac9846995473098 Mon Sep 17 00:00:00 2001
From: Jelle Raaijmakers <jelle@gmta.nl>
Date: Thu, 8 Jun 2023 09:22:48 +0200
Subject: [PATCH] Kernel: De-atomicize fields for promises in `Process`

These 4 fields were made `Atomic` in
c3f668a758addcf80154d42e95a701edce4bbe59, at which time these were still
accessed unserialized and TOCTOU bugs could happen. Later, in
8ed06ad814734430193f3673b5e00861eda9aa47, we serialized access to these
fields in a number of helper methods, removing the need for `Atomic`.
---
 Kernel/Syscalls/execve.cpp |  4 ++--
 Kernel/Syscalls/fork.cpp   |  8 ++++----
 Kernel/Tasks/Process.h     | 10 +++++-----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp
index c18d11f0cd..3f26682ce6 100644
--- a/Kernel/Syscalls/execve.cpp
+++ b/Kernel/Syscalls/execve.cpp
@@ -712,8 +712,8 @@ ErrorOr<void> Process::do_exec(NonnullRefPtr<OpenFileDescription> main_program_d
     // NOTE: Be careful to not trigger any page faults below!
 
     with_mutable_protected_data([&](auto& protected_data) {
-        protected_data.promises = protected_data.execpromises.load();
-        protected_data.has_promises = protected_data.has_execpromises.load();
+        protected_data.promises = protected_data.execpromises;
+        protected_data.has_promises = protected_data.has_execpromises;
 
         protected_data.execpromises = 0;
         protected_data.has_execpromises = false;
diff --git a/Kernel/Syscalls/fork.cpp b/Kernel/Syscalls/fork.cpp
index 556d5e86e9..a148132007 100644
--- a/Kernel/Syscalls/fork.cpp
+++ b/Kernel/Syscalls/fork.cpp
@@ -99,10 +99,10 @@ ErrorOr<FlatPtr> Process::sys$fork(RegisterState& regs)
 
     with_protected_data([&](auto& my_protected_data) {
         child->with_mutable_protected_data([&](auto& child_protected_data) {
-            child_protected_data.promises = my_protected_data.promises.load();
-            child_protected_data.execpromises = my_protected_data.execpromises.load();
-            child_protected_data.has_promises = my_protected_data.has_promises.load();
-            child_protected_data.has_execpromises = my_protected_data.has_execpromises.load();
+            child_protected_data.promises = my_protected_data.promises;
+            child_protected_data.execpromises = my_protected_data.execpromises;
+            child_protected_data.has_promises = my_protected_data.has_promises;
+            child_protected_data.has_execpromises = my_protected_data.has_execpromises;
             child_protected_data.credentials = my_protected_data.credentials;
             child_protected_data.umask = my_protected_data.umask;
             child_protected_data.signal_trampoline = my_protected_data.signal_trampoline;
diff --git a/Kernel/Tasks/Process.h b/Kernel/Tasks/Process.h
index 91cb1927a8..2ce39557ac 100644
--- a/Kernel/Tasks/Process.h
+++ b/Kernel/Tasks/Process.h
@@ -120,10 +120,10 @@ class Process final
         RefPtr<TTY> tty;
         bool dumpable { false };
         bool executable_is_setid { false };
-        Atomic<bool> has_promises { false };
-        Atomic<u32> promises { 0 };
-        Atomic<bool> has_execpromises { false };
-        Atomic<u32> execpromises { 0 };
+        bool has_promises { false };
+        u32 promises { 0 };
+        bool has_execpromises { false };
+        u32 execpromises { 0 };
         mode_t umask { 022 };
         VirtualAddress signal_trampoline;
         Atomic<u32> thread_count { 0 };
@@ -520,7 +520,7 @@ public:
 
     bool has_promises() const
     {
-        return with_protected_data([](auto& protected_data) { return protected_data.has_promises.load(); });
+        return with_protected_data([](auto& protected_data) { return protected_data.has_promises; });
     }
     bool has_promised(Pledge pledge) const
     {