Sfoglia il codice sorgente

Kernel: Move select Process members into protected memory

Process member variable like m_euid are very valuable targets for
kernel exploits and until now they have been writable at all times.

This patch moves m_euid along with a whole bunch of other members
into a new Process::ProtectedData struct. This struct is remapped
as read-only memory whenever we don't need to write to it.

This means that a kernel write primitive is no longer enough to
overwrite a process's effective UID, you must first unprotect the
protected data where the UID is stored. :^)
Andreas Kling 4 anni fa
parent
commit
cbcf891040

+ 45 - 18
Kernel/Process.cpp

@@ -25,7 +25,6 @@
  */
 
 #include <AK/Demangle.h>
-#include <AK/QuickSort.h>
 #include <AK/StdLibExtras.h>
 #include <AK/StringBuilder.h>
 #include <AK/Time.h>
@@ -38,7 +37,6 @@
 #include <Kernel/FileSystem/Custody.h>
 #include <Kernel/FileSystem/FileDescription.h>
 #include <Kernel/FileSystem/VirtualFileSystem.h>
-#include <Kernel/Heap/kmalloc.h>
 #include <Kernel/KBufferBuilder.h>
 #include <Kernel/KSyms.h>
 #include <Kernel/Module.h>
@@ -51,7 +49,6 @@
 #include <Kernel/VM/AnonymousVMObject.h>
 #include <Kernel/VM/PageDirectory.h>
 #include <Kernel/VM/PrivateInodeVMObject.h>
-#include <Kernel/VM/ProcessPagingScope.h>
 #include <Kernel/VM/SharedInodeVMObject.h>
 #include <LibC/errno_numbers.h>
 #include <LibC/limits.h>
@@ -113,7 +110,7 @@ NonnullRefPtrVector<Process> Process::all_processes()
 
 bool Process::in_group(gid_t gid) const
 {
-    return m_gid == gid || m_extra_gids.contains_slow(gid);
+    return this->gid() == gid || m_extra_gids.contains_slow(gid);
 }
 
 void Process::kill_threads_except_self()
@@ -212,15 +209,31 @@ RefPtr<Process> Process::create_kernel_process(RefPtr<Thread>& first_thread, Str
     return process;
 }
 
+const Process::ProtectedData& Process::protected_data() const
+{
+    return *reinterpret_cast<const ProtectedData*>(m_protected_data->data());
+}
+
+void Process::protect_data()
+{
+    auto& region = m_protected_data->impl().region();
+    if (!region.is_writable())
+        return;
+    region.set_writable(false);
+    region.remap();
+}
+
+void Process::unprotect_data()
+{
+    auto& region = m_protected_data->impl().region();
+    if (region.is_writable())
+        return;
+    region.set_writable(true);
+    region.remap();
+}
+
 Process::Process(RefPtr<Thread>& first_thread, const String& name, uid_t uid, gid_t gid, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> cwd, RefPtr<Custody> executable, TTY* tty, Process* fork_parent)
     : m_name(move(name))
-    , m_pid(allocate_pid())
-    , m_euid(uid)
-    , m_egid(gid)
-    , m_uid(uid)
-    , m_gid(gid)
-    , m_suid(uid)
-    , m_sgid(gid)
     , m_is_kernel_process(is_kernel_process)
     , m_executable(move(executable))
     , m_cwd(move(cwd))
@@ -228,7 +241,21 @@ Process::Process(RefPtr<Thread>& first_thread, const String& name, uid_t uid, gi
     , m_ppid(ppid)
     , m_wait_block_condition(*this)
 {
-    dbgln_if(PROCESS_DEBUG, "Created new process {}({})", m_name, m_pid.value());
+    m_protected_data = KBuffer::try_create_with_size(sizeof(ProtectedData));
+    VERIFY(m_protected_data);
+
+    {
+        MutableProtectedData protected_data { *this };
+        protected_data->pid = allocate_pid();
+        protected_data->uid = uid;
+        protected_data->gid = gid;
+        protected_data->euid = uid;
+        protected_data->egid = gid;
+        protected_data->suid = uid;
+        protected_data->sgid = gid;
+    }
+
+    dbgln_if(PROCESS_DEBUG, "Created new process {}({})", m_name, this->pid().value());
 
     m_space = Space::create(*this, fork_parent ? &fork_parent->space() : nullptr);
 
@@ -433,8 +460,8 @@ bool Process::dump_core()
 {
     VERIFY(is_dumpable());
     VERIFY(should_core_dump());
-    dbgln("Generating coredump for pid: {}", m_pid.value());
-    auto coredump_path = String::formatted("/tmp/coredump/{}_{}_{}", name(), m_pid.value(), RTC::now());
+    dbgln("Generating coredump for pid: {}", pid().value());
+    auto coredump_path = String::formatted("/tmp/coredump/{}_{}_{}", name(), pid().value(), RTC::now());
     auto coredump = CoreDump::create(*this, coredump_path);
     if (!coredump)
         return false;
@@ -445,8 +472,8 @@ bool Process::dump_perfcore()
 {
     VERIFY(is_dumpable());
     VERIFY(m_perf_event_buffer);
-    dbgln("Generating perfcore for pid: {}", m_pid.value());
-    auto description_or_error = VFS::the().open(String::formatted("perfcore.{}", m_pid.value()), O_CREAT | O_EXCL, 0400, current_directory(), UidAndGid { m_uid, m_gid });
+    dbgln("Generating perfcore for pid: {}", pid().value());
+    auto description_or_error = VFS::the().open(String::formatted("perfcore.{}", pid().value()), O_CREAT | O_EXCL, 0400, current_directory(), UidAndGid { uid(), gid() });
     if (description_or_error.is_error())
         return false;
     auto& description = description_or_error.value();
@@ -547,7 +574,7 @@ void Process::die()
         ScopedSpinLock lock(g_processes_lock);
         for (auto* process = g_processes->head(); process;) {
             auto* next_process = process->next();
-            if (process->has_tracee_thread(m_pid)) {
+            if (process->has_tracee_thread(pid())) {
                 dbgln_if(PROCESS_DEBUG, "Process {} ({}) is attached by {} ({}) which will exit", process->name(), process->pid(), name(), pid());
                 process->stop_tracing();
                 auto err = process->send_signal(SIGSTOP, this);
@@ -576,7 +603,7 @@ void Process::terminate_due_to_signal(u8 signal)
 KResult Process::send_signal(u8 signal, Process* sender)
 {
     // Try to send it to the "obvious" main thread:
-    auto receiver_thread = Thread::from_tid(m_pid.value());
+    auto receiver_thread = Thread::from_tid(pid().value());
     // If the main thread has died, there may still be other threads:
     if (!receiver_thread) {
         // The first one should be good enough.

+ 43 - 23
Kernel/Process.h

@@ -112,6 +112,33 @@ class Process
     friend class Thread;
     friend class CoreDump;
 
+    struct ProtectedData {
+        ProcessID pid { 0 };
+        SessionID sid { 0 };
+        uid_t euid { 0 };
+        gid_t egid { 0 };
+        uid_t uid { 0 };
+        gid_t gid { 0 };
+        uid_t suid { 0 };
+        gid_t sgid { 0 };
+    };
+
+    // Helper class to temporarily unprotect a process's protected data so you can write to it.
+    class MutableProtectedData {
+    public:
+        explicit MutableProtectedData(Process& process)
+            : m_process(process)
+        {
+            m_process.unprotect_data();
+        }
+
+        ~MutableProtectedData() { m_process.protect_data(); }
+        ProtectedData* operator->() { return &const_cast<ProtectedData&>(m_process.protected_data()); }
+
+    private:
+        Process& m_process;
+    };
+
 public:
     inline static Process* current()
     {
@@ -169,18 +196,18 @@ public:
     static SessionID get_sid_from_pgid(ProcessGroupID pgid);
 
     const String& name() const { return m_name; }
-    ProcessID pid() const { return m_pid; }
-    SessionID sid() const { return m_sid; }
-    bool is_session_leader() const { return m_sid.value() == m_pid.value(); }
+    ProcessID pid() const { return protected_data().pid; }
+    SessionID sid() const { return protected_data().sid; }
+    bool is_session_leader() const { return protected_data().sid.value() == protected_data().pid.value(); }
     ProcessGroupID pgid() const { return m_pg ? m_pg->pgid() : 0; }
-    bool is_group_leader() const { return pgid().value() == m_pid.value(); }
+    bool is_group_leader() const { return pgid().value() == protected_data().pid.value(); }
     Span<const gid_t> extra_gids() const { return m_extra_gids; }
-    uid_t euid() const { return m_euid; }
-    gid_t egid() const { return m_egid; }
-    uid_t uid() const { return m_uid; }
-    gid_t gid() const { return m_gid; }
-    uid_t suid() const { return m_suid; }
-    gid_t sgid() const { return m_sgid; }
+    uid_t euid() const { return protected_data().euid; }
+    gid_t egid() const { return protected_data().egid; }
+    uid_t uid() const { return protected_data().uid; }
+    gid_t gid() const { return protected_data().gid; }
+    uid_t suid() const { return protected_data().suid; }
+    gid_t sgid() const { return protected_data().sgid; }
     ProcessID ppid() const { return m_ppid; }
 
     bool is_dumpable() const { return m_dumpable; }
@@ -392,10 +419,7 @@ public:
 
     KResultOr<LoadResult> load(NonnullRefPtr<FileDescription> main_program_description, RefPtr<FileDescription> interpreter_description, const Elf32_Ehdr& main_program_header);
 
-    bool is_superuser() const
-    {
-        return m_euid == 0;
-    }
+    bool is_superuser() const { return euid() == 0; }
 
     void terminate_due_to_signal(u8 signal);
     KResult send_signal(u8 signal, Process* sender);
@@ -510,16 +534,12 @@ private:
     OwnPtr<Space> m_space;
     VirtualAddress m_signal_trampoline;
 
-    ProcessID m_pid { 0 };
-    SessionID m_sid { 0 };
     RefPtr<ProcessGroup> m_pg;
 
-    uid_t m_euid { 0 };
-    gid_t m_egid { 0 };
-    uid_t m_uid { 0 };
-    gid_t m_gid { 0 };
-    uid_t m_suid { 0 };
-    gid_t m_sgid { 0 };
+    const ProtectedData& protected_data() const;
+    void protect_data();
+    void unprotect_data();
+    OwnPtr<KBuffer> m_protected_data;
 
     OwnPtr<ThreadTracer> m_tracer;
 
@@ -631,7 +651,7 @@ inline void Process::for_each_child(Callback callback)
     ScopedSpinLock lock(g_processes_lock);
     for (auto* process = g_processes->head(); process;) {
         auto* next_process = process->next();
-        if (process->ppid() == my_pid || process->has_tracee_thread(m_pid)) {
+        if (process->ppid() == my_pid || process->has_tracee_thread(pid())) {
             if (callback(*process) == IterationDecision::Break)
                 break;
         }

+ 9 - 5
Kernel/Syscalls/execve.cpp

@@ -507,11 +507,15 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
     if (!(main_program_description->custody()->mount_flags() & MS_NOSUID)) {
         if (main_program_metadata.is_setuid()) {
             executable_is_setid = true;
-            m_euid = m_suid = main_program_metadata.uid;
+            MutableProtectedData protected_data { *this };
+            protected_data->euid = main_program_metadata.uid;
+            protected_data->suid = main_program_metadata.uid;
         }
         if (main_program_metadata.is_setgid()) {
             executable_is_setid = true;
-            m_egid = m_sgid = main_program_metadata.gid;
+            MutableProtectedData protected_data { *this };
+            protected_data->egid = main_program_metadata.gid;
+            protected_data->sgid = main_program_metadata.gid;
         }
     }
 
@@ -574,7 +578,7 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
     }
     VERIFY(new_main_thread);
 
-    auto auxv = generate_auxiliary_vector(load_result.load_base, load_result.entry_eip, m_uid, m_euid, m_gid, m_egid, path, main_program_fd);
+    auto auxv = generate_auxiliary_vector(load_result.load_base, load_result.entry_eip, uid(), euid(), gid(), egid(), path, main_program_fd);
 
     // NOTE: We create the new stack before disabling interrupts since it will zero-fault
     //       and we don't want to deal with faults after this point.
@@ -600,7 +604,7 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
     new_main_thread->set_name(m_name);
 
     // FIXME: PID/TID ISSUE
-    m_pid = new_main_thread->tid().value();
+    MutableProtectedData(*this)->pid = new_main_thread->tid().value();
     auto tsr_result = new_main_thread->make_thread_specific_region({});
     if (tsr_result.is_error()) {
         // FIXME: We cannot fail this late. Refactor this so the allocation happens before we commit to the new executable.
@@ -618,7 +622,7 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
     tss.eip = load_result.entry_eip;
     tss.esp = new_userspace_esp;
     tss.cr3 = space().page_directory().cr3();
-    tss.ss2 = m_pid.value();
+    tss.ss2 = pid().value();
 
     // Throw away any recorded performance events in this process.
     if (m_perf_event_buffer)

+ 6 - 2
Kernel/Syscalls/fork.cpp

@@ -36,7 +36,7 @@ KResultOr<pid_t> Process::sys$fork(RegisterState& regs)
 {
     REQUIRE_PROMISE(proc);
     RefPtr<Thread> child_first_thread;
-    auto child = adopt(*new Process(child_first_thread, m_name, m_uid, m_gid, m_pid, m_is_kernel_process, m_cwd, m_executable, m_tty, this));
+    auto child = adopt(*new Process(child_first_thread, m_name, uid(), gid(), pid(), m_is_kernel_process, m_cwd, m_executable, m_tty, this));
     if (!child_first_thread)
         return ENOMEM;
     child->m_root_directory = m_root_directory;
@@ -48,12 +48,16 @@ KResultOr<pid_t> Process::sys$fork(RegisterState& regs)
     child->m_veil_state = m_veil_state;
     child->m_unveiled_paths = m_unveiled_paths.deep_copy();
     child->m_fds = m_fds;
-    child->m_sid = m_sid;
     child->m_pg = m_pg;
     child->m_umask = m_umask;
     child->m_extra_gids = m_extra_gids;
     child->m_signal_trampoline = m_signal_trampoline;
 
+    {
+        MutableProtectedData child_data { *child };
+        child_data->sid = this->sid();
+    }
+
     dbgln_if(FORK_DEBUG, "fork: child={}", child);
     child->space().set_enforces_syscall_regions(space().enforces_syscall_regions());
 

+ 7 - 6
Kernel/Syscalls/getuid.cpp

@@ -31,31 +31,31 @@ namespace Kernel {
 KResultOr<uid_t> Process::sys$getuid()
 {
     REQUIRE_PROMISE(stdio);
-    return m_uid;
+    return uid();
 }
 
 KResultOr<gid_t> Process::sys$getgid()
 {
     REQUIRE_PROMISE(stdio);
-    return m_gid;
+    return gid();
 }
 
 KResultOr<uid_t> Process::sys$geteuid()
 {
     REQUIRE_PROMISE(stdio);
-    return m_euid;
+    return euid();
 }
 
 KResultOr<gid_t> Process::sys$getegid()
 {
     REQUIRE_PROMISE(stdio);
-    return m_egid;
+    return egid();
 }
 
 KResultOr<int> Process::sys$getresuid(Userspace<uid_t*> ruid, Userspace<uid_t*> euid, Userspace<uid_t*> suid)
 {
     REQUIRE_PROMISE(stdio);
-    if (!copy_to_user(ruid, &m_uid) || !copy_to_user(euid, &m_euid) || !copy_to_user(suid, &m_suid))
+    if (!copy_to_user(ruid, &protected_data().uid) || !copy_to_user(euid, &protected_data().euid) || !copy_to_user(suid, &protected_data().suid))
         return EFAULT;
     return 0;
 }
@@ -63,7 +63,7 @@ KResultOr<int> Process::sys$getresuid(Userspace<uid_t*> ruid, Userspace<uid_t*>
 KResultOr<int> Process::sys$getresgid(Userspace<gid_t*> rgid, Userspace<gid_t*> egid, Userspace<gid_t*> sgid)
 {
     REQUIRE_PROMISE(stdio);
-    if (!copy_to_user(rgid, &m_gid) || !copy_to_user(egid, &m_egid) || !copy_to_user(sgid, &m_sgid))
+    if (!copy_to_user(rgid, &protected_data().gid) || !copy_to_user(egid, &protected_data().egid) || !copy_to_user(sgid, &protected_data().sgid))
         return EFAULT;
     return 0;
 }
@@ -80,6 +80,7 @@ KResultOr<int> Process::sys$getgroups(ssize_t count, Userspace<gid_t*> user_gids
 
     if (!copy_to_user(user_gids, m_extra_gids.data(), sizeof(gid_t) * count))
         return EFAULT;
+
     return 0;
 }
 

+ 4 - 4
Kernel/Syscalls/kill.cpp

@@ -32,7 +32,7 @@ KResult Process::do_kill(Process& process, int signal)
 {
     // FIXME: Allow sending SIGCONT to everyone in the process group.
     // FIXME: Should setuid processes have some special treatment here?
-    if (!is_superuser() && m_euid != process.m_uid && m_uid != process.m_uid)
+    if (!is_superuser() && euid() != process.uid() && uid() != process.uid())
         return EPERM;
     if (process.is_kernel_process() && signal == SIGKILL) {
         klog() << "attempted to send SIGKILL to kernel process " << process.name().characters() << "(" << process.pid().value() << ")";
@@ -89,7 +89,7 @@ KResult Process::do_killall(int signal)
     ScopedSpinLock lock(g_processes_lock);
     for (auto& process : *g_processes) {
         KResult res = KSuccess;
-        if (process.pid() == m_pid)
+        if (process.pid() == pid())
             res = do_killself(signal);
         else
             res = do_kill(process, signal);
@@ -119,7 +119,7 @@ KResult Process::do_killself(int signal)
 
 KResultOr<int> Process::sys$kill(pid_t pid_or_pgid, int signal)
 {
-    if (pid_or_pgid == m_pid.value())
+    if (pid_or_pgid == pid().value())
         REQUIRE_PROMISE(stdio);
     else
         REQUIRE_PROMISE(proc);
@@ -133,7 +133,7 @@ KResultOr<int> Process::sys$kill(pid_t pid_or_pgid, int signal)
     }
     if (pid_or_pgid == -1)
         return do_killall(signal);
-    if (pid_or_pgid == m_pid.value()) {
+    if (pid_or_pgid == pid().value()) {
         return do_killself(signal);
     }
     VERIFY(pid_or_pgid >= 0);

+ 1 - 1
Kernel/Syscalls/pipe.cpp

@@ -40,7 +40,7 @@ KResultOr<int> Process::sys$pipe(int pipefd[2], int flags)
         return EINVAL;
 
     u32 fd_flags = (flags & O_CLOEXEC) ? FD_CLOEXEC : 0;
-    auto fifo = FIFO::create(m_uid);
+    auto fifo = FIFO::create(uid());
 
     auto open_reader_result = fifo->open_direction(FIFO::Direction::Reader);
     if (open_reader_result.is_error())

+ 1 - 1
Kernel/Syscalls/process.cpp

@@ -32,7 +32,7 @@ namespace Kernel {
 KResultOr<pid_t> Process::sys$getpid()
 {
     REQUIRE_PROMISE(stdio);
-    return m_pid.value();
+    return pid().value();
 }
 
 KResultOr<pid_t> Process::sys$getppid()

+ 2 - 2
Kernel/Syscalls/profiling.cpp

@@ -57,7 +57,7 @@ KResultOr<int> Process::sys$profiling_enable(pid_t pid)
         return ESRCH;
     if (process->is_dead())
         return ESRCH;
-    if (!is_superuser() && process->uid() != m_euid)
+    if (!is_superuser() && process->uid() != euid())
         return EPERM;
     if (!process->create_perf_events_buffer_if_needed())
         return ENOMEM;
@@ -79,7 +79,7 @@ KResultOr<int> Process::sys$profiling_disable(pid_t pid)
     auto process = Process::from_pid(pid);
     if (!process)
         return ESRCH;
-    if (!is_superuser() && process->uid() != m_euid)
+    if (!is_superuser() && process->uid() != euid())
         return EPERM;
     if (!process->is_profiling())
         return EINVAL;

+ 2 - 2
Kernel/Syscalls/sched.cpp

@@ -67,7 +67,7 @@ KResultOr<int> Process::sys$sched_setparam(int pid, Userspace<const struct sched
     if (!peer)
         return ESRCH;
 
-    if (!is_superuser() && m_euid != peer->process().m_uid && m_uid != peer->process().m_uid)
+    if (!is_superuser() && euid() != peer->process().uid() && uid() != peer->process().uid())
         return EPERM;
 
     peer->set_priority((u32)desired_param.sched_priority);
@@ -90,7 +90,7 @@ KResultOr<int> Process::sys$sched_getparam(pid_t pid, Userspace<struct sched_par
         if (!peer)
             return ESRCH;
 
-        if (!is_superuser() && m_euid != peer->process().m_uid && m_uid != peer->process().m_uid)
+        if (!is_superuser() && euid() != peer->process().uid() && uid() != peer->process().uid())
             return EPERM;
 
         priority = (int)peer->priority();

+ 11 - 11
Kernel/Syscalls/setpgid.cpp

@@ -33,14 +33,14 @@ KResultOr<pid_t> Process::sys$getsid(pid_t pid)
 {
     REQUIRE_PROMISE(proc);
     if (pid == 0)
-        return m_sid.value();
+        return sid().value();
     ScopedSpinLock lock(g_processes_lock);
     auto process = Process::from_pid(pid);
     if (!process)
         return ESRCH;
-    if (m_sid != process->m_sid)
+    if (sid() != process->sid())
         return EPERM;
-    return process->m_sid.value();
+    return process->sid().value();
 }
 
 KResultOr<pid_t> Process::sys$setsid()
@@ -55,10 +55,10 @@ KResultOr<pid_t> Process::sys$setsid()
     if (found_process_with_same_pgid_as_my_pid)
         return EPERM;
     // Create a new Session and a new ProcessGroup.
-    m_sid = m_pid.value();
-    m_pg = ProcessGroup::create(ProcessGroupID(m_pid.value()));
+    MutableProtectedData(*this)->sid = pid().value();
+    m_pg = ProcessGroup::create(ProcessGroupID(pid().value()));
     m_tty = nullptr;
-    return m_sid.value();
+    return sid().value();
 }
 
 KResultOr<pid_t> Process::sys$getpgid(pid_t pid)
@@ -97,7 +97,7 @@ KResultOr<int> Process::sys$setpgid(pid_t specified_pid, pid_t specified_pgid)
 {
     REQUIRE_PROMISE(proc);
     ScopedSpinLock lock(g_processes_lock); // FIXME: Use a ProcessHandle
-    ProcessID pid = specified_pid ? ProcessID(specified_pid) : m_pid;
+    ProcessID pid = specified_pid ? ProcessID(specified_pid) : this->pid();
     if (specified_pgid < 0) {
         // The value of the pgid argument is less than 0, or is not a value supported by the implementation.
         return EINVAL;
@@ -105,7 +105,7 @@ KResultOr<int> Process::sys$setpgid(pid_t specified_pid, pid_t specified_pgid)
     auto process = Process::from_pid(pid);
     if (!process)
         return ESRCH;
-    if (process != this && process->ppid() != m_pid) {
+    if (process != this && process->ppid() != this->pid()) {
         // The value of the pid argument does not match the process ID
         // of the calling process or of a child process of the calling process.
         return ESRCH;
@@ -114,21 +114,21 @@ KResultOr<int> Process::sys$setpgid(pid_t specified_pid, pid_t specified_pgid)
         // The process indicated by the pid argument is a session leader.
         return EPERM;
     }
-    if (process->ppid() == m_pid && process->sid() != sid()) {
+    if (process->ppid() == this->pid() && process->sid() != sid()) {
         // The value of the pid argument matches the process ID of a child
         // process of the calling process and the child process is not in
         // the same session as the calling process.
         return EPERM;
     }
 
-    ProcessGroupID new_pgid = specified_pgid ? ProcessGroupID(specified_pgid) : process->m_pid.value();
+    ProcessGroupID new_pgid = specified_pgid ? ProcessGroupID(specified_pgid) : process->pid().value();
     SessionID current_sid = sid();
     SessionID new_sid = get_sid_from_pgid(new_pgid);
     if (new_sid != -1 && current_sid != new_sid) {
         // Can't move a process between sessions.
         return EPERM;
     }
-    if (new_sid == -1 && new_pgid != process->m_pid.value()) {
+    if (new_sid == -1 && new_pgid != process->pid().value()) {
         // The value of the pgid argument is valid, but is not
         // the calling pid, and is not an existing process group.
         return EPERM;

+ 59 - 55
Kernel/Syscalls/setuid.cpp

@@ -28,110 +28,114 @@
 
 namespace Kernel {
 
-KResultOr<int> Process::sys$seteuid(uid_t euid)
+KResultOr<int> Process::sys$seteuid(uid_t new_euid)
 {
     REQUIRE_PROMISE(id);
 
-    if (euid != m_uid && euid != m_suid && !is_superuser())
+    if (new_euid != uid() && new_euid != suid() && !is_superuser())
         return EPERM;
 
-    if (m_euid != euid)
+    if (euid() != new_euid)
         set_dumpable(false);
-    m_euid = euid;
+    MutableProtectedData(*this)->euid = new_euid;
     return 0;
 }
 
-KResultOr<int> Process::sys$setegid(gid_t egid)
+KResultOr<int> Process::sys$setegid(gid_t new_egid)
 {
     REQUIRE_PROMISE(id);
 
-    if (egid != m_gid && egid != m_sgid && !is_superuser())
+    if (new_egid != gid() && new_egid != sgid() && !is_superuser())
         return EPERM;
 
-    if (m_egid != egid)
+    if (egid() != new_egid)
         set_dumpable(false);
 
-    m_egid = egid;
+    MutableProtectedData(*this)->egid = new_egid;
     return 0;
 }
 
-KResultOr<int> Process::sys$setuid(uid_t uid)
+KResultOr<int> Process::sys$setuid(uid_t new_uid)
 {
     REQUIRE_PROMISE(id);
 
-    if (uid != m_uid && uid != m_euid && !is_superuser())
+    if (new_uid != uid() && new_uid != euid() && !is_superuser())
         return EPERM;
 
-    if (m_euid != uid)
+    if (euid() != new_uid)
         set_dumpable(false);
 
-    m_uid = uid;
-    m_euid = uid;
-    m_suid = uid;
+    MutableProtectedData protected_data { *this };
+    protected_data->uid = new_uid;
+    protected_data->euid = new_uid;
+    protected_data->suid = new_uid;
     return 0;
 }
 
-KResultOr<int> Process::sys$setgid(gid_t gid)
+KResultOr<int> Process::sys$setgid(gid_t new_gid)
 {
     REQUIRE_PROMISE(id);
 
-    if (gid != m_gid && gid != m_egid && !is_superuser())
+    if (new_gid != gid() && new_gid != egid() && !is_superuser())
         return EPERM;
 
-    if (m_egid != gid)
+    if (egid() != new_gid)
         set_dumpable(false);
 
-    m_gid = gid;
-    m_egid = gid;
-    m_sgid = gid;
+    MutableProtectedData protected_data { *this };
+    protected_data->gid = new_gid;
+    protected_data->egid = new_gid;
+    protected_data->sgid = new_gid;
     return 0;
 }
 
-KResultOr<int> Process::sys$setresuid(uid_t ruid, uid_t euid, uid_t suid)
+KResultOr<int> Process::sys$setresuid(uid_t new_ruid, uid_t new_euid, uid_t new_suid)
 {
     REQUIRE_PROMISE(id);
 
-    if (ruid == (uid_t)-1)
-        ruid = m_uid;
-    if (euid == (uid_t)-1)
-        euid = m_euid;
-    if (suid == (uid_t)-1)
-        suid = m_suid;
+    if (new_ruid == (uid_t)-1)
+        new_ruid = uid();
+    if (new_euid == (uid_t)-1)
+        new_euid = euid();
+    if (new_suid == (uid_t)-1)
+        new_suid = suid();
 
-    auto ok = [this](uid_t id) { return id == m_uid || id == m_euid || id == m_suid; };
-    if ((!ok(ruid) || !ok(euid) || !ok(suid)) && !is_superuser())
+    auto ok = [this](uid_t id) { return id == uid() || id == euid() || id == suid(); };
+    if ((!ok(new_ruid) || !ok(new_euid) || !ok(new_suid)) && !is_superuser())
         return EPERM;
 
-    if (m_euid != euid)
+    if (euid() != new_euid)
         set_dumpable(false);
 
-    m_uid = ruid;
-    m_euid = euid;
-    m_suid = suid;
+    MutableProtectedData protected_data { *this };
+    protected_data->uid = new_ruid;
+    protected_data->euid = new_euid;
+    protected_data->suid = new_suid;
     return 0;
 }
 
-KResultOr<int> Process::sys$setresgid(gid_t rgid, gid_t egid, gid_t sgid)
+KResultOr<int> Process::sys$setresgid(gid_t new_rgid, gid_t new_egid, gid_t new_sgid)
 {
     REQUIRE_PROMISE(id);
 
-    if (rgid == (gid_t)-1)
-        rgid = m_gid;
-    if (egid == (gid_t)-1)
-        egid = m_egid;
-    if (sgid == (gid_t)-1)
-        sgid = m_sgid;
+    if (new_rgid == (gid_t)-1)
+        new_rgid = gid();
+    if (new_egid == (gid_t)-1)
+        new_egid = egid();
+    if (new_sgid == (gid_t)-1)
+        new_sgid = sgid();
 
-    auto ok = [this](gid_t id) { return id == m_gid || id == m_egid || id == m_sgid; };
-    if ((!ok(rgid) || !ok(egid) || !ok(sgid)) && !is_superuser())
+    auto ok = [this](gid_t id) { return id == gid() || id == egid() || id == sgid(); };
+    if ((!ok(new_rgid) || !ok(new_egid) || !ok(new_sgid)) && !is_superuser())
         return EPERM;
 
-    if (m_egid != egid)
+    if (egid() != new_egid)
         set_dumpable(false);
 
-    m_gid = rgid;
-    m_egid = egid;
-    m_sgid = sgid;
+    MutableProtectedData protected_data { *this };
+    protected_data->gid = new_rgid;
+    protected_data->egid = new_egid;
+    protected_data->sgid = new_sgid;
     return 0;
 }
 
@@ -148,23 +152,23 @@ KResultOr<int> Process::sys$setgroups(ssize_t count, Userspace<const gid_t*> use
         return 0;
     }
 
-    Vector<gid_t> gids;
-    gids.resize(count);
-    if (!copy_n_from_user(gids.data(), user_gids, count))
+    Vector<gid_t> new_extra_gids;
+    new_extra_gids.resize(count);
+    if (!copy_n_from_user(new_extra_gids.data(), user_gids, count))
         return EFAULT;
 
     HashTable<gid_t> unique_extra_gids;
-    for (auto& gid : gids) {
-        if (gid != m_gid)
-            unique_extra_gids.set(gid);
+    for (auto& extra_gid : new_extra_gids) {
+        if (extra_gid != gid())
+            unique_extra_gids.set(extra_gid);
     }
 
     m_extra_gids.resize(unique_extra_gids.size());
     size_t i = 0;
-    for (auto& gid : unique_extra_gids) {
-        if (gid == m_gid)
+    for (auto& extra_gid : unique_extra_gids) {
+        if (extra_gid == gid())
             continue;
-        m_extra_gids[i++] = gid;
+        m_extra_gids[i++] = extra_gid;
     }
     return 0;
 }