Kernel: Rework the uid/gid system to make it compliant

apps/src/session.c

@@ -80,8 +80,8 @@ static void login_as(struct passwd* user)
     }
     if (child == 0)
     {
-        setuid(user->pw_uid);
         setgid(user->pw_gid);
+        setuid(user->pw_uid);
         char* argv[] = {user->pw_shell, NULL};
         execv(argv[0], argv);
         perror("execv");

apps/src/su.c

@@ -40,7 +40,7 @@ int main(int argc, char** argv)
     else
         username = argv[1];
 
-    if (getuid() != 0)
+    if (geteuid() != 0)
     {
         fprintf(stderr, "%s must be setuid root", argv[0]);
         return EXIT_FAILURE;
@@ -68,18 +68,18 @@ int main(int argc, char** argv)
         }
     }
 
-    if (setuid(user->pw_uid) < 0)
-    {
-        perror("setuid");
-        return EXIT_FAILURE;
-    }
-
     if (setgid(user->pw_gid) < 0)
     {
         perror("setgid");
         return EXIT_FAILURE;
     }
 
+    if (setuid(user->pw_uid) < 0)
+    {
+        perror("setuid");
+        return EXIT_FAILURE;
+    }
+
     char* default_argv[] = {user->pw_shell, NULL};
 
     if (argc < 3) run_program(default_argv);

kernel/include/sys/Syscall.h

@@ -31,6 +31,8 @@
 #define SYS_setgid 25
 #define SYS_umask 26
 #define SYS_ioctl 27
+#define SYS_seteuid 28
+#define SYS_setegid 29
 
 struct stat;
 struct pstat;
@@ -66,7 +68,9 @@ void sys_pstat(Context* context, long pid, struct pstat* buf);
 void sys_getdents(Context* context, int fd, struct luna_dirent* buf, size_t count);
 void sys_stat(Context* context, const char* path, struct stat* buf);
 void sys_dup2(Context* context, int fd, int fd2);
-void sys_setuid(Context* context, int new_uid, int new_euid);
+void sys_setuid(Context* context, uid_t uid);
-void sys_setgid(Context* context, int new_gid, int new_egid);
+void sys_setgid(Context* context, gid_t gid);
 void sys_umask(Context* context, mode_t cmask);
 void sys_ioctl(Context* context, int fd, int request, uintptr_t arg);
+void sys_seteuid(Context* context, uid_t euid);
+void sys_setegid(Context* context, gid_t egid);

kernel/include/thread/Task.h

@@ -4,6 +4,7 @@
 #include "memory/AddressSpace.h"
 #include "memory/UserHeap.h"
 #include "sys/elf/Image.h"
+#include <sys/types.h>
 
 #define TASK_MAX_FDS 32
 
@@ -14,6 +15,9 @@ enum class BlockReason
     Waiting,
 };
 
+// FIXME: To make this struct more C++-styled, maybe we could make a lot of these variables private and add
+// getters/setters?
+
 struct Task
 {
     enum TaskState
@@ -36,10 +40,10 @@ struct Task
 
     int64_t task_time = 0;
 
-    int uid;
+    uid_t uid;
-    int euid;
+    uid_t euid;
-    int gid;
+    gid_t gid;
-    int egid;
+    gid_t egid;
 
     Task* next_task = nullptr;
     Task* prev_task = nullptr;
@@ -57,7 +61,8 @@ struct Task
 
     bool is_user_task();
 
-    ELFImage* image = nullptr;
+    ELFImage* image = nullptr; // FIXME: we probably don't need to keep track of this anymore since the ELF sections are
+                               // freed automatically when calling destroy() or clear() on the address space.
 
     Descriptor files[TASK_MAX_FDS];
 
@@ -103,6 +108,7 @@ struct Task
 
     bool is_still_blocking();
 
+    // FIXME: These two functions are a bit clunky.
     Descriptor* open_descriptor_from_fd(int fd, int& error);
     Descriptor* descriptor_from_fd(int fd, int& error);
 

kernel/src/sys/Syscall.cpp

@@ -39,10 +39,12 @@ void Syscall::entry(Context* context)
         sys_getdents(context, (int)context->rdi, (struct luna_dirent*)context->rsi, (size_t)context->rdx);
         break;
     case SYS_dup2: sys_dup2(context, (int)context->rdi, (int)context->rsi); break;
-    case SYS_setuid: sys_setuid(context, (int)context->rdi, (int)context->rsi); break;
+    case SYS_setuid: sys_setuid(context, (uid_t)context->rdi); break;
-    case SYS_setgid: sys_setgid(context, (int)context->rdi, (int)context->rsi); break;
+    case SYS_setgid: sys_setgid(context, (gid_t)context->rdi); break;
     case SYS_umask: sys_umask(context, (mode_t)context->rdi); break;
     case SYS_ioctl: sys_ioctl(context, (int)context->rdi, (int)context->rsi, (uintptr_t)context->rdx); break;
+    case SYS_seteuid: sys_seteuid(context, (uid_t)context->rdi); break;
+    case SYS_setegid: sys_setegid(context, (gid_t)context->rdi); break;
     default: context->rax = -ENOSYS; break;
     }
     VMM::exit_syscall_context();

kernel/src/sys/exec.cpp

@@ -121,7 +121,7 @@ void sys_execv(Context* context, const char* pathname, char** argv)
 
     uint64_t kargc = 0;
     char** kargv = nullptr;
-    char** arg;
+    char* arg;
 
     auto free_kernel_argv_copy = [&]() {
         for (uint64_t i = 0; i < kargc; i++)
@@ -131,36 +131,27 @@ void sys_execv(Context* context, const char* pathname, char** argv)
         if (kargv) kfree(kargv);
     };
 
-    // FIXME: This is a bit messy.
+    // FIXME: This code is a bit messy. Should probably be refactored and moved into a separate function.
 
     do {
-        if (!validate_user_read((uintptr_t)argv, sizeof(char*)))
+        if (!copy_from_user(argv, &arg, sizeof(char*)))
         {
             free_kernel_argv_copy();
             context->rax = -EFAULT;
             return;
         }
-        arg = obtain_user_ref(argv);
+        kargv = (char**)krealloc(kargv, (kargc + 1) * sizeof(char*)); // we need a vector class for the kernel.
-        if (!arg) // This should never happen, since it was already checked just before.
-        {
-            free_kernel_argv_copy();
-            context->rax = -EFAULT;
-            return;
-        }
-        kargv = (char**)krealloc(kargv, (kargc + 1) * sizeof(char*));
         if (!kargv)
         {
-            release_user_ref(arg);
             free_kernel_argv_copy();
             context->rax = -ENOMEM;
             return;
         }
-        if (*arg)
+        if (arg)
         {
-            char* kcopy = strdup_from_user(*arg);
+            char* kcopy = strdup_from_user(arg);
-            if (!kcopy)
+            if (!kcopy) // FIXME: This could also be EFAULT.
             {
-                release_user_ref(arg);
                 free_kernel_argv_copy();
                 context->rax = -ENOMEM;
                 return;
@@ -172,7 +163,6 @@ void sys_execv(Context* context, const char* pathname, char** argv)
             kargv[kargc] = nullptr;
             break;
         }
-        release_user_ref(arg);
         kargc++;
         argv++;
     } while (arg != nullptr);
@@ -228,8 +218,8 @@ void sys_execv(Context* context, const char* pathname, char** argv)
     ensure(image); // If check_elf_image succeeded, load_elf_from_vfs MUST succeed, unless something has gone terribly
                    // wrong.
 
-    if (VFS::is_setuid(program)) task->uid = program->uid;
+    if (VFS::is_setuid(program)) task->euid = program->uid;
-    if (VFS::is_setgid(program)) task->gid = program->gid;
+    if (VFS::is_setgid(program)) task->egid = program->gid;
 
     strlcpy(task->name, kpathname, sizeof(task->name));
 

kernel/src/sys/id.cpp

@@ -1,5 +1,6 @@
 #include "std/errno.h"
 #include "thread/Scheduler.h"
+#include <sys/types.h>
 
 #define ID_PID 0
 #define ID_PPID 1
@@ -47,50 +48,76 @@ void sys_getprocid(Context* context, int field)
     }
 }
 
-void sys_setuid(Context* context, int new_uid, int new_euid)
+void sys_setuid(Context* context, uid_t uid)
 {
     Task* current_task = Scheduler::current_task();
 
     if (!current_task->is_superuser())
     {
-        if (new_uid != current_task->uid && new_uid != current_task->euid)
+        if (uid != current_task->uid && uid != current_task->euid)
-        {
-            context->rax = -EPERM;
-            return;
-        }
-        if (new_euid != current_task->euid && new_euid != current_task->uid)
         {
             context->rax = -EPERM;
             return;
         }
     }
 
-    current_task->uid = new_uid;
+    current_task->uid = uid;
-    current_task->euid = new_euid;
+    current_task->euid = uid;
 
     context->rax = 0;
 }
 
-void sys_setgid(Context* context, int new_gid, int new_egid)
+void sys_seteuid(Context* context, uid_t euid)
 {
     Task* current_task = Scheduler::current_task();
 
     if (!current_task->is_superuser())
     {
-        if (new_gid != current_task->gid && new_gid != current_task->egid)
+        if (euid != current_task->uid)
-        {
-            context->rax = -EPERM;
-            return;
-        }
-        if (new_egid != current_task->egid && new_egid != current_task->gid)
         {
             context->rax = -EPERM;
             return;
         }
     }
 
-    current_task->gid = new_gid;
+    current_task->euid = euid;
-    current_task->egid = new_egid;
+
+    context->rax = 0;
+}
+
+void sys_setgid(Context* context, gid_t gid)
+{
+    Task* current_task = Scheduler::current_task();
+
+    if (!current_task->is_superuser())
+    {
+        if (gid != current_task->gid && gid != current_task->egid)
+        {
+            context->rax = -EPERM;
+            return;
+        }
+    }
+
+    current_task->gid = gid;
+    current_task->egid = gid;
+
+    context->rax = 0;
+}
+
+void sys_setegid(Context* context, gid_t egid)
+{
+    Task* current_task = Scheduler::current_task();
+
+    if (!current_task->is_superuser())
+    {
+        if (egid != current_task->gid)
+        {
+            context->rax = -EPERM;
+            return;
+        }
+    }
+
+    current_task->egid = egid;
 
     context->rax = 0;
 }

kernel/src/thread/Task.cpp

@@ -136,5 +136,5 @@ Descriptor* Task::descriptor_from_fd(int fd, int& error)
 
 bool Task::is_superuser()
 {
-    return euid == 0 || egid == 0;
+    return euid == 0;
 }

libs/libc/include/sys/syscall.h

@@ -29,5 +29,7 @@
 #define SYS_setgid 25
 #define SYS_umask 26
 #define SYS_ioctl 27
+#define SYS_seteuid 28
+#define SYS_setegid 29
 
 #endif

libs/libc/include/unistd.h

@@ -54,13 +54,13 @@ extern "C"
     int setuid(uid_t uid);
 
     /* Sets the current process' effective user ID. */
-    int seteuid(uid_t uid);
+    int seteuid(uid_t euid);
 
     /* Sets the current process' real and effective group IDs. */
     int setgid(gid_t gid);
 
     /* Sets the current process' effective group ID. */
-    int setegid(gid_t gid);
+    int setegid(gid_t egid);
 
     /* Terminates the program with the status code status. */
     __lc_noreturn void _exit(int status);

libs/libc/src/syscall.cpp

@@ -18,6 +18,10 @@ extern "C" long syscall(long number, ...)
     case SYS_getprocid:
     case SYS_close:
     case SYS_umask:
+    case SYS_setuid:
+    case SYS_setgid:
+    case SYS_seteuid:
+    case SYS_setegid:
     case SYS_sleep: result = __luna_syscall1(number, va_arg(ap, arg)); break;
     case SYS_munmap:
     case SYS_execv:
@@ -27,8 +31,6 @@ extern "C" long syscall(long number, ...)
     case SYS_mkdir:
     case SYS_dup2:
     case SYS_clock_gettime:
-    case SYS_setuid:
-    case SYS_setgid:
     case SYS_pstat: {
         arg arg0 = va_arg(ap, arg);
         arg arg1 = va_arg(ap, arg);

libs/libc/src/unistd.cpp

@@ -60,22 +60,22 @@ extern "C"
 
     int setuid(uid_t uid)
     {
-        return (int)__lc_fast_syscall2(SYS_setuid, uid, uid);
+        return (int)__lc_fast_syscall1(SYS_setuid, uid);
     }
 
-    int seteuid(uid_t uid)
+    int seteuid(uid_t euid)
     {
-        return (int)__lc_fast_syscall2(SYS_setuid, getprocid(ID_UID), uid);
+        return (int)__lc_fast_syscall1(SYS_seteuid, euid);
     }
 
     int setgid(gid_t gid)
     {
-        return (int)__lc_fast_syscall2(SYS_setgid, gid, gid);
+        return (int)__lc_fast_syscall1(SYS_setgid, gid);
     }
 
-    int setegid(gid_t gid)
+    int setegid(gid_t egid)
     {
-        return (int)__lc_fast_syscall2(SYS_setgid, getprocid(ID_GID), gid);
+        return (int)__lc_fast_syscall1(SYS_setegid, egid);
     }
 
     unsigned int sleep(unsigned int seconds)

tools/generate-symbols.sh

@@ -6,6 +6,8 @@ source $(dirname $0)/env.sh
 
 cd $LUNA_ROOT
 
+rm -f initrd/sys/moon.sym
+
 nm -C -n initrd/boot/moon | grep -vE \\.Lubsan_data | awk '{ if ($2 != "a") print; }' | uniq > initrd/sys/moon.sym
 
 chmod 400 initrd/sys/moon.sym