From e56daf547c120adfcdab1e1e8ea189a1f369bd0c Mon Sep 17 00:00:00 2001
From: Andreas Kling <awesomekling@gmail.com>
Date: Fri, 29 Nov 2019 16:15:30 +0100
Subject: [PATCH] Kernel: Disallow syscalls from writeable memory

Processes will now crash with SIGSEGV if they attempt making a syscall
from PROT_WRITE memory.

This neat idea comes from OpenBSD. :^)
---
 Base/usr/share/man/man1/crash.md |  1 +
 Kernel/Syscall.cpp               | 13 +++++++++++++
 Kernel/VM/MemoryManager.cpp      |  1 -
 Kernel/VM/MemoryManager.h        |  8 ++++----
 Userland/crash.cpp               |  9 +++++++++
 5 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/Base/usr/share/man/man1/crash.md b/Base/usr/share/man/man1/crash.md
index 78a8ba83a91..da38a466dd9 100644
--- a/Base/usr/share/man/man1/crash.md
+++ b/Base/usr/share/man/man1/crash.md
@@ -27,6 +27,7 @@ kinds of crashes.
 * `-r`: Write to read-only memory.
 * `-T`: Make a syscall while using an invalid stack pointer.
 * `-t`: Trigger a page fault while using an invalid stack pointer.
+* `-S`: Make a syscall from writeable memory.
 
 ## Examples
 
diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp
index 708bfc23b3f..1bd5e7e71cd 100644
--- a/Kernel/Syscall.cpp
+++ b/Kernel/Syscall.cpp
@@ -105,6 +105,19 @@ void syscall_trap_entry(RegisterDump regs)
         ASSERT_NOT_REACHED();
     }
 
+    auto* calling_region = MM.region_from_vaddr(process, VirtualAddress(regs.eip));
+    if (!calling_region) {
+        dbgprintf("Syscall from %p which has no region\n", regs.eip);
+        handle_crash(regs, "Syscall from unknown region", SIGSEGV);
+        ASSERT_NOT_REACHED();
+    }
+
+    if (calling_region->is_writable()) {
+        dbgprintf("Syscall from writable memory at %p\n", regs.eip);
+        handle_crash(regs, "Syscall from writable memory", SIGSEGV);
+        ASSERT_NOT_REACHED();
+    }
+
     process.big_lock().lock();
     u32 function = regs.eax;
     u32 arg1 = regs.edx;
diff --git a/Kernel/VM/MemoryManager.cpp b/Kernel/VM/MemoryManager.cpp
index 540081d26cb..5e5c5a9b884 100644
--- a/Kernel/VM/MemoryManager.cpp
+++ b/Kernel/VM/MemoryManager.cpp
@@ -289,7 +289,6 @@ Region* MemoryManager::user_region_from_vaddr(Process& process, VirtualAddress v
 
 Region* MemoryManager::region_from_vaddr(Process& process, VirtualAddress vaddr)
 {
-    ASSERT_INTERRUPTS_DISABLED();
     if (auto* region = kernel_region_from_vaddr(vaddr))
         return region;
     return user_region_from_vaddr(process, vaddr);
diff --git a/Kernel/VM/MemoryManager.h b/Kernel/VM/MemoryManager.h
index 619f95630c3..b4c7cac06d0 100644
--- a/Kernel/VM/MemoryManager.h
+++ b/Kernel/VM/MemoryManager.h
@@ -1,6 +1,5 @@
 #pragma once
 
-#include <AK/String.h>
 #include <AK/Badge.h>
 #include <AK/Bitmap.h>
 #include <AK/ByteBuffer.h>
@@ -8,6 +7,7 @@
 #include <AK/NonnullRefPtrVector.h>
 #include <AK/RefCounted.h>
 #include <AK/RefPtr.h>
+#include <AK/String.h>
 #include <AK/Types.h>
 #include <AK/Vector.h>
 #include <AK/Weakable.h>
@@ -79,6 +79,9 @@ public:
         }
     }
 
+    static Region* region_from_vaddr(Process&, VirtualAddress);
+    static const Region* region_from_vaddr(const Process&, VirtualAddress);
+
 private:
     MemoryManager(u32 physical_address_for_kernel_page_tables);
     ~MemoryManager();
@@ -96,9 +99,6 @@ private:
 
     void create_identity_mapping(PageDirectory&, VirtualAddress, size_t length);
 
-    static Region* region_from_vaddr(Process&, VirtualAddress);
-    static const Region* region_from_vaddr(const Process&, VirtualAddress);
-
     static Region* user_region_from_vaddr(Process&, VirtualAddress);
     static Region* kernel_region_from_vaddr(VirtualAddress);
 
diff --git a/Userland/crash.cpp b/Userland/crash.cpp
index fa6f6e9b3e1..7879cda8441 100644
--- a/Userland/crash.cpp
+++ b/Userland/crash.cpp
@@ -1,4 +1,5 @@
 #include <AK/String.h>
+#include <Kernel/Syscall.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/mman.h>
@@ -24,6 +25,7 @@ int main(int argc, char** argv)
         WriteToReadonlyMemory,
         InvalidStackPointerOnSyscall,
         InvalidStackPointerOnPageFault,
+        SyscallFromWritableMemory,
     };
     Mode mode = SegmentationViolation;
 
@@ -52,6 +54,8 @@ int main(int argc, char** argv)
         mode = InvalidStackPointerOnSyscall;
     else if (String(argv[1]) == "-t")
         mode = InvalidStackPointerOnPageFault;
+    else if (String(argv[1]) == "-S")
+        mode = SyscallFromWritableMemory;
     else
         print_usage_and_exit();
 
@@ -152,6 +156,11 @@ int main(int argc, char** argv)
         ASSERT_NOT_REACHED();
     }
 
+    if (mode == SyscallFromWritableMemory) {
+        u8 buffer[] = { 0xb8, Syscall::SC_getuid, 0, 0, 0, 0xcd, 0x82 };
+        ((void(*)())buffer)();
+    }
+
     ASSERT_NOT_REACHED();
     return 0;
 }