From 44e4ca804af7cf2716547eb19c097cec3d54366f Mon Sep 17 00:00:00 2001 From: apio Date: Thu, 4 May 2023 16:36:24 +0200 Subject: [PATCH] kernel: Make sure argument vectors passed to execve() are not too big --- kernel/src/sys/exec.cpp | 25 +++++++++++++++++++++++-- kernel/src/thread/ThreadImage.cpp | 8 ++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/kernel/src/sys/exec.cpp b/kernel/src/sys/exec.cpp index 4259b8a8..5aa01261 100644 --- a/kernel/src/sys/exec.cpp +++ b/kernel/src/sys/exec.cpp @@ -31,15 +31,36 @@ static Result> copy_string_vector_from_userspace(u64 address) return result; } +static u64 calculate_userspace_stack_size(const Vector& v) +{ + u64 total { 0 }; + + for (const auto& str : v) + { + // The string's byte count + a terminating NUL byte. + total += str.length() + 1; + // The pointer to said string in the userspace array. + total += sizeof(char*); + } + + // The NULL pointer at the end of the userspace array. + total += sizeof(char*); + + return total; +} + +static constexpr usize MAX_ARGV_STACK_SIZE = 2 * ARCH_PAGE_SIZE; + Result sys_execve(Registers* regs, SyscallArgs args) { auto path = TRY(MemoryManager::strdup_from_user(args[0])); auto argv = TRY(copy_string_vector_from_userspace(args[1])); auto envp = TRY(copy_string_vector_from_userspace(args[2])); - auto current = Scheduler::current(); + if ((calculate_userspace_stack_size(argv) + calculate_userspace_stack_size(envp)) > MAX_ARGV_STACK_SIZE) + return err(E2BIG); - // FIXME: Make sure argv & envp are not too big. + auto current = Scheduler::current(); auto inode = TRY(VFS::resolve_path(path.chars(), current->auth, current->current_directory)); diff --git a/kernel/src/thread/ThreadImage.cpp b/kernel/src/thread/ThreadImage.cpp index 19692d70..1ff9e0a9 100644 --- a/kernel/src/thread/ThreadImage.cpp +++ b/kernel/src/thread/ThreadImage.cpp @@ -3,11 +3,15 @@ #include "thread/Thread.h" #include +static constexpr usize DEFAULT_USER_STACK_PAGES = 6; +static constexpr usize DEFAULT_USER_STACK_SIZE = DEFAULT_USER_STACK_PAGES * ARCH_PAGE_SIZE; + static Result create_stacks(Stack& user_stack, Stack& kernel_stack) { const u64 THREAD_STACK_BASE = 0x10000; - TRY(MemoryManager::alloc_at_zeroed(THREAD_STACK_BASE, 4, MMU::ReadWrite | MMU::NoExecute | MMU::User)); + TRY(MemoryManager::alloc_at_zeroed(THREAD_STACK_BASE, DEFAULT_USER_STACK_PAGES, + MMU::ReadWrite | MMU::NoExecute | MMU::User)); auto guard = make_scope_guard([&] { MemoryManager::unmap_owned(THREAD_STACK_BASE, 4); }); @@ -15,7 +19,7 @@ static Result create_stacks(Stack& user_stack, Stack& kernel_stack) guard.deactivate(); - user_stack = { THREAD_STACK_BASE, 4 * ARCH_PAGE_SIZE }; + user_stack = { THREAD_STACK_BASE, DEFAULT_USER_STACK_SIZE }; kernel_stack = { kernel_stack_base, 4 * ARCH_PAGE_SIZE }; return {};