Kernel: Guard Process "protected data" with a spinlock

This ensures that both mutable and immutable access to the protected
data of a process is serialized.

Note that there may still be multiple TOCTOU issues around this, as we
have a bunch of convenience accessors that make it easy to introduce
them. We'll need to audit those as well.

Kernel/Process.cpp

@@ -211,14 +211,14 @@ LockRefPtr<Process> Process::create_kernel_process(LockRefPtr<Thread>& first_thr
 void Process::protect_data()
 {
     m_protected_data_refs.unref([&]() {
-        MM.set_page_writable_direct(VirtualAddress { &this->m_protected_values }, false);
+        MM.set_page_writable_direct(VirtualAddress { &this->m_protected_values_do_not_access_directly }, false);
     });
 }
 
 void Process::unprotect_data()
 {
     m_protected_data_refs.ref([&]() {
-        MM.set_page_writable_direct(VirtualAddress { &this->m_protected_values }, true);
+        MM.set_page_writable_direct(VirtualAddress { &this->m_protected_values_do_not_access_directly }, true);
     });
 }
 
@@ -234,6 +234,7 @@ ErrorOr<NonnullLockRefPtr<Process>> Process::try_create(LockRefPtr<Thread>& firs
 
 Process::Process(NonnullOwnPtr<KString> name, NonnullRefPtr<Credentials> credentials, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory, RefPtr<Custody> executable, TTY* tty, UnveilNode unveil_tree)
     : m_name(move(name))
+    , m_protected_data_lock(LockRank::None)
     , m_is_kernel_process(is_kernel_process)
     , m_executable(LockRank::None, move(executable))
     , m_current_directory(LockRank::None, move(current_directory))
@@ -242,11 +243,11 @@ Process::Process(NonnullOwnPtr<KString> name, NonnullRefPtr<Credentials> credent
     , m_wait_blocker_set(*this)
 {
     // Ensure that we protect the process data when exiting the constructor.
-    ProtectedDataMutationScope scope { *this };
-
-    m_protected_values.pid = allocate_pid();
-    m_protected_values.ppid = ppid;
-    m_protected_values.credentials = move(credentials);
+    with_mutable_protected_data([&](auto& protected_data) {
+        protected_data.pid = allocate_pid();
+        protected_data.ppid = ppid;
+        protected_data.credentials = move(credentials);
+    });
 
     dbgln_if(PROCESS_DEBUG, "Created new process {}({})", m_name, this->pid().value());
 }
@@ -402,10 +403,9 @@ void Process::crash(int signal, FlatPtr ip, bool out_of_memory)
         }
         dump_backtrace();
     }
-    {
-        ProtectedDataMutationScope scope { *this };
-        m_protected_values.termination_signal = signal;
-    }
+    with_mutable_protected_data([&](auto& protected_data) {
+        protected_data.termination_signal = signal;
+    });
     set_should_generate_coredump(!out_of_memory);
     address_space().dump_regions();
     VERIFY(is_user_process());
@@ -527,13 +527,15 @@ siginfo_t Process::wait_info() const
     siginfo.si_pid = pid().value();
     siginfo.si_uid = uid().value();
 
-    if (m_protected_values.termination_signal != 0) {
-        siginfo.si_status = m_protected_values.termination_signal;
-        siginfo.si_code = CLD_KILLED;
-    } else {
-        siginfo.si_status = m_protected_values.termination_status;
-        siginfo.si_code = CLD_EXITED;
-    }
+    with_protected_data([&](auto& protected_data) {
+        if (protected_data.termination_signal != 0) {
+            siginfo.si_status = protected_data.termination_signal;
+            siginfo.si_code = CLD_KILLED;
+        } else {
+            siginfo.si_status = protected_data.termination_status;
+            siginfo.si_code = CLD_EXITED;
+        }
+    });
     return siginfo;
 }
 
@@ -619,7 +621,7 @@ void Process::finalize()
         dbgln("\x1b[01;31mProcess '{}' exited with the veil left open\x1b[0m", name());
 
     if (g_init_pid != 0 && pid() == g_init_pid)
-        PANIC("Init process quit unexpectedly. Exit code: {}", m_protected_values.termination_status);
+        PANIC("Init process quit unexpectedly. Exit code: {}", termination_status());
 
     if (is_dumpable()) {
         if (m_should_generate_coredump) {
@@ -741,11 +743,10 @@ void Process::terminate_due_to_signal(u8 signal)
     VERIFY(signal < NSIG);
     VERIFY(&Process::current() == this);
     dbgln("Terminating {} due to signal {}", *this, signal);
-    {
-        ProtectedDataMutationScope scope { *this };
-        m_protected_values.termination_status = 0;
-        m_protected_values.termination_signal = signal;
-    }
+    with_mutable_protected_data([&](auto& protected_data) {
+        protected_data.termination_status = 0;
+        protected_data.termination_signal = signal;
+    });
     die();
 }
 
@@ -851,31 +852,34 @@ void Process::delete_perf_events_buffer()
 
 bool Process::remove_thread(Thread& thread)
 {
-    ProtectedDataMutationScope scope { *this };
-    auto thread_cnt_before = m_protected_values.thread_count.fetch_sub(1, AK::MemoryOrder::memory_order_acq_rel);
-    VERIFY(thread_cnt_before != 0);
+    u32 thread_count_before = 0;
     thread_list().with([&](auto& thread_list) {
         thread_list.remove(thread);
+        with_mutable_protected_data([&](auto& protected_data) {
+            thread_count_before = protected_data.thread_count.fetch_sub(1, AK::MemoryOrder::memory_order_acq_rel);
+            VERIFY(thread_count_before != 0);
+        });
     });
-    return thread_cnt_before == 1;
+    return thread_count_before == 1;
 }
 
 bool Process::add_thread(Thread& thread)
 {
-    ProtectedDataMutationScope scope { *this };
-    bool is_first = m_protected_values.thread_count.fetch_add(1, AK::MemoryOrder::memory_order_relaxed) == 0;
+    bool is_first = false;
     thread_list().with([&](auto& thread_list) {
         thread_list.append(thread);
+        with_mutable_protected_data([&](auto& protected_data) {
+            is_first = protected_data.thread_count.fetch_add(1, AK::MemoryOrder::memory_order_relaxed) == 0;
+        });
     });
     return is_first;
 }
 
 void Process::set_dumpable(bool dumpable)
 {
-    if (dumpable == m_protected_values.dumpable)
-        return;
-    ProtectedDataMutationScope scope { *this };
-    m_protected_values.dumpable = dumpable;
+    with_mutable_protected_data([&](auto& protected_data) {
+        protected_data.dumpable = dumpable;
+    });
 }
 
 ErrorOr<void> Process::set_coredump_property(NonnullOwnPtr<KString> key, NonnullOwnPtr<KString> value)
@@ -968,7 +972,9 @@ GroupID Process::sgid() const
 
 NonnullRefPtr<Credentials> Process::credentials() const
 {
-    return *m_protected_values.credentials;
+    return with_protected_data([&](auto& protected_data) -> NonnullRefPtr<Credentials> {
+        return *protected_data.credentials;
+    });
 }
 
 RefPtr<Custody> Process::executable()

Kernel/Process.h

@@ -135,20 +135,19 @@ public:
     friend class Thread;
     friend class Coredump;
 
-    // Helper class to temporarily unprotect a process's protected data so you can write to it.
-    class ProtectedDataMutationScope {
-    public:
-        explicit ProtectedDataMutationScope(Process& process)
-            : m_process(process)
-        {
-            m_process.unprotect_data();
-        }
-
-        ~ProtectedDataMutationScope() { m_process.protect_data(); }
+    auto with_protected_data(auto&& callback) const
+    {
+        SpinlockLocker locker(m_protected_data_lock);
+        return callback(m_protected_values_do_not_access_directly);
+    }
 
-    private:
-        Process& m_process;
-    };
+    auto with_mutable_protected_data(auto&& callback)
+    {
+        SpinlockLocker locker(m_protected_data_lock);
+        unprotect_data();
+        auto guard = ScopeGuard([&] { protect_data(); });
+        return callback(m_protected_values_do_not_access_directly);
+    }
 
     enum class State : u8 {
         Running = 0,
@@ -218,12 +217,21 @@ public:
     static SessionID get_sid_from_pgid(ProcessGroupID pgid);
 
     StringView name() const { return m_name->view(); }
-    ProcessID pid() const { return m_protected_values.pid; }
-    SessionID sid() const { return m_protected_values.sid; }
+    ProcessID pid() const
+    {
+        return with_protected_data([](auto& protected_data) { return protected_data.pid; });
+    }
+    SessionID sid() const
+    {
+        return with_protected_data([](auto& protected_data) { return protected_data.sid; });
+    }
     bool is_session_leader() const { return sid().value() == pid().value(); }
     ProcessGroupID pgid() const { return m_pg ? m_pg->pgid() : 0; }
     bool is_group_leader() const { return pgid().value() == pid().value(); }
-    ProcessID ppid() const { return m_protected_values.ppid; }
+    ProcessID ppid() const
+    {
+        return with_protected_data([](auto& protected_data) { return protected_data.ppid; });
+    }
 
     NonnullRefPtr<Credentials> credentials() const;
 
@@ -234,10 +242,16 @@ public:
     UserID suid() const;
     GroupID sgid() const;
 
-    bool is_dumpable() const { return m_protected_values.dumpable; }
+    bool is_dumpable() const
+    {
+        return with_protected_data([](auto& protected_data) { return protected_data.dumpable; });
+    }
     void set_dumpable(bool);
 
-    mode_t umask() const { return m_protected_values.umask; }
+    mode_t umask() const
+    {
+        return with_protected_data([](auto& protected_data) { return protected_data.umask; });
+    }
 
     bool in_group(GroupID) const;
 
@@ -467,19 +481,37 @@ public:
     void terminate_due_to_signal(u8 signal);
     ErrorOr<void> send_signal(u8 signal, Process* sender);
 
-    u8 termination_signal() const { return m_protected_values.termination_signal; }
-    u8 termination_status() const { return m_protected_values.termination_status; }
+    u8 termination_signal() const
+    {
+        return with_protected_data([](auto& protected_data) -> u8 {
+            return protected_data.termination_signal;
+        });
+    }
+    u8 termination_status() const
+    {
+        return with_protected_data([](auto& protected_data) { return protected_data.termination_status; });
+    }
 
     u16 thread_count() const
     {
-        return m_protected_values.thread_count.load(AK::MemoryOrder::memory_order_relaxed);
+        return with_protected_data([](auto& protected_data) {
+            return protected_data.thread_count.load(AK::MemoryOrder::memory_order_relaxed);
+        });
     }
 
     Mutex& big_lock() { return m_big_lock; }
     Mutex& ptrace_lock() { return m_ptrace_lock; }
 
-    bool has_promises() const { return m_protected_values.has_promises; }
-    bool has_promised(Pledge pledge) const { return (m_protected_values.promises & (1U << (u32)pledge)) != 0; }
+    bool has_promises() const
+    {
+        return with_protected_data([](auto& protected_data) { return protected_data.has_promises.load(); });
+    }
+    bool has_promised(Pledge pledge) const
+    {
+        return with_protected_data([&](auto& protected_data) {
+            return (protected_data.promises & (1U << (u32)pledge)) != 0;
+        });
+    }
 
     VeilState veil_state() const
     {
@@ -539,7 +571,10 @@ public:
     Memory::AddressSpace& address_space() { return *m_space; }
     Memory::AddressSpace const& address_space() const { return *m_space; }
 
-    VirtualAddress signal_trampoline() const { return m_protected_values.signal_trampoline; }
+    VirtualAddress signal_trampoline() const
+    {
+        return with_protected_data([](auto& protected_data) { return protected_data.signal_trampoline; });
+    }
 
     ErrorOr<void> require_promise(Pledge);
     ErrorOr<void> require_no_promises() const;
@@ -635,6 +670,7 @@ private:
 
     LockRefPtr<ProcessGroup> m_pg;
 
+    RecursiveSpinlock mutable m_protected_data_lock;
     AtomicEdgeAction<u32> m_protected_data_refs;
     void protect_data();
     void unprotect_data();
@@ -870,7 +906,7 @@ private:
     Array<SignalActionData, NSIG> m_signal_action_data;
 
     static_assert(sizeof(ProtectedValues) < (PAGE_SIZE));
-    alignas(4096) ProtectedValues m_protected_values;
+    alignas(4096) ProtectedValues m_protected_values_do_not_access_directly;
     u8 m_protected_values_padding[PAGE_SIZE - sizeof(ProtectedValues)];
 
 public:

Kernel/Syscalls/disown.cpp

@@ -17,8 +17,9 @@ ErrorOr<FlatPtr> Process::sys$disown(ProcessID pid)
         return ESRCH;
     if (process->ppid() != this->pid())
         return ECHILD;
-    ProtectedDataMutationScope scope(*process);
-    process->m_protected_values.ppid = 0;
+    process->with_mutable_protected_data([](auto& protected_data) {
+        protected_data.ppid = 0;
+    });
     process->disowned_by_waiter(*this);
     return 0;
 }

Kernel/Syscalls/execve.cpp

@@ -542,11 +542,10 @@ ErrorOr<void> Process::do_exec(NonnullLockRefPtr<OpenFileDescription> main_progr
 
     kill_threads_except_self();
 
-    {
-        ProtectedDataMutationScope scope { *this };
-        m_protected_values.credentials = move(new_credentials);
-    }
-    set_dumpable(!executable_is_setid);
+    with_mutable_protected_data([&](auto& protected_data) {
+        protected_data.credentials = move(new_credentials);
+        protected_data.dumpable = !executable_is_setid;
+    });
 
     // We make sure to enter the new address space before destroying the old one.
     // This ensures that the process always has a valid page directory.
@@ -627,19 +626,18 @@ ErrorOr<void> Process::do_exec(NonnullLockRefPtr<OpenFileDescription> main_progr
 
     // NOTE: Be careful to not trigger any page faults below!
 
-    {
-        ProtectedDataMutationScope scope { *this };
-        m_protected_values.promises = m_protected_values.execpromises.load();
-        m_protected_values.has_promises = m_protected_values.has_execpromises.load();
+    with_mutable_protected_data([&](auto& protected_data) {
+        protected_data.promises = protected_data.execpromises.load();
+        protected_data.has_promises = protected_data.has_execpromises.load();
 
-        m_protected_values.execpromises = 0;
-        m_protected_values.has_execpromises = false;
+        protected_data.execpromises = 0;
+        protected_data.has_execpromises = false;
 
-        m_protected_values.signal_trampoline = signal_trampoline_region->vaddr();
+        protected_data.signal_trampoline = signal_trampoline_region->vaddr();
 
         // FIXME: PID/TID ISSUE
-        m_protected_values.pid = new_main_thread->tid().value();
-    }
+        protected_data.pid = new_main_thread->tid().value();
+    });
 
     auto tsr_result = new_main_thread->make_thread_specific_region({});
     if (tsr_result.is_error()) {

Kernel/Syscalls/exit.cpp

@@ -18,11 +18,10 @@ void Process::sys$exit(int status)
         VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this);
     }
 
-    {
-        ProtectedDataMutationScope scope { *this };
-        m_protected_values.termination_status = status;
-        m_protected_values.termination_signal = 0;
-    }
+    with_mutable_protected_data([status](auto& protected_data) {
+        protected_data.termination_status = status;
+        protected_data.termination_signal = 0;
+    });
 
     auto* current_thread = Thread::current();
     current_thread->set_profiling_suppressed();

Kernel/Syscalls/fork.cpp

@@ -49,18 +49,19 @@ ErrorOr<FlatPtr> Process::sys$fork(RegisterState& regs)
 
     child->m_pg = m_pg;
 
-    {
-        ProtectedDataMutationScope scope { *child };
-        child->m_protected_values.promises = m_protected_values.promises.load();
-        child->m_protected_values.execpromises = m_protected_values.execpromises.load();
-        child->m_protected_values.has_promises = m_protected_values.has_promises.load();
-        child->m_protected_values.has_execpromises = m_protected_values.has_execpromises.load();
-        child->m_protected_values.sid = m_protected_values.sid;
-        child->m_protected_values.credentials = m_protected_values.credentials;
-        child->m_protected_values.umask = m_protected_values.umask;
-        child->m_protected_values.signal_trampoline = m_protected_values.signal_trampoline;
-        child->m_protected_values.dumpable = m_protected_values.dumpable;
-    }
+    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.sid = my_protected_data.sid;
+            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;
+            child_protected_data.dumpable = my_protected_data.dumpable;
+        });
+    });
 
     dbgln_if(FORK_DEBUG, "fork: child={}", child);
     child->address_space().set_enforces_syscall_regions(address_space().enforces_syscall_regions());

Kernel/Syscalls/pledge.cpp

@@ -46,43 +46,48 @@ ErrorOr<FlatPtr> Process::sys$pledge(Userspace<Syscall::SC_pledge_params const*>
     if (promises) {
         if (!parse_pledge(promises->view(), new_promises))
             return EINVAL;
-
-        if (m_protected_values.has_promises && (new_promises & ~m_protected_values.promises)) {
-            if (!(m_protected_values.promises & (1u << (u32)Pledge::no_error)))
-                return EPERM;
-            new_promises &= m_protected_values.promises;
-        }
     }
 
     u32 new_execpromises = 0;
     if (execpromises) {
         if (!parse_pledge(execpromises->view(), new_execpromises))
             return EINVAL;
-        if (m_protected_values.has_execpromises && (new_execpromises & ~m_protected_values.execpromises)) {
-            if (!(m_protected_values.promises & (1u << (u32)Pledge::no_error)))
-                return EPERM;
-            new_execpromises &= m_protected_values.execpromises;
-        }
     }
 
-    // Only apply promises after all validation has occurred, this ensures
-    // we don't introduce logic bugs like applying the promises, and then
-    // erroring out when parsing the exec promises later. Such bugs silently
-    // leave the caller in an unexpected state.
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        if (promises) {
+            if (protected_data.has_promises && (new_promises & ~protected_data.promises)) {
+                if (!(protected_data.promises & (1u << (u32)Pledge::no_error)))
+                    return EPERM;
+                new_promises &= protected_data.promises;
+            }
+        }
 
-    ProtectedDataMutationScope scope { *this };
+        if (execpromises) {
+            if (protected_data.has_execpromises && (new_execpromises & ~protected_data.execpromises)) {
+                if (!(protected_data.promises & (1u << (u32)Pledge::no_error)))
+                    return EPERM;
+                new_execpromises &= protected_data.execpromises;
+            }
+        }
 
-    if (promises) {
-        m_protected_values.has_promises = true;
-        m_protected_values.promises = new_promises;
-    }
+        // Only apply promises after all validation has occurred, this ensures
+        // we don't introduce logic bugs like applying the promises, and then
+        // erroring out when parsing the exec promises later. Such bugs silently
+        // leave the caller in an unexpected state.
 
-    if (execpromises) {
-        m_protected_values.has_execpromises = true;
-        m_protected_values.execpromises = new_execpromises;
-    }
+        if (promises) {
+            protected_data.has_promises = true;
+            protected_data.promises = new_promises;
+        }
+
+        if (execpromises) {
+            protected_data.has_execpromises = true;
+            protected_data.execpromises = new_execpromises;
+        }
 
-    return 0;
+        return 0;
+    });
 }
 
 }

Kernel/Syscalls/process.cpp

@@ -20,7 +20,7 @@ ErrorOr<FlatPtr> Process::sys$getppid()
 {
     VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this);
     TRY(require_promise(Pledge::stdio));
-    return m_protected_values.ppid.value();
+    return with_protected_data([](auto& protected_data) { return protected_data.ppid.value(); });
 }
 
 ErrorOr<FlatPtr> Process::sys$get_process_name(Userspace<char*> buffer, size_t buffer_size)

Kernel/Syscalls/setpgid.cpp

@@ -40,9 +40,10 @@ ErrorOr<FlatPtr> Process::sys$setsid()
 
     m_pg = TRY(ProcessGroup::try_create(ProcessGroupID(pid().value())));
     m_tty = nullptr;
-    ProtectedDataMutationScope scope { *this };
-    m_protected_values.sid = pid().value();
-    return sid().value();
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        protected_data.sid = pid().value();
+        return protected_data.sid.value();
+    });
 }
 
 ErrorOr<FlatPtr> Process::sys$getpgid(pid_t pid)

Kernel/Syscalls/setuid.cpp

@@ -17,27 +17,27 @@ ErrorOr<FlatPtr> Process::sys$seteuid(UserID new_euid)
     if (new_euid == (uid_t)-1)
         return EINVAL;
 
-    auto credentials = this->credentials();
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        auto credentials = this->credentials();
 
-    if (new_euid != credentials->uid() && new_euid != credentials->suid() && !credentials->is_superuser())
-        return EPERM;
+        if (new_euid != credentials->uid() && new_euid != credentials->suid() && !credentials->is_superuser())
+            return EPERM;
 
-    auto new_credentials = TRY(Credentials::create(
-        credentials->uid(),
-        credentials->gid(),
-        new_euid,
-        credentials->egid(),
-        credentials->suid(),
-        credentials->sgid(),
-        credentials->extra_gids()));
-
-    ProtectedDataMutationScope scope { *this };
+        auto new_credentials = TRY(Credentials::create(
+            credentials->uid(),
+            credentials->gid(),
+            new_euid,
+            credentials->egid(),
+            credentials->suid(),
+            credentials->sgid(),
+            credentials->extra_gids()));
 
-    if (credentials->euid() != new_euid)
-        set_dumpable(false);
+        if (credentials->euid() != new_euid)
+            protected_data.dumpable = false;
 
-    m_protected_values.credentials = move(new_credentials);
-    return 0;
+        protected_data.credentials = move(new_credentials);
+        return 0;
+    });
 }
 
 ErrorOr<FlatPtr> Process::sys$setegid(GroupID new_egid)
@@ -48,27 +48,27 @@ ErrorOr<FlatPtr> Process::sys$setegid(GroupID new_egid)
     if (new_egid == (uid_t)-1)
         return EINVAL;
 
-    auto credentials = this->credentials();
-
-    if (new_egid != credentials->gid() && new_egid != credentials->sgid() && !credentials->is_superuser())
-        return EPERM;
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        auto credentials = this->credentials();
 
-    auto new_credentials = TRY(Credentials::create(
-        credentials->uid(),
-        credentials->gid(),
-        credentials->euid(),
-        new_egid,
-        credentials->suid(),
-        credentials->sgid(),
-        credentials->extra_gids()));
+        if (new_egid != credentials->gid() && new_egid != credentials->sgid() && !credentials->is_superuser())
+            return EPERM;
 
-    ProtectedDataMutationScope scope { *this };
+        auto new_credentials = TRY(Credentials::create(
+            credentials->uid(),
+            credentials->gid(),
+            credentials->euid(),
+            new_egid,
+            credentials->suid(),
+            credentials->sgid(),
+            credentials->extra_gids()));
 
-    if (credentials->egid() != new_egid)
-        set_dumpable(false);
+        if (credentials->egid() != new_egid)
+            protected_data.dumpable = false;
 
-    m_protected_values.credentials = move(new_credentials);
-    return 0;
+        protected_data.credentials = move(new_credentials);
+        return 0;
+    });
 }
 
 ErrorOr<FlatPtr> Process::sys$setuid(UserID new_uid)
@@ -79,27 +79,27 @@ ErrorOr<FlatPtr> Process::sys$setuid(UserID new_uid)
     if (new_uid == (uid_t)-1)
         return EINVAL;
 
-    auto credentials = this->credentials();
-
-    if (new_uid != credentials->uid() && new_uid != credentials->euid() && !credentials->is_superuser())
-        return EPERM;
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        auto credentials = this->credentials();
 
-    auto new_credentials = TRY(Credentials::create(
-        new_uid,
-        credentials->gid(),
-        new_uid,
-        credentials->egid(),
-        new_uid,
-        credentials->sgid(),
-        credentials->extra_gids()));
+        if (new_uid != credentials->uid() && new_uid != credentials->euid() && !credentials->is_superuser())
+            return EPERM;
 
-    ProtectedDataMutationScope scope { *this };
+        auto new_credentials = TRY(Credentials::create(
+            new_uid,
+            credentials->gid(),
+            new_uid,
+            credentials->egid(),
+            new_uid,
+            credentials->sgid(),
+            credentials->extra_gids()));
 
-    if (credentials->euid() != new_uid)
-        set_dumpable(false);
+        if (credentials->euid() != new_uid)
+            protected_data.dumpable = false;
 
-    m_protected_values.credentials = move(new_credentials);
-    return 0;
+        protected_data.credentials = move(new_credentials);
+        return 0;
+    });
 }
 
 ErrorOr<FlatPtr> Process::sys$setgid(GroupID new_gid)
@@ -110,27 +110,27 @@ ErrorOr<FlatPtr> Process::sys$setgid(GroupID new_gid)
     if (new_gid == (uid_t)-1)
         return EINVAL;
 
-    auto credentials = this->credentials();
-
-    if (new_gid != credentials->gid() && new_gid != credentials->egid() && !credentials->is_superuser())
-        return EPERM;
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        auto credentials = this->credentials();
 
-    auto new_credentials = TRY(Credentials::create(
-        credentials->uid(),
-        new_gid,
-        credentials->euid(),
-        new_gid,
-        credentials->suid(),
-        new_gid,
-        credentials->extra_gids()));
+        if (new_gid != credentials->gid() && new_gid != credentials->egid() && !credentials->is_superuser())
+            return EPERM;
 
-    ProtectedDataMutationScope scope { *this };
+        auto new_credentials = TRY(Credentials::create(
+            credentials->uid(),
+            new_gid,
+            credentials->euid(),
+            new_gid,
+            credentials->suid(),
+            new_gid,
+            credentials->extra_gids()));
 
-    if (credentials->egid() != new_gid)
-        set_dumpable(false);
+        if (credentials->egid() != new_gid)
+            protected_data.dumpable = false;
 
-    m_protected_values.credentials = move(new_credentials);
-    return 0;
+        protected_data.credentials = move(new_credentials);
+        return 0;
+    });
 }
 
 ErrorOr<FlatPtr> Process::sys$setreuid(UserID new_ruid, UserID new_euid)
@@ -138,36 +138,36 @@ ErrorOr<FlatPtr> Process::sys$setreuid(UserID new_ruid, UserID new_euid)
     VERIFY_NO_PROCESS_BIG_LOCK(this);
     TRY(require_promise(Pledge::id));
 
-    auto credentials = this->credentials();
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        auto credentials = this->credentials();
 
-    if (new_ruid == (uid_t)-1)
-        new_ruid = credentials->uid();
-    if (new_euid == (uid_t)-1)
-        new_euid = credentials->euid();
-
-    auto ok = [&credentials](UserID id) { return id == credentials->uid() || id == credentials->euid() || id == credentials->suid(); };
-    if (!ok(new_ruid) || !ok(new_euid))
-        return EPERM;
+        if (new_ruid == (uid_t)-1)
+            new_ruid = credentials->uid();
+        if (new_euid == (uid_t)-1)
+            new_euid = credentials->euid();
 
-    if (new_ruid < (uid_t)-1 || new_euid < (uid_t)-1)
-        return EINVAL;
+        auto ok = [&credentials](UserID id) { return id == credentials->uid() || id == credentials->euid() || id == credentials->suid(); };
+        if (!ok(new_ruid) || !ok(new_euid))
+            return EPERM;
 
-    auto new_credentials = TRY(Credentials::create(
-        new_ruid,
-        credentials->gid(),
-        new_euid,
-        credentials->egid(),
-        credentials->suid(),
-        credentials->sgid(),
-        credentials->extra_gids()));
+        if (new_ruid < (uid_t)-1 || new_euid < (uid_t)-1)
+            return EINVAL;
 
-    ProtectedDataMutationScope scope { *this };
+        auto new_credentials = TRY(Credentials::create(
+            new_ruid,
+            credentials->gid(),
+            new_euid,
+            credentials->egid(),
+            credentials->suid(),
+            credentials->sgid(),
+            credentials->extra_gids()));
 
-    if (credentials->euid() != new_euid)
-        set_dumpable(false);
+        if (credentials->euid() != new_euid)
+            protected_data.dumpable = false;
 
-    m_protected_values.credentials = move(new_credentials);
-    return 0;
+        protected_data.credentials = move(new_credentials);
+        return 0;
+    });
 }
 
 ErrorOr<FlatPtr> Process::sys$setresuid(UserID new_ruid, UserID new_euid, UserID new_suid)
@@ -175,35 +175,35 @@ ErrorOr<FlatPtr> Process::sys$setresuid(UserID new_ruid, UserID new_euid, UserID
     VERIFY_NO_PROCESS_BIG_LOCK(this);
     TRY(require_promise(Pledge::id));
 
-    auto credentials = this->credentials();
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        auto credentials = this->credentials();
 
-    if (new_ruid == (uid_t)-1)
-        new_ruid = credentials->uid();
-    if (new_euid == (uid_t)-1)
-        new_euid = credentials->euid();
-    if (new_suid == (uid_t)-1)
-        new_suid = credentials->suid();
-
-    auto ok = [&credentials](UserID id) { return id == credentials->uid() || id == credentials->euid() || id == credentials->suid(); };
-    if ((!ok(new_ruid) || !ok(new_euid) || !ok(new_suid)) && !credentials->is_superuser())
-        return EPERM;
-
-    auto new_credentials = TRY(Credentials::create(
-        new_ruid,
-        credentials->gid(),
-        new_euid,
-        credentials->egid(),
-        new_suid,
-        credentials->sgid(),
-        credentials->extra_gids()));
-
-    ProtectedDataMutationScope scope { *this };
-
-    if (credentials->euid() != new_euid)
-        set_dumpable(false);
-
-    m_protected_values.credentials = move(new_credentials);
-    return 0;
+        if (new_ruid == (uid_t)-1)
+            new_ruid = credentials->uid();
+        if (new_euid == (uid_t)-1)
+            new_euid = credentials->euid();
+        if (new_suid == (uid_t)-1)
+            new_suid = credentials->suid();
+
+        auto ok = [&credentials](UserID id) { return id == credentials->uid() || id == credentials->euid() || id == credentials->suid(); };
+        if ((!ok(new_ruid) || !ok(new_euid) || !ok(new_suid)) && !credentials->is_superuser())
+            return EPERM;
+
+        auto new_credentials = TRY(Credentials::create(
+            new_ruid,
+            credentials->gid(),
+            new_euid,
+            credentials->egid(),
+            new_suid,
+            credentials->sgid(),
+            credentials->extra_gids()));
+
+        if (credentials->euid() != new_euid)
+            protected_data.dumpable = false;
+
+        protected_data.credentials = move(new_credentials);
+        return 0;
+    });
 }
 
 ErrorOr<FlatPtr> Process::sys$setresgid(GroupID new_rgid, GroupID new_egid, GroupID new_sgid)
@@ -211,35 +211,35 @@ ErrorOr<FlatPtr> Process::sys$setresgid(GroupID new_rgid, GroupID new_egid, Grou
     VERIFY_NO_PROCESS_BIG_LOCK(this);
     TRY(require_promise(Pledge::id));
 
-    auto credentials = this->credentials();
-
-    if (new_rgid == (gid_t)-1)
-        new_rgid = credentials->gid();
-    if (new_egid == (gid_t)-1)
-        new_egid = credentials->egid();
-    if (new_sgid == (gid_t)-1)
-        new_sgid = credentials->sgid();
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        auto credentials = this->credentials();
 
-    auto ok = [&credentials](GroupID id) { return id == credentials->gid() || id == credentials->egid() || id == credentials->sgid(); };
-    if ((!ok(new_rgid) || !ok(new_egid) || !ok(new_sgid)) && !credentials->is_superuser())
-        return EPERM;
+        if (new_rgid == (gid_t)-1)
+            new_rgid = credentials->gid();
+        if (new_egid == (gid_t)-1)
+            new_egid = credentials->egid();
+        if (new_sgid == (gid_t)-1)
+            new_sgid = credentials->sgid();
 
-    auto new_credentials = TRY(Credentials::create(
-        credentials->uid(),
-        new_rgid,
-        credentials->euid(),
-        new_egid,
-        credentials->suid(),
-        new_sgid,
-        credentials->extra_gids()));
+        auto ok = [&credentials](GroupID id) { return id == credentials->gid() || id == credentials->egid() || id == credentials->sgid(); };
+        if ((!ok(new_rgid) || !ok(new_egid) || !ok(new_sgid)) && !credentials->is_superuser())
+            return EPERM;
 
-    ProtectedDataMutationScope scope { *this };
+        auto new_credentials = TRY(Credentials::create(
+            credentials->uid(),
+            new_rgid,
+            credentials->euid(),
+            new_egid,
+            credentials->suid(),
+            new_sgid,
+            credentials->extra_gids()));
 
-    if (credentials->egid() != new_egid)
-        set_dumpable(false);
+        if (credentials->egid() != new_egid)
+            protected_data.dumpable = false;
 
-    m_protected_values.credentials = move(new_credentials);
-    return 0;
+        protected_data.credentials = move(new_credentials);
+        return 0;
+    });
 }
 
 ErrorOr<FlatPtr> Process::sys$setgroups(size_t count, Userspace<GroupID const*> user_gids)
@@ -250,49 +250,49 @@ ErrorOr<FlatPtr> Process::sys$setgroups(size_t count, Userspace<GroupID const*>
     if (count > NGROUPS_MAX)
         return EINVAL;
 
-    auto credentials = this->credentials();
-
-    if (!credentials->is_superuser())
-        return EPERM;
-
-    if (!count) {
-        ProtectedDataMutationScope scope { *this };
-        m_protected_values.credentials = TRY(Credentials::create(
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        auto credentials = this->credentials();
+
+        if (!credentials->is_superuser())
+            return EPERM;
+
+        if (!count) {
+            protected_data.credentials = TRY(Credentials::create(
+                credentials->uid(),
+                credentials->gid(),
+                credentials->euid(),
+                credentials->egid(),
+                credentials->suid(),
+                credentials->sgid(),
+                {}));
+            return 0;
+        }
+
+        Vector<GroupID> new_extra_gids;
+        TRY(new_extra_gids.try_resize(count));
+        TRY(copy_n_from_user(new_extra_gids.data(), user_gids, count));
+
+        HashTable<GroupID> unique_extra_gids;
+        for (auto& extra_gid : new_extra_gids) {
+            if (extra_gid != credentials->gid())
+                TRY(unique_extra_gids.try_set(extra_gid));
+        }
+
+        new_extra_gids.clear_with_capacity();
+        for (auto extra_gid : unique_extra_gids) {
+            TRY(new_extra_gids.try_append(extra_gid));
+        }
+
+        protected_data.credentials = TRY(Credentials::create(
             credentials->uid(),
             credentials->gid(),
             credentials->euid(),
             credentials->egid(),
             credentials->suid(),
             credentials->sgid(),
-            {}));
+            new_extra_gids.span()));
         return 0;
-    }
-
-    Vector<GroupID> new_extra_gids;
-    TRY(new_extra_gids.try_resize(count));
-    TRY(copy_n_from_user(new_extra_gids.data(), user_gids, count));
-
-    HashTable<GroupID> unique_extra_gids;
-    for (auto& extra_gid : new_extra_gids) {
-        if (extra_gid != credentials->gid())
-            TRY(unique_extra_gids.try_set(extra_gid));
-    }
-
-    new_extra_gids.clear_with_capacity();
-    for (auto extra_gid : unique_extra_gids) {
-        TRY(new_extra_gids.try_append(extra_gid));
-    }
-
-    ProtectedDataMutationScope scope { *this };
-    m_protected_values.credentials = TRY(Credentials::create(
-        credentials->uid(),
-        credentials->gid(),
-        credentials->euid(),
-        credentials->egid(),
-        credentials->suid(),
-        credentials->sgid(),
-        new_extra_gids.span()));
-    return 0;
+    });
 }
 
 }

Kernel/Syscalls/umask.cpp

@@ -12,10 +12,11 @@ ErrorOr<FlatPtr> Process::sys$umask(mode_t mask)
 {
     VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this);
     TRY(require_promise(Pledge::stdio));
-    auto old_mask = m_protected_values.umask;
-    ProtectedDataMutationScope scope { *this };
-    m_protected_values.umask = mask & 0777;
-    return old_mask;
+    return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
+        auto old_mask = protected_data.umask;
+        protected_data.umask = mask & 0777;
+        return old_mask;
+    });
 }
 
 }