1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-05-31 03:08:11 +00:00

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! :^)
This commit is contained in:
Andreas Kling 2019-11-17 12:11:43 +01:00
parent 197ed1bb2a
commit 794758df3a
12 changed files with 101 additions and 5 deletions

View file

@ -131,7 +131,7 @@ static void dump(const RegisterDump& regs)
u16 ss; u16 ss;
u32 esp; u32 esp;
if (!current || current->process().is_ring0()) { if (!current || current->process().is_ring0()) {
ss = regs.ds; ss = regs.ss;
esp = regs.esp; esp = regs.esp;
} else { } else {
ss = regs.ss_if_crossRing; 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) { if (!current) {
kprintf("%s with !current\n", description); kprintf("%s with !current\n", description);
hang(); hang();
} }
kprintf("\033[31;1mCRASH: %s %s: %s(%u)\033[0m\n", kprintf("\033[31;1mCRASH: %s. %s: %s(%u)\033[0m\n",
description, description,
current->process().is_ring0() ? "Kernel" : "Process", current->process().is_ring0() ? "Kernel" : "Process",
current->process().name().characters(), current->process().name().characters(),
@ -181,6 +181,7 @@ static void handle_crash(RegisterDump& regs, const char* description, int signal
hang(); hang();
} }
cli();
current->process().crash(signal, regs.eip); current->process().crash(signal, regs.eip);
} }
@ -263,6 +264,13 @@ void exception_14_handler(RegisterDump regs)
dump(regs); dump(regs);
#endif #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))); auto response = MM.handle_page_fault(PageFault(regs.exception_code, VirtualAddress(fault_address)));
if (response == PageFaultResponse::ShouldCrash) { if (response == PageFaultResponse::ShouldCrash) {

View file

@ -193,6 +193,7 @@ static_assert(sizeof(PageDirectoryEntry) == 4);
static_assert(sizeof(PageTableEntry) == 4); static_assert(sizeof(PageTableEntry) == 4);
class IRQHandler; class IRQHandler;
struct RegisterDump;
void gdt_init(); void gdt_init();
void idt_init(); void idt_init();
@ -208,6 +209,7 @@ u16 gdt_alloc_entry();
void gdt_free_entry(u16); void gdt_free_entry(u16);
Descriptor& get_gdt_entry(u16 selector); Descriptor& get_gdt_entry(u16 selector);
void write_gdt_entry(u16 selector, Descriptor&); void write_gdt_entry(u16 selector, Descriptor&);
void handle_crash(RegisterDump&, const char* description, int signal);
[[noreturn]] static inline void hang() [[noreturn]] static inline void hang()
{ {

View file

@ -207,12 +207,24 @@ void* Process::sys$mmap(const Syscall::SC_mmap_params* params)
return (void*)-EINVAL; return (void*)-EINVAL;
if ((flags & MAP_SHARED) && (flags & MAP_PRIVATE)) if ((flags & MAP_SHARED) && (flags & MAP_PRIVATE))
return (void*)-EINVAL; 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) { if (flags & MAP_ANONYMOUS) {
auto* region = allocate_region(VirtualAddress((u32)addr), size, name ? name : "mmap", prot, false); auto* region = allocate_region(VirtualAddress((u32)addr), size, name ? name : "mmap", prot, false);
if (!region) if (!region)
return (void*)-ENOMEM; return (void*)-ENOMEM;
if (flags & MAP_SHARED) if (flags & MAP_SHARED)
region->set_shared(true); region->set_shared(true);
if (flags & MAP_STACK)
region->set_stack(true);
return region->vaddr().as_ptr(); return region->vaddr().as_ptr();
} }
if (offset & ~PAGE_MASK) if (offset & ~PAGE_MASK)
@ -813,13 +825,15 @@ void Process::dump_regions()
kprintf("Process %s(%u) regions:\n", name().characters(), pid()); kprintf("Process %s(%u) regions:\n", name().characters(), pid());
kprintf("BEGIN END SIZE ACCESS NAME\n"); kprintf("BEGIN END SIZE ACCESS NAME\n");
for (auto& region : m_regions) { 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().get(),
region.vaddr().offset(region.size() - 1).get(), region.vaddr().offset(region.size() - 1).get(),
region.size(), region.size(),
region.is_readable() ? 'R' : ' ', region.is_readable() ? 'R' : ' ',
region.is_writable() ? 'W' : ' ', region.is_writable() ? 'W' : ' ',
region.is_executable() ? 'X' : ' ', region.is_executable() ? 'X' : ' ',
region.is_shared() ? 'S' : ' ',
region.is_stack() ? 'T' : ' ',
region.name().characters()); region.name().characters());
} }
} }

View file

@ -2,6 +2,7 @@
#include <Kernel/Process.h> #include <Kernel/Process.h>
#include <Kernel/ProcessTracer.h> #include <Kernel/ProcessTracer.h>
#include <Kernel/Syscall.h> #include <Kernel/Syscall.h>
#include <Kernel/VM/MemoryManager.h>
extern "C" void syscall_trap_entry(RegisterDump); extern "C" void syscall_trap_entry(RegisterDump);
extern "C" void syscall_trap_handler(); 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) void syscall_trap_entry(RegisterDump regs)
{ {
auto& process = current->process(); 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(); process.big_lock().lock();
u32 function = regs.eax; u32 function = regs.eax;
u32 arg1 = regs.edx; u32 arg1 = regs.edx;

View file

@ -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); auto* region = m_process.allocate_region(VirtualAddress(), default_userspace_stack_size, "Stack (Main thread)", PROT_READ | PROT_WRITE, false);
ASSERT(region); ASSERT(region);
region->set_stack(true);
m_tss.esp = region->vaddr().offset(default_userspace_stack_size).get(); m_tss.esp = region->vaddr().offset(default_userspace_stack_size).get();
char* stack_base = (char*)region->vaddr().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); 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); 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(); m_tss.esp = m_userspace_stack_region->vaddr().offset(default_userspace_stack_size).get();
// NOTE: The stack needs to be 16-byte aligned. // NOTE: The stack needs to be 16-byte aligned.

View file

@ -26,6 +26,7 @@
#define MAP_FIXED 0x10 #define MAP_FIXED 0x10
#define MAP_ANONYMOUS 0x20 #define MAP_ANONYMOUS 0x20
#define MAP_ANON MAP_ANONYMOUS #define MAP_ANON MAP_ANONYMOUS
#define MAP_STACK 0x40
#define PROT_READ 0x1 #define PROT_READ 0x1
#define PROT_WRITE 0x2 #define PROT_WRITE 0x2

View file

@ -549,6 +549,12 @@ void MemoryManager::unquickmap_page()
m_quickmap_in_use = false; 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 bool MemoryManager::validate_user_read(const Process& process, VirtualAddress vaddr) const
{ {
auto* region = region_from_vaddr(process, vaddr); auto* region = region_from_vaddr(process, vaddr);

View file

@ -46,6 +46,7 @@ public:
void enter_process_paging_scope(Process&); 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_read(const Process&, VirtualAddress) const;
bool validate_user_write(const Process&, VirtualAddress) const; bool validate_user_write(const Process&, VirtualAddress) const;

View file

@ -58,6 +58,7 @@ NonnullOwnPtr<Region> Region::clone()
ASSERT(is_user_accessible()); ASSERT(is_user_accessible());
if (m_shared || (is_readable() && !is_writable())) { if (m_shared || (is_readable() && !is_writable())) {
ASSERT(!m_stack);
#ifdef MM_DEBUG #ifdef MM_DEBUG
dbgprintf("%s<%u> Region::clone(): sharing %s (V%p)\n", dbgprintf("%s<%u> Region::clone(): sharing %s (V%p)\n",
current->process().name().characters(), current->process().name().characters(),
@ -81,6 +82,13 @@ NonnullOwnPtr<Region> Region::clone()
remap(); remap();
auto clone_region = Region::create_user_accessible(m_range, m_vmobject->clone(), m_offset_in_vmo, m_name, m_access); 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(); 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; return clone_region;
} }

View file

@ -50,6 +50,9 @@ public:
bool is_shared() const { return m_shared; } bool is_shared() const { return m_shared; }
void set_shared(bool shared) { m_shared = 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; } bool is_user_accessible() const { return m_user_accessible; }
PageFaultResponse handle_fault(const PageFault&); PageFaultResponse handle_fault(const PageFault&);
@ -141,5 +144,6 @@ private:
u8 m_access { 0 }; u8 m_access { 0 };
bool m_shared { false }; bool m_shared { false };
bool m_user_accessible { false }; bool m_user_accessible { false };
bool m_stack { false };
mutable OwnPtr<Bitmap> m_cow_map; mutable OwnPtr<Bitmap> m_cow_map;
}; };

View file

@ -8,6 +8,7 @@
#define MAP_FIXED 0x10 #define MAP_FIXED 0x10
#define MAP_ANONYMOUS 0x20 #define MAP_ANONYMOUS 0x20
#define MAP_ANON MAP_ANONYMOUS #define MAP_ANON MAP_ANONYMOUS
#define MAP_STACK 0x40
#define PROT_READ 0x1 #define PROT_READ 0x1
#define PROT_WRITE 0x2 #define PROT_WRITE 0x2

View file

@ -5,7 +5,7 @@
static void print_usage_and_exit() static void print_usage_and_exit()
{ {
printf("usage: crash -[sdiamfMF]\n"); printf("usage: crash -[sdiamfMFTt]\n");
exit(0); exit(0);
} }
@ -22,6 +22,8 @@ int main(int argc, char** argv)
ReadFromUninitializedMallocMemory, ReadFromUninitializedMallocMemory,
ReadFromFreedMemory, ReadFromFreedMemory,
WriteToReadonlyMemory, WriteToReadonlyMemory,
InvalidStackPointerOnSyscall,
InvalidStackPointerOnPageFault,
}; };
Mode mode = SegmentationViolation; Mode mode = SegmentationViolation;
@ -46,6 +48,10 @@ int main(int argc, char** argv)
mode = WriteToFreedMemory; mode = WriteToFreedMemory;
else if (String(argv[1]) == "-r") else if (String(argv[1]) == "-r")
mode = WriteToReadonlyMemory; mode = WriteToReadonlyMemory;
else if (String(argv[1]) == "-T")
mode = InvalidStackPointerOnSyscall;
else if (String(argv[1]) == "-t")
mode = InvalidStackPointerOnPageFault;
else else
print_usage_and_exit(); print_usage_and_exit();
@ -111,6 +117,41 @@ int main(int argc, char** argv)
*ptr = 'y'; // This should crash! *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(); ASSERT_NOT_REACHED();
return 0; return 0;
} }