Implement on-demand stack growth via page faults

CMakeLists.txt

@@ -10,6 +10,9 @@ set(CMAKE_TOOLCHAIN_FILE cmake/toolchain.cmake)
 # Extra checks in malloc
 set(KERNEL_MEMORY_INTEGRITY_CHECKS_ENABLED 1)
 
+# Default maximum size (in KB) a process' stack is allowed to grow to.
+set(KERNEL_MAX_STACK_SIZE_KB 8192)
+
 execute_process(
     COMMAND git describe --tags --abbrev=0
     OUTPUT_VARIABLE OS_VERSION

cmake/config.h.in

@@ -11,6 +11,7 @@
 #define CPU_ARCH "@CPU_ARCH@"
 #define FILE_PREFIX_LENGTH @FILE_PREFIX_LENGTH@
 #define KERNEL_MEMORY_INTEGRITY_CHECKS_ENABLED @KERNEL_MEMORY_INTEGRITY_CHECKS_ENABLED@
+#define KERNEL_MAX_STACK_SIZE_KB @KERNEL_MAX_STACK_SIZE_KB@
 
 #define PROJECT_DESCRIPTION PROJECT_NAME \
     " version=" PROJECT_VERSION \

design/kernel_parameters.md

@@ -16,4 +16,5 @@ generated from `cmake/config.h.in`).
 | CPU_ARCH                               | CPU Architecture                                                                      |
 | FILE_PREFIX_LENGTH                     | Length of absolute path to `src/`, used in logger macro to isolate relative file path |
 | KERNEL_MEMORY_INTEGRITY_CHECKS_ENABLED | Perform extra checks during malloc operations                                         |
+| KERNEL_MAX_STACK_SIZE_KB               | Default maximum size in KB a process' stack may grow to                              |
 | PROJECT_DESCRIPTION                    | String that combines name, version and arguments to the kernel                        |

src/cpu/i386/src/isr.c

@@ -2,8 +2,13 @@
 
 #include "cpu/isr.h"
 
+#include <stdbool.h>
+
+#include "addr.h"
 #include "cpu/idt.h"
+#include "cpu/mmu.h"
 #include "cpu/ports.h"
+#include "defs.h"
 #include "kernel.h"
 #include "kernel/logs.h"
 #include "libc/proc.h"
@@ -13,6 +18,8 @@
 
 static isr_t __interrupt_handlers[256];
 
+static int handle_stack_growth_fault(registers_t * r);
+
 /* Can't do this with a loop because we need the address
  * of the function names */
 void isr_install() {
@@ -125,6 +132,12 @@ char * exception_messages[] = {
 };
 
 void isr_handler(registers_t r) {
+    if (r.int_no == 14 && !handle_stack_growth_fault(&r)) {
+        // Stack grown enough to retry the faulting instruction (possibly
+        // not all the way to the fault yet - see handle_stack_growth_fault).
+        return;
+    }
+
     print_trace(&r);
     printf("ISR %u (err 0x%X)\n", r.int_no, r.err_code);
     printf("%s\n", exception_messages[r.int_no]);
@@ -264,6 +277,78 @@ void print_trace(registers_t * r) {
     print_cr0(r->cr0);
     putc('\n');
 }
+
+// A faulting PUSH/PUSHA is restartable: on fault, the CPU rolls back to the
+// pre-instruction ESP (the useresp saved on the ring3->ring0 transition), not
+// to wherever the in-flight instruction had reached. So the lowest address an
+// otherwise-legitimate stack write can fault on is up to this many bytes
+// below useresp - PUSHA being the worst case (8 dwords). Without this slack,
+// a fault landing just below a page boundary (CR2 one page lower than
+// useresp) would be wrongly rejected as illegitimate.
+#define MAX_STACK_FAULT_SLACK_BYTES 32
+
+// Caps how many pages a single page fault will grow the stack by. Larger
+// requests (eg. a huge single access after `sub esp, <huge>`) are only
+// grown up to this many pages per fault; if that isn't enough to reach the
+// faulting page, the faulting instruction is retried, re-faults at the same
+// address, and growth continues from there on the next call - bounding how
+// much work (and RAM) one fault can commit while interrupts are disabled,
+// instead of one fault being able to claim the whole configured stack size
+// limit at once. Either way, total stack growth remains bounded by the
+// process' own proc->max_stack_pages, enforced in process_grow_stack.
+#define MAX_STACK_GROWTH_PAGES_PER_FAULT 16
+
+/**
+ * @brief Try to handle a page fault by growing the faulting process' stack
+ * down to (or partway to, see MAX_STACK_GROWTH_PAGES_PER_FAULT) the faulting
+ * page.
+ *
+ * Only handles a user-mode fault on a non-present page that is at or below
+ * the current stack end and at or above (allowing for
+ * MAX_STACK_FAULT_SLACK_BYTES) the saved user ESP - anything else
+ * (protection fault, supervisor fault, or a fault outside that range) is
+ * left for the caller to report and panic on.
+ *
+ * @param r page fault registers (int_no == 14)
+ * @return 0 if the fault was handled (stack grown far enough to retry, even
+ * if not all the way to the fault - see MAX_STACK_GROWTH_PAGES_PER_FAULT),
+ * non-zero otherwise
+ */
+static int handle_stack_growth_fault(registers_t * r) {
+    bool user_mode    = (r->err_code & MMU_DIR_FLAG_USER_SUPERVISOR) != 0;
+    bool page_present = (r->err_code & MMU_DIR_FLAG_PRESENT) != 0;
+
+    if (!user_mode || page_present) {
+        return -1;
+    }
+
+    process_t * proc = get_current_process();
+    if (!proc) {
+        return -1;
+    }
+
+    size_t fault_page      = ADDR2PAGE(r->cr2);
+    size_t next_stack_page = ADDR2PAGE(VADDR_USER_STACK) - proc->stack_page_count;
+
+    if (fault_page > next_stack_page) {
+        return -1;
+    }
+
+    size_t lowest_valid_page = ADDR2PAGE(r->useresp - MAX_STACK_FAULT_SLACK_BYTES);
+    if (fault_page < lowest_valid_page) {
+        return -1;
+    }
+
+    for (int i = 0; i < MAX_STACK_GROWTH_PAGES_PER_FAULT && next_stack_page >= fault_page; i++) {
+        if (process_grow_stack(proc)) {
+            return -1;
+        }
+        next_stack_page = ADDR2PAGE(VADDR_USER_STACK) - proc->stack_page_count;
+    }
+
+    return 0;
+}
+
 /*
 Bit	Label	Description
 0	PE	Protected Mode Enable

src/kernel/include/process.h

@@ -68,6 +68,8 @@ typedef struct _process {
     uint32_t next_heap_page;
     /// Number of pages allocated to the stack
     uint32_t stack_page_count;
+    /// Maximum number of pages the stack may grow to
+    uint32_t max_stack_pages;
 
     /// PID of parent process (0 is no parent)
     uint32_t parent_pid;
@@ -167,10 +169,29 @@ void * process_add_pages(process_t * proc, size_t count);
  * and heap can never overlap.
  *
  * @param proc pointer to the process object
- * @return int 0 for success, -1 if it would collide with the heap or on error
+ * @return 0 for success, -1 if it would collide with the heap, would
+ * exceed proc->max_stack_pages, or on error
  */
 int process_grow_stack(process_t * proc);
 
+/**
+ * @brief Change the maximum size (in KB) a process' stack may grow to,
+ * rounding up to a whole page if `max_stack_size_kb` is not already page
+ * aligned.
+ *
+ * @todo This currently checks against *allocated* stack pages
+ * (stack_page_count), not how much of the stack is actually *in use*. Once
+ * that's tracked, this should instead check against actual stack usage, and
+ * shrinking should free any allocated pages that fall outside the new,
+ * smaller limit.
+ *
+ * @param proc pointer to the process object
+ * @param max_stack_size_kb new maximum stack size in KB
+ * @return 0 for success, -1 if null process or the rounded page count is
+ * smaller than the currently allocated stack page count
+ */
+int process_set_max_stack_size_kb(process_t * proc, uint32_t max_stack_size_kb);
+
 /**
  * @brief Allocate pages in the heap and copy data from `buff` into the pages.
  *

src/kernel/src/exec.c

@@ -54,18 +54,6 @@ int command_exec(uint8_t * buff, const char * filepath, size_t size, size_t argc
         return -1;
     }
 
-    KLOG_TRACE("Increasing stack by 1022 pages");
-    for (size_t i = 0; i < 1022; i++) {
-        if (process_grow_stack(proc)) {
-            KLOG_DEBUG("Failed to add page %u", i);
-            if (process_free(proc)) {
-                KPANIC("Failed to free process while handling stack grow error");
-            }
-            kfree(proc);
-            return -1;
-        }
-    }
-
     if (process_copy_args(proc, filepath, argc, argv)) {
         KLOG_DEBUG("Failed to copy args");
         if (process_free(proc)) {

src/kernel/src/kernel.c

@@ -167,7 +167,7 @@ int kernel_exec(const char * filename, size_t argc, char ** argv) {
         return -1;
     }
 
-    KLOG_DEBUG("Process for exec %d finished for filename %s", pid);
+    KLOG_DEBUG("Process for exec %d finished for filename %s", pid, filename);
 
     io_device_close(file);
     KLOG_TRACE("File device closed");

src/kernel/src/loader.c

@@ -237,15 +237,6 @@ static process_t * load_init() {
         return 0;
     }
 
-    for (size_t i = 0; i < 1022; i++) {
-        if (process_grow_stack(proc)) {
-            KLOG_DEBUG("Failed to grow process stack");
-            return 0;
-        }
-    }
-
-    KLOG_TRACE("Stack allocated for process %u", proc->pid);
-
     process_copy_args(proc, filename, 0, 0);
 
     KLOG_TRACE("Copied filename %s to process %u", filename, proc->pid);

src/kernel/src/process.c

@@ -2,6 +2,7 @@
 
 #include "process.h"
 
+#include "config.h"
 #include "cpu/gdt.h"
 #include "cpu/mmu.h"
 #include "cpu/tss.h"
@@ -16,9 +17,10 @@
 
 static int open_stdio_handles(process_t * proc);
 
-static char * copy_string(const char * str);
-static int    copy_to_process_pages(process_t * proc, uint32_t page_start, size_t count, const char * buff, size_t size);
-static int    write_process_dwords(process_t * proc, uint32_t first_addr, const uint32_t * values, size_t count);
+static char *   copy_string(const char * str);
+static int      copy_to_process_pages(process_t * proc, uint32_t page_start, size_t count, const char * buff, size_t size);
+static int      write_process_dwords(process_t * proc, uint32_t first_addr, const uint32_t * values, size_t count);
+static uint32_t stack_size_kb_to_pages(uint32_t size_kb);
 
 static uint32_t next_pid();
 static uint32_t next_handle_id();
@@ -93,6 +95,7 @@ int process_create(process_t * proc) {
     proc->pid              = next_pid();
     proc->next_heap_page   = ADDR2PAGE(VADDR_USER_MEM);
     proc->stack_page_count = 1;
+    proc->max_stack_pages  = stack_size_kb_to_pages(KERNEL_MAX_STACK_SIZE_KB);
 
     // TODO parent process pid
 
@@ -426,6 +429,11 @@ int process_grow_stack(process_t * proc) {
         return -1;
     }
 
+    if (proc->stack_page_count >= proc->max_stack_pages) {
+        KLOG_WARNING("Cannot grow stack past %u pages (max_stack_pages)", proc->max_stack_pages);
+        return -1;
+    }
+
     // Stack pages grow down starting immediately below the first user stack
     // page allocated by process_create (at ADDR2PAGE(VADDR_USER_STACK)).
     // proc->stack_page_count starts at 1 (that first page), so the Nth call
@@ -463,6 +471,28 @@ int process_grow_stack(process_t * proc) {
     return 0;
 }
 
+int process_set_max_stack_size_kb(process_t * proc, uint32_t max_stack_size_kb) {
+    if (!proc) {
+        KLOG_WARNING("Process struct is null pointer");
+        return -1;
+    }
+
+    uint32_t max_stack_pages = stack_size_kb_to_pages(max_stack_size_kb);
+
+    // TODO this should check against actual stack usage rather than pages
+    // allocated so far (proc->stack_page_count) - once usage is tracked,
+    // shrinking below the current allocation should free the pages that
+    // fall outside the new, smaller limit instead of being rejected outright.
+    if (max_stack_pages < proc->stack_page_count) {
+        KLOG_WARNING("Cannot set max stack size to %u KB (%u pages), %u pages are already allocated", max_stack_size_kb, max_stack_pages, proc->stack_page_count);
+        return -1;
+    }
+
+    proc->max_stack_pages = max_stack_pages;
+
+    return 0;
+}
+
 /**
  * @brief Copy `size` bytes from `buff` (kernel memory) into `count` pages of
  * the process' own address space, starting at page index `page_start`. The
@@ -881,3 +911,27 @@ void set_next_handle_id(uint32_t next) {
     next_handle_id(); // Force handle_id_set to true so it doesn't override this value
     __handle_id = next;
 }
+
+/**
+ * @brief Convert a stack size in KB to a whole number of pages, rounding up
+ * if `size_kb` is not already page aligned.
+ *
+ * Works entirely in KB (never converting to bytes) so that a large
+ * `size_kb` cannot overflow uint32_t during the conversion - multiplying by
+ * 1024 (or adding PAGE_SIZE - 1 to the result) can wrap for inputs
+ * approaching UINT32_MAX, which would silently turn a large configured
+ * limit into a much smaller (or zero) page count.
+ *
+ * @param size_kb stack size in KB
+ * @return number of pages
+ */
+static uint32_t stack_size_kb_to_pages(uint32_t size_kb) {
+    const uint32_t page_size_kb = PAGE_SIZE / 1024;
+
+    uint32_t pages = size_kb / page_size_kb;
+    if (size_kb % page_size_kb != 0) {
+        pages++;
+    }
+
+    return pages;
+}

tests/CMakeLists.txt

@@ -12,6 +12,9 @@ set(CMAKE_C_STANDARD 17)
 
 # Set the global FFF_GCC_FUNCTION_ATTRIBUTES for config.h
 set(FFF_GCC_FUNCTION_ATTRIBUTES "__attribute__((weak))")
+# Mirrors the default in the root CMakeLists.txt so process.c (which reads
+# this to seed process_t.max_stack_pages) compiles the same way under test.
+set(KERNEL_MAX_STACK_SIZE_KB 8192)
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config/config.h.in ${CMAKE_BINARY_DIR}/include/config.h)
 include_directories(PRIVATE ${CMAKE_BINARY_DIR}/include)
 

tests/config/config.h.in

@@ -3,3 +3,5 @@
  */
 
 #cmakedefine FFF_GCC_FUNCTION_ATTRIBUTES @FFF_GCC_FUNCTION_ATTRIBUTES@
+
+#define KERNEL_MAX_STACK_SIZE_KB @KERNEL_MAX_STACK_SIZE_KB@