Browse Source

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. :)
Andrew Kaster 5 years ago
parent
commit
98c86e5109
4 changed files with 20 additions and 16 deletions
  1. 13 0
      Kernel/Process.cpp
  2. 0 14
      Kernel/Thread.cpp
  3. 4 1
      Kernel/Thread.h
  4. 3 1
      Libraries/LibC/limits.h

+ 13 - 0
Kernel/Process.cpp

@@ -33,6 +33,7 @@
 #include <Kernel/StdLib.h>
 #include <Kernel/StdLib.h>
 #include <Kernel/Syscall.h>
 #include <Kernel/Syscall.h>
 #include <Kernel/TTY/MasterPTY.h>
 #include <Kernel/TTY/MasterPTY.h>
+#include <Kernel/Thread.h>
 #include <Kernel/VM/InodeVMObject.h>
 #include <Kernel/VM/InodeVMObject.h>
 #include <LibC/errno_numbers.h>
 #include <LibC/errno_numbers.h>
 #include <LibC/signal_numbers.h>
 #include <LibC/signal_numbers.h>
@@ -379,6 +380,18 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir
         ASSERT_NOT_REACHED();
         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('/');
     auto parts = path.split('/');
     if (parts.is_empty())
     if (parts.is_empty())
         return -ENOENT;
         return -ENOENT;

+ 0 - 14
Kernel/Thread.cpp

@@ -40,9 +40,6 @@ HashTable<Thread*>& thread_table()
     return *table;
     return *table;
 }
 }
 
 
-static const u32 default_kernel_stack_size = 65536;
-static const u32 default_userspace_stack_size = 65536;
-
 Thread::Thread(Process& process)
 Thread::Thread(Process& process)
     : m_process(process)
     : m_process(process)
     , m_tid(process.m_next_tid++)
     , m_tid(process.m_next_tid++)
@@ -515,17 +512,6 @@ void Thread::make_userspace_stack_for_main_thread(Vector<String> arguments, Vect
     char** env = argv + arguments.size() + 1;
     char** env = argv + arguments.size() + 1;
     char* bufptr = stack_base + (sizeof(char*) * (arguments.size() + 1)) + (sizeof(char*) * (environment.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) {
     for (int i = 0; i < arguments.size(); ++i) {
         argv[i] = bufptr;
         argv[i] = bufptr;
         memcpy(bufptr, arguments[i].characters(), arguments[i].length());
         memcpy(bufptr, arguments[i].characters(), arguments[i].length());

+ 4 - 1
Kernel/Thread.h

@@ -1,10 +1,10 @@
 #pragma once
 #pragma once
 
 
-#include <AK/String.h>
 #include <AK/Function.h>
 #include <AK/Function.h>
 #include <AK/IntrusiveList.h>
 #include <AK/IntrusiveList.h>
 #include <AK/OwnPtr.h>
 #include <AK/OwnPtr.h>
 #include <AK/RefPtr.h>
 #include <AK/RefPtr.h>
+#include <AK/String.h>
 #include <AK/Vector.h>
 #include <AK/Vector.h>
 #include <Kernel/Arch/i386/CPU.h>
 #include <Kernel/Arch/i386/CPU.h>
 #include <Kernel/KResult.h>
 #include <Kernel/KResult.h>
@@ -312,6 +312,9 @@ public:
         return state == Thread::State::Running || state == Thread::State::Runnable;
         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:
 private:
     IntrusiveListNode m_runnable_list_node;
     IntrusiveListNode m_runnable_list_node;
 
 

+ 3 - 1
Libraries/LibC/limits.h

@@ -6,7 +6,7 @@
 
 
 #define PATH_MAX 4096
 #define PATH_MAX 4096
 #if !defined MAXPATHLEN && defined PATH_MAX
 #if !defined MAXPATHLEN && defined PATH_MAX
-# define MAXPATHLEN  PATH_MAX
+#    define MAXPATHLEN PATH_MAX
 #endif
 #endif
 
 
 #define INT_MAX INT32_MAX
 #define INT_MAX INT32_MAX
@@ -30,3 +30,5 @@
 #define CHAR_MAX SCHAR_MAX
 #define CHAR_MAX SCHAR_MAX
 
 
 #define MB_LEN_MAX 16
 #define MB_LEN_MAX 16
+
+#define ARG_MAX 65536