Kernel: Implement some basic stack pointer validation

VM regions can now be marked as stack regions, which is then validated
on syscall, and on page fault.

If a thread is caught with its stack pointer pointing into anything
that's *not* a Region with its stack bit set, we'll crash the whole
process with SIGSTKFLT.

Userspace must now allocate custom stacks by using mmap() with the new
MAP_STACK flag. This mechanism was first introduced in OpenBSD, and now
we have it too, yay! :^)

12 files changed, 101 insertions, 5 deletions

diff --git a/Kernel/Arch/i386/CPU.cpp b/Kernel/Arch/i386/CPU.cpp
index 1bbdb0bd50..1b88059ade 100644
--- a/Kernel/Arch/i386/CPU.cpp
+++ b/Kernel/Arch/i386/CPU.cpp
@@ -131,7 +131,7 @@ static void dump(const RegisterDump& regs)
     u16 ss;
     u32 esp;
     if (!current || current->process().is_ring0()) {
-        ss = regs.ds;
+        ss = regs.ss;
         esp = regs.esp;
     } else {
         ss = regs.ss_if_crossRing;
@@ -160,14 +160,14 @@ static void dump(const RegisterDump& regs)
     }
 }
 
-static void handle_crash(RegisterDump& regs, const char* description, int signal)
+void handle_crash(RegisterDump& regs, const char* description, int signal)
 {
     if (!current) {
         kprintf("%s with !current\n", description);
         hang();
     }
 
-    kprintf("\033[31;1mCRASH: %s %s: %s(%u)\033[0m\n",
+    kprintf("\033[31;1mCRASH: %s. %s: %s(%u)\033[0m\n",
         description,
         current->process().is_ring0() ? "Kernel" : "Process",
         current->process().name().characters(),
@@ -181,6 +181,7 @@ static void handle_crash(RegisterDump& regs, const char* description, int signal
         hang();
     }
 
+    cli();
     current->process().crash(signal, regs.eip);
 }
 
@@ -263,6 +264,13 @@ void exception_14_handler(RegisterDump regs)
     dump(regs);
 #endif
 
+    bool faulted_in_userspace = (regs.cs & 3) == 3;
+    if (faulted_in_userspace && !MM.validate_user_stack(current->process(), VirtualAddress(regs.esp_if_crossRing))) {
+        dbgprintf("Invalid stack pointer: %p\n", regs.esp_if_crossRing);
+        handle_crash(regs, "Bad stack on page fault", SIGSTKFLT);
+        ASSERT_NOT_REACHED();
+    }
+
     auto response = MM.handle_page_fault(PageFault(regs.exception_code, VirtualAddress(fault_address)));
 
     if (response == PageFaultResponse::ShouldCrash) {
diff --git a/Kernel/Arch/i386/CPU.h b/Kernel/Arch/i386/CPU.h
index 5b14c84d98..5fa9057bb1 100644
--- a/Kernel/Arch/i386/CPU.h
+++ b/Kernel/Arch/i386/CPU.h
@@ -193,6 +193,7 @@ static_assert(sizeof(PageDirectoryEntry) == 4);
 static_assert(sizeof(PageTableEntry) == 4);
 
 class IRQHandler;
+struct RegisterDump;
 
 void gdt_init();
 void idt_init();
@@ -208,6 +209,7 @@ u16 gdt_alloc_entry();
 void gdt_free_entry(u16);
 Descriptor& get_gdt_entry(u16 selector);
 void write_gdt_entry(u16 selector, Descriptor&);
+void handle_crash(RegisterDump&, const char* description, int signal);
 
 [[noreturn]] static inline void hang()
 {
diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index 2d15eab79d..314d2df948 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -207,12 +207,24 @@ void* Process::sys$mmap(const Syscall::SC_mmap_params* params)
         return (void*)-EINVAL;
     if ((flags & MAP_SHARED) && (flags & MAP_PRIVATE))
         return (void*)-EINVAL;
+
+    // EINVAL: MAP_STACK cannot be used with shared or file-backed mappings
+    if ((flags & MAP_STACK) && ((flags & MAP_SHARED) || !(flags & MAP_PRIVATE) || !(flags & MAP_ANONYMOUS)))
+        return (void*)-EINVAL;
+
+    // EINVAL: MAP_STACK cannot be used with non-readable or non-writable memory
+    if ((flags & MAP_STACK) && (!(prot & PROT_READ) || !(prot & PROT_WRITE)))
+        return (void*)-EINVAL;
+
+    // FIXME: The rest of this function seems like it could share more code..
     if (flags & MAP_ANONYMOUS) {
         auto* region = allocate_region(VirtualAddress((u32)addr), size, name ? name : "mmap", prot, false);
         if (!region)
             return (void*)-ENOMEM;
         if (flags & MAP_SHARED)
             region->set_shared(true);
+        if (flags & MAP_STACK)
+            region->set_stack(true);
         return region->vaddr().as_ptr();
     }
     if (offset & ~PAGE_MASK)
@@ -813,13 +825,15 @@ void Process::dump_regions()
     kprintf("Process %s(%u) regions:\n", name().characters(), pid());
     kprintf("BEGIN       END         SIZE        ACCESS  NAME\n");
     for (auto& region : m_regions) {
-        kprintf("%08x -- %08x    %08x    %c%c%c     %s\n",
+        kprintf("%08x -- %08x    %08x    %c%c%c%c%c     %s\n",
             region.vaddr().get(),
             region.vaddr().offset(region.size() - 1).get(),
             region.size(),
             region.is_readable() ? 'R' : ' ',
             region.is_writable() ? 'W' : ' ',
             region.is_executable() ? 'X' : ' ',
+            region.is_shared() ? 'S' : ' ',
+            region.is_stack() ? 'T' : ' ',
             region.name().characters());
     }
 }
diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp
index e86c075ad5..a83c093613 100644
--- a/Kernel/Syscall.cpp
+++ b/Kernel/Syscall.cpp
@@ -2,6 +2,7 @@
 #include <Kernel/Process.h>
 #include <Kernel/ProcessTracer.h>
 #include <Kernel/Syscall.h>
+#include <Kernel/VM/MemoryManager.h>
 
 extern "C" void syscall_trap_entry(RegisterDump);
 extern "C" void syscall_trap_handler();
@@ -91,6 +92,13 @@ int handle(RegisterDump& regs, u32 function, u32 arg1, u32 arg2, u32 arg3)
 void syscall_trap_entry(RegisterDump regs)
 {
     auto& process = current->process();
+
+    if (!MM.validate_user_stack(process, VirtualAddress(regs.esp_if_crossRing))) {
+        dbgprintf("Invalid stack pointer: %p\n", regs.esp_if_crossRing);
+        handle_crash(regs, "Bad stack on syscall entry", SIGSTKFLT);
+        ASSERT_NOT_REACHED();
+    }
+
     process.big_lock().lock();
     u32 function = regs.eax;
     u32 arg1 = regs.edx;
diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp
index 4e4f8fae14..f1034e61e4 100644
--- a/Kernel/Thread.cpp
+++ b/Kernel/Thread.cpp
@@ -569,6 +569,7 @@ void Thread::make_userspace_stack_for_main_thread(Vector<String> arguments, Vect
 {
     auto* region = m_process.allocate_region(VirtualAddress(), default_userspace_stack_size, "Stack (Main thread)", PROT_READ | PROT_WRITE, false);
     ASSERT(region);
+    region->set_stack(true);
     m_tss.esp = region->vaddr().offset(default_userspace_stack_size).get();
 
     char* stack_base = (char*)region->vaddr().get();
@@ -604,6 +605,7 @@ void Thread::make_userspace_stack_for_secondary_thread(void* argument)
 {
     m_userspace_stack_region = m_process.allocate_region(VirtualAddress(), default_userspace_stack_size, String::format("Stack (Thread %d)", tid()), PROT_READ | PROT_WRITE, false);
     ASSERT(m_userspace_stack_region);
+    m_userspace_stack_region->set_stack(true);
     m_tss.esp = m_userspace_stack_region->vaddr().offset(default_userspace_stack_size).get();
 
     // NOTE: The stack needs to be 16-byte aligned.
diff --git a/Kernel/UnixTypes.h b/Kernel/UnixTypes.h
index e50bfc0cb6..20af416642 100644
--- a/Kernel/UnixTypes.h
+++ b/Kernel/UnixTypes.h
@@ -26,6 +26,7 @@
 #define MAP_FIXED 0x10
 #define MAP_ANONYMOUS 0x20
 #define MAP_ANON MAP_ANONYMOUS
+#define MAP_STACK 0x40
 
 #define PROT_READ 0x1
 #define PROT_WRITE 0x2
diff --git a/Kernel/VM/MemoryManager.cpp b/Kernel/VM/MemoryManager.cpp
index 885cdbd19f..dcfb041828 100644
--- a/Kernel/VM/MemoryManager.cpp
+++ b/Kernel/VM/MemoryManager.cpp
@@ -549,6 +549,12 @@ void MemoryManager::unquickmap_page()
     m_quickmap_in_use = false;
 }
 
+bool MemoryManager::validate_user_stack(const Process& process, VirtualAddress vaddr) const
+{
+    auto* region = region_from_vaddr(process, vaddr);
+    return region && region->is_stack();
+}
+
 bool MemoryManager::validate_user_read(const Process& process, VirtualAddress vaddr) const
 {
     auto* region = region_from_vaddr(process, vaddr);
diff --git a/Kernel/VM/MemoryManager.h b/Kernel/VM/MemoryManager.h
index 399d0f1502..619f95630c 100644
--- a/Kernel/VM/MemoryManager.h
+++ b/Kernel/VM/MemoryManager.h
@@ -46,6 +46,7 @@ public:
 
     void enter_process_paging_scope(Process&);
 
+    bool validate_user_stack(const Process&, VirtualAddress) const;
     bool validate_user_read(const Process&, VirtualAddress) const;
     bool validate_user_write(const Process&, VirtualAddress) const;
 
diff --git a/Kernel/VM/Region.cpp b/Kernel/VM/Region.cpp
index 65495779d3..7104dd1534 100644
--- a/Kernel/VM/Region.cpp
+++ b/Kernel/VM/Region.cpp
@@ -58,6 +58,7 @@ NonnullOwnPtr<Region> Region::clone()
     ASSERT(is_user_accessible());
 
     if (m_shared || (is_readable() && !is_writable())) {
+        ASSERT(!m_stack);
 #ifdef MM_DEBUG
         dbgprintf("%s<%u> Region::clone(): sharing %s (V%p)\n",
             current->process().name().characters(),
@@ -81,6 +82,13 @@ NonnullOwnPtr<Region> Region::clone()
     remap();
     auto clone_region = Region::create_user_accessible(m_range, m_vmobject->clone(), m_offset_in_vmo, m_name, m_access);
     clone_region->ensure_cow_map();
+    if (m_stack) {
+        ASSERT(is_readable());
+        ASSERT(is_writable());
+        ASSERT(!is_shared());
+        ASSERT(vmobject().is_anonymous());
+        clone_region->set_stack(true);
+    }
     return clone_region;
 }
 
diff --git a/Kernel/VM/Region.h b/Kernel/VM/Region.h
index 50ea87ff04..5501e6b2e3 100644
--- a/Kernel/VM/Region.h
+++ b/Kernel/VM/Region.h
@@ -50,6 +50,9 @@ public:
     bool is_shared() const { return m_shared; }
     void set_shared(bool shared) { m_shared = shared; }
 
+    bool is_stack() const { return m_stack; }
+    void set_stack(bool stack) { m_stack = stack; }
+
     bool is_user_accessible() const { return m_user_accessible; }
 
     PageFaultResponse handle_fault(const PageFault&);
@@ -141,5 +144,6 @@ private:
     u8 m_access { 0 };
     bool m_shared { false };
     bool m_user_accessible { false };
+    bool m_stack { false };
     mutable OwnPtr<Bitmap> m_cow_map;
 };
diff --git a/Libraries/LibC/mman.h b/Libraries/LibC/mman.h
index 513328bb06..1720242915 100644
--- a/Libraries/LibC/mman.h
+++ b/Libraries/LibC/mman.h
@@ -8,6 +8,7 @@
 #define MAP_FIXED 0x10
 #define MAP_ANONYMOUS 0x20
 #define MAP_ANON MAP_ANONYMOUS
+#define MAP_STACK 0x40
 
 #define PROT_READ 0x1
 #define PROT_WRITE 0x2
diff --git a/Userland/crash.cpp b/Userland/crash.cpp
index 7584c8359e..fa6f6e9b3e 100644
--- a/Userland/crash.cpp
+++ b/Userland/crash.cpp
@@ -5,7 +5,7 @@
 
 static void print_usage_and_exit()
 {
-    printf("usage: crash -[sdiamfMF]\n");
+    printf("usage: crash -[sdiamfMFTt]\n");
     exit(0);
 }
 
@@ -22,6 +22,8 @@ int main(int argc, char** argv)
         ReadFromUninitializedMallocMemory,
         ReadFromFreedMemory,
         WriteToReadonlyMemory,
+        InvalidStackPointerOnSyscall,
+        InvalidStackPointerOnPageFault,
     };
     Mode mode = SegmentationViolation;
 
@@ -46,6 +48,10 @@ int main(int argc, char** argv)
         mode = WriteToFreedMemory;
     else if (String(argv[1]) == "-r")
         mode = WriteToReadonlyMemory;
+    else if (String(argv[1]) == "-T")
+        mode = InvalidStackPointerOnSyscall;
+    else if (String(argv[1]) == "-t")
+        mode = InvalidStackPointerOnPageFault;
     else
         print_usage_and_exit();
 
@@ -111,6 +117,41 @@ int main(int argc, char** argv)
         *ptr = 'y'; // This should crash!
     }
 
+    if (mode == InvalidStackPointerOnSyscall) {
+        u8* makeshift_stack = (u8*)mmap(nullptr, PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE | MAP_STACK, 0, 0);
+        if (!makeshift_stack) {
+            perror("mmap");
+            return 1;
+        }
+        u8* makeshift_esp = makeshift_stack + 2048;
+        asm volatile("mov %%eax, %%esp" :: "a"(makeshift_esp));
+        getuid();
+        dbgprintf("Survived syscall with MAP_STACK stack\n");
+
+        u8* bad_stack = (u8*)mmap(nullptr, PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+        if (!bad_stack) {
+            perror("mmap");
+            return 1;
+        }
+        u8* bad_esp = bad_stack + 2048;
+        asm volatile("mov %%eax, %%esp" :: "a"(bad_esp));
+        getuid();
+
+        ASSERT_NOT_REACHED();
+    }
+
+    if (mode == InvalidStackPointerOnPageFault) {
+        u8* bad_stack = (u8*)mmap(nullptr, PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+        if (!bad_stack) {
+            perror("mmap");
+            return 1;
+        }
+        u8* bad_esp = bad_stack + 2048;
+        asm volatile("mov %%eax, %%esp" :: "a"(bad_esp));
+        asm volatile("pushl $0");
+        ASSERT_NOT_REACHED();
+    }
+
     ASSERT_NOT_REACHED();
     return 0;
 }