From cf8dd312abcc31d5cd61b80c93775c169a2233b5 Mon Sep 17 00:00:00 2001
From: Andreas Kling <awesomekling@gmail.com>
Date: Sun, 17 Feb 2019 10:18:25 +0100
Subject: [PATCH] Kernel: Fix String leaks in exec().

When the kernel performs a successful exec(), whatever was on the kernel
stack for that process before goes away. For this reason, we need to make
sure we don't have any stack objects holding onto kmalloc memory.
---
 Kernel/Process.cpp | 37 ++++++++++++++++++++-----------------
 Kernel/Process.h   |  4 ++--
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index 69c407289e..a9e7a28138 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -282,7 +282,7 @@ pid_t Process::sys$fork(RegisterDump& regs)
     return child->pid();
 }
 
-int Process::do_exec(const String& path, Vector<String>&& arguments, Vector<String>&& environment)
+int Process::do_exec(String path, Vector<String> arguments, Vector<String> environment)
 {
     ASSERT(is_ring3());
 
@@ -419,11 +419,11 @@ int Process::do_exec(const String& path, Vector<String>&& arguments, Vector<Stri
     return 0;
 }
 
-int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>&& environment)
+int Process::exec(String path, Vector<String> arguments, Vector<String> environment)
 {
     // The bulk of exec() is done by do_exec(), which ensures that all locals
     // are cleaned up by the time we yield-teleport below.
-    int rc = do_exec(path, move(arguments), move(environment));
+    int rc = do_exec(move(path), move(arguments), move(environment));
     if (rc < 0)
         return rc;
 
@@ -436,6 +436,8 @@ int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>
 
 int Process::sys$execve(const char* filename, const char** argv, const char** envp)
 {
+    // NOTE: Be extremely careful with allocating any kernel memory in exec().
+    //       On success, the kernel stack will be lost.
     if (!validate_read_str(filename))
         return -EFAULT;
     if (argv) {
@@ -456,24 +458,25 @@ int Process::sys$execve(const char* filename, const char** argv, const char** en
     }
 
     String path(filename);
-    auto parts = path.split('/');
-
     Vector<String> arguments;
-    if (argv) {
-        for (size_t i = 0; argv[i]; ++i) {
-            arguments.append(argv[i]);
-        }
-    } else {
-        arguments.append(parts.last());
-    }
-
     Vector<String> environment;
-    if (envp) {
-        for (size_t i = 0; envp[i]; ++i)
-            environment.append(envp[i]);
+    {
+        auto parts = path.split('/');
+        if (argv) {
+            for (size_t i = 0; argv[i]; ++i) {
+                arguments.append(argv[i]);
+            }
+        } else {
+            arguments.append(parts.last());
+        }
+
+        if (envp) {
+            for (size_t i = 0; envp[i]; ++i)
+                environment.append(envp[i]);
+        }
     }
 
-    int rc = exec(path, move(arguments), move(environment));
+    int rc = exec(move(path), move(arguments), move(environment));
     ASSERT(rc < 0); // We should never continue after a successful exec!
     return rc;
 }
diff --git a/Kernel/Process.h b/Kernel/Process.h
index bbad42d35e..b44fda0dce 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -279,7 +279,7 @@ public:
     size_t amount_shared() const;
 
     Process* fork(RegisterDump&);
-    int exec(const String& path, Vector<String>&& arguments, Vector<String>&& environment);
+    int exec(String path, Vector<String> arguments, Vector<String> environment);
 
     bool is_root() const { return m_euid == 0; }
 
@@ -302,7 +302,7 @@ private:
 
     Process(String&& name, uid_t, gid_t, pid_t ppid, RingLevel, RetainPtr<Inode>&& cwd = nullptr, RetainPtr<Inode>&& executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr);
 
-    int do_exec(const String& path, Vector<String>&& arguments, Vector<String>&& environment);
+    int do_exec(String path, Vector<String> arguments, Vector<String> environment);
     void push_value_on_stack(dword);
 
     int alloc_fd();