From 98c86e5109b67b081d1ceac2de1a17179c1e006d Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Sun, 20 Oct 2019 10:11:40 -0600 Subject: [PATCH] Kernel: Move E2BIG calculation from Thread to Process Thread::make_userspace_stack_for_main_thread is only ever called from Process::do_exec, after all the fun ELF loading and TSS setup has occured. The calculations in there that check if the combined argv + envp size will exceed the default stack size are not used in the rest of the stack setup. So, it should be safe to move this to the beginning of do_exec and bail early with -E2BIG, just like the man pages say. Additionally, advertise this limit in limits.h to be a good POSIX.1 citizen. :) --- Kernel/Process.cpp | 13 +++++++++++++ Kernel/Thread.cpp | 14 -------------- Kernel/Thread.h | 5 ++++- Libraries/LibC/limits.h | 4 +++- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 2c4b43e183..9887772a10 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -379,6 +380,18 @@ int Process::do_exec(String path, Vector arguments, Vector envir ASSERT_NOT_REACHED(); } + size_t total_blob_size = 0; + for (auto& a : arguments) + total_blob_size += a.length() + 1; + for (auto& e : environment) + total_blob_size += e.length() + 1; + + size_t total_meta_size = sizeof(char*) * (arguments.size() + 1) + sizeof(char*) * (environment.size() + 1); + + // FIXME: How much stack space does process startup need? + if ((total_blob_size + total_meta_size) >= Thread::default_userspace_stack_size) + return -E2BIG; + auto parts = path.split('/'); if (parts.is_empty()) return -ENOENT; diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index 8963b2b54c..467c4903ca 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -40,9 +40,6 @@ HashTable& thread_table() return *table; } -static const u32 default_kernel_stack_size = 65536; -static const u32 default_userspace_stack_size = 65536; - Thread::Thread(Process& process) : m_process(process) , m_tid(process.m_next_tid++) @@ -515,17 +512,6 @@ void Thread::make_userspace_stack_for_main_thread(Vector arguments, Vect char** env = argv + arguments.size() + 1; char* bufptr = stack_base + (sizeof(char*) * (arguments.size() + 1)) + (sizeof(char*) * (environment.size() + 1)); - size_t total_blob_size = 0; - for (auto& a : arguments) - total_blob_size += a.length() + 1; - for (auto& e : environment) - total_blob_size += e.length() + 1; - - size_t total_meta_size = sizeof(char*) * (arguments.size() + 1) + sizeof(char*) * (environment.size() + 1); - - // FIXME: It would be better if this didn't make us panic. - ASSERT((total_blob_size + total_meta_size) < default_userspace_stack_size); - for (int i = 0; i < arguments.size(); ++i) { argv[i] = bufptr; memcpy(bufptr, arguments[i].characters(), arguments[i].length()); diff --git a/Kernel/Thread.h b/Kernel/Thread.h index dc728f9d35..baeb85146d 100644 --- a/Kernel/Thread.h +++ b/Kernel/Thread.h @@ -1,10 +1,10 @@ #pragma once -#include #include #include #include #include +#include #include #include #include @@ -312,6 +312,9 @@ public: return state == Thread::State::Running || state == Thread::State::Runnable; } + static constexpr u32 default_kernel_stack_size = 65536; + static constexpr u32 default_userspace_stack_size = 65536; + private: IntrusiveListNode m_runnable_list_node; diff --git a/Libraries/LibC/limits.h b/Libraries/LibC/limits.h index 63156af450..08d8148381 100644 --- a/Libraries/LibC/limits.h +++ b/Libraries/LibC/limits.h @@ -6,7 +6,7 @@ #define PATH_MAX 4096 #if !defined MAXPATHLEN && defined PATH_MAX -# define MAXPATHLEN PATH_MAX +# define MAXPATHLEN PATH_MAX #endif #define INT_MAX INT32_MAX @@ -30,3 +30,5 @@ #define CHAR_MAX SCHAR_MAX #define MB_LEN_MAX 16 + +#define ARG_MAX 65536