From 80cbb72f2ff48b7eff98ee0a5d83326e5ef98a6e Mon Sep 17 00:00:00 2001
From: Andreas Kling <awesomekling@gmail.com>
Date: Sun, 5 Jan 2020 21:51:06 +0100
Subject: [PATCH] Kernel: Remove SmapDisablers in open(), openat() and
 set_thread_name()

This patch introduces a helpful copy_string_from_user() function
that takes a bounded null-terminated string from userspace memory
and copies it into a String object.
---
 Kernel/Process.cpp | 56 +++++++++++++++++++---------------------------
 Kernel/StdLib.cpp  |  8 +++++++
 Kernel/StdLib.h    |  6 +++++
 3 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index b63d0b5538c..dd0f9dbcd14 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -1600,34 +1600,22 @@ int Process::number_of_open_file_descriptors() const
     return count;
 }
 
-int Process::sys$open(const Syscall::SC_open_params* params)
+int Process::sys$open(const Syscall::SC_open_params* user_params)
 {
-    if (!validate_read_typed(params))
+    if (!validate_read_typed(user_params))
         return -EFAULT;
 
-    const char* path_data;
-    int path_length;
-    int options;
-    u16 mode;
+    Syscall::SC_open_params params;
+    copy_from_user(&params, user_params, sizeof(params));
+    auto options = params.options;
+    auto mode = params.mode;
 
-    {
-        SmapDisabler disabler;
-        path_data = params->path;
-        path_length = params->path_length;
-        options = params->options;
-        mode = params->mode;
-    }
-
-    if (!path_length)
+    if (params.path_length <= 0)
         return -EINVAL;
-    if (!validate_read(path_data, path_length))
+    if (!validate_read(params.path, params.path_length))
         return -EFAULT;
 
-    String path;
-    {
-        SmapDisabler disabler;
-        path = String(path_data, path_length);
-    }
+    String path = copy_string_from_user(params.path, params.path_length);
     int fd = alloc_fd();
 #ifdef DEBUG_IO
     dbgprintf("%s(%u) sys$open(\"%s\") -> %d\n", name().characters(), pid(), path, fd);
@@ -1645,20 +1633,22 @@ int Process::sys$open(const Syscall::SC_open_params* params)
     return fd;
 }
 
-int Process::sys$openat(const Syscall::SC_openat_params* params)
+int Process::sys$openat(const Syscall::SC_openat_params* user_params)
 {
-    if (!validate_read_typed(params))
+    if (!validate_read_typed(user_params))
         return -EFAULT;
 
-    SmapDisabler disabler;
-    int dirfd = params->dirfd;
-    const char* path = params->path;
-    int path_length = params->path_length;
-    int options = params->options;
-    u16 mode = params->mode;
+    Syscall::SC_openat_params params;
+    copy_from_user(&params, user_params, sizeof(params));
+    int dirfd = params.dirfd;
+    int options = params.options;
+    u16 mode = params.mode;
 
-    if (!validate_read(path, path_length))
+    if (params.path_length <= 0)
+        return -EINVAL;
+    if (!validate_read(params.path, params.path_length))
         return -EFAULT;
+    auto path = copy_string_from_user(params.path, params.path_length);
 #ifdef DEBUG_IO
     dbgprintf("%s(%u) sys$openat(%d, \"%s\")\n", dirfd, name().characters(), pid(), path);
 #endif
@@ -3377,17 +3367,17 @@ int Process::sys$set_thread_name(int tid, const char* buffer, int buffer_size)
     if (!validate_read(buffer, buffer_size))
         return -EFAULT;
 
-    SmapDisabler disabler;
+    auto name = copy_string_from_user(buffer, buffer_size);
 
     const size_t max_thread_name_size = 64;
-    if (strnlen(buffer, (size_t)buffer_size) > max_thread_name_size)
+    if (name.length() > max_thread_name_size)
         return -EINVAL;
 
     auto* thread = Thread::from_tid(tid);
     if (!thread || thread->pid() != pid())
         return -ESRCH;
 
-    thread->set_name({ buffer, (size_t)buffer_size });
+    thread->set_name(name);
     return 0;
 }
 int Process::sys$get_thread_name(int tid, char* buffer, int buffer_size)
diff --git a/Kernel/StdLib.cpp b/Kernel/StdLib.cpp
index 1221ad3064e..9054d163c37 100644
--- a/Kernel/StdLib.cpp
+++ b/Kernel/StdLib.cpp
@@ -1,9 +1,17 @@
 #include <AK/Assertions.h>
+#include <AK/String.h>
 #include <AK/Types.h>
 #include <Kernel/Arch/i386/CPU.h>
 #include <Kernel/Heap/kmalloc.h>
 #include <Kernel/StdLib.h>
 
+String copy_string_from_user(const char* user_str, size_t user_str_size)
+{
+    SmapDisabler disabler;
+    size_t length = strnlen(user_str, user_str_size);
+    return String(user_str, length);
+}
+
 extern "C" {
 
 void* copy_to_user(void* dest_ptr, const void* src_ptr, size_t n)
diff --git a/Kernel/StdLib.h b/Kernel/StdLib.h
index 67e0fb8f4a4..f721c008fe2 100644
--- a/Kernel/StdLib.h
+++ b/Kernel/StdLib.h
@@ -2,6 +2,12 @@
 
 #include <AK/Types.h>
 
+namespace AK {
+class String;
+}
+
+AK::String copy_string_from_user(const char*, size_t);
+
 extern "C" {
 
 static_assert(sizeof(size_t) == 4);