Merge pull request #62 from HeatCrab/u-mode/basic-support

Implement kernel stack isolation for U-mode tasks

Author: jserv

Documentation/hal-calling-convention.md

@@ -109,14 +109,14 @@ void hal_context_restore(jmp_buf env, int32_t val); /* Restore context + process
 The ISR in `boot.c` performs a complete context save of all registers:
 
 ```
-Stack Frame Layout (144 bytes, 33 words × 4 bytes, offsets from sp):
+Stack Frame Layout (144 bytes, 36 words × 4 bytes, offsets from sp):
   0: ra,   4: gp,   8: tp,  12: t0,  16: t1,  20: t2
  24: s0,  28: s1,  32: a0,  36: a1,  40: a2,  44: a3
  48: a4,  52: a5,  56: a6,  60: a7,  64: s2,  68: s3
  72: s4,  76: s5,  80: s6,  84: s7,  88: s8,  92: s9
  96: s10, 100:s11, 104:t3, 108: t4, 112: t5, 116: t6
-120: mcause, 124: mepc, 128: mstatus
-132-143: padding (12 bytes for 16-byte alignment)
+120: mcause, 124: mepc, 128: mstatus, 132: sp (for restore)
+136-143: padding (8 bytes for 16-byte alignment)
 ```
 
 Why full context save in ISR?
@@ -127,12 +127,14 @@ Why full context save in ISR?
 
 ### ISR Stack Requirements
 
-Each task stack must reserve space for the ISR frame:
+Each task requires space for the ISR frame:
 ```c
-#define ISR_STACK_FRAME_SIZE 144  /* 33 words × 4 bytes, 16-byte aligned */
+#define ISR_STACK_FRAME_SIZE 144  /* 36 words × 4 bytes, 16-byte aligned */
 ```
 
-This "red zone" is reserved at the top of every task stack to guarantee ISR safety.
+**M-mode tasks**: This "red zone" is reserved at the top of the task stack to guarantee ISR safety.
+
+**U-mode tasks**: The ISR frame is allocated on the per-task kernel stack (1024 bytes), not on the user stack. This provides stack isolation and prevents user tasks from corrupting kernel trap handling state.
 
 ## Function Calling in Linmo
 
@@ -200,7 +202,9 @@ void task_function(void) {
 
 ### Stack Layout
 
-Each task has its own stack with this layout:
+#### Machine Mode Tasks
+
+Each M-mode task has its own stack with this layout:
 
 ```
 High Address
@@ -216,6 +220,43 @@ High Address
 Low Address
 ```
 
+#### User Mode Tasks (Per-Task Kernel Stack)
+
+U-mode tasks maintain separate user and kernel stacks for isolation:
+
+**User Stack** (application execution):
+```
+High Address
++------------------+ <- user_stack_base + user_stack_size
+|                  |
+| User Stack       | <- Grows downward
+| (Dynamic)        | <- Task executes here in U-mode
+|                  |
++------------------+ <- user_stack_base
+Low Address
+```
+
+**Kernel Stack** (trap handling):
+```
+High Address
++------------------+ <- kernel_stack_base + kernel_stack_size (1024 bytes)
+| ISR Frame        | <- 144 bytes for trap context
+| (144 bytes)      | <- Traps switch to this stack
++------------------+
+| Trap Handler     | <- Kernel code execution during traps
+| Stack Space      |
++------------------+ <- kernel_stack_base
+Low Address
+```
+
+When a U-mode task enters a trap (syscall, interrupt, exception):
+1. ISR swaps SP with `mscratch` via `csrrw` (mscratch contains kernel stack top)
+2. ISR saves full context to kernel stack
+3. Trap handler executes on kernel stack
+4. Return path restores user SP and switches back
+
+This dual-stack design prevents user tasks from corrupting kernel state and provides strong isolation between privilege levels.
+
 ### Stack Alignment
 - 16-byte alignment: Required by RISC-V ABI for stack pointer
 - 4-byte alignment: Minimum for all memory accesses on RV32I

Documentation/hal-riscv-context-switch.md

@@ -123,14 +123,26 @@ a complete interrupt service routine frame:
 ```c
 void *hal_build_initial_frame(void *stack_top,
                               void (*task_entry)(void),
-                              int user_mode)
+                              int user_mode,
+                              void *kernel_stack,
+                              size_t kernel_stack_size)
 {
-    /* Place frame in stack with initial reserve below for proper startup */
-    uint32_t *frame = (uint32_t *) ((uint8_t *) stack_top - 256 -
-                                    ISR_STACK_FRAME_SIZE);
+    /* For U-mode tasks, build frame on kernel stack for stack isolation.
+     * For M-mode tasks, build frame on user stack as before.
+     */
+    uint32_t *frame;
+    if (user_mode && kernel_stack) {
+        /* U-mode: Place frame on per-task kernel stack */
+        void *kstack_top = (uint8_t *) kernel_stack + kernel_stack_size;
+        frame = (uint32_t *) ((uint8_t *) kstack_top - ISR_STACK_FRAME_SIZE);
+    } else {
+        /* M-mode: Place frame on user stack with reserve below */
+        frame = (uint32_t *) ((uint8_t *) stack_top - 256 -
+                              ISR_STACK_FRAME_SIZE);
+    }
 
     /* Initialize all general purpose registers to zero */
-    for (int i = 0; i < 32; i++)
+    for (int i = 0; i < 36; i++)
         frame[i] = 0;
 
     /* Compute thread pointer: aligned to 64 bytes from _end */
@@ -152,6 +164,18 @@ void *hal_build_initial_frame(void *stack_top,
     /* Set entry point */
     frame[FRAME_EPC] = (uint32_t) task_entry;
 
+    /* SP value for when ISR returns (stored in frame[33]).
+     * For U-mode: Set to user stack top.
+     * For M-mode: Set to frame + ISR_STACK_FRAME_SIZE.
+     */
+    if (user_mode && kernel_stack) {
+        /* U-mode: frame[33] should contain user SP */
+        frame[FRAME_SP] = (uint32_t) ((uint8_t *) stack_top - 256);
+    } else {
+        /* M-mode: frame[33] contains kernel SP after frame deallocation */
+        frame[FRAME_SP] = (uint32_t) ((uint8_t *) frame + ISR_STACK_FRAME_SIZE);
+    }
+
     return frame;  /* Return frame base as initial stack pointer */
 }
 ```

app/umode.c

@@ -1,59 +1,84 @@
 #include <linmo.h>
 
-/* U-mode Validation Task
+/* Architecture-specific helper for SP manipulation testing.
+ * Implemented in arch/riscv/entry.c as a naked function.
+ */
+extern uint32_t __switch_sp(uint32_t new_sp);
+
+/* U-mode validation: syscall stability and privilege isolation.
  *
- * Integrates two tests into a single task flow to ensure sequential execution:
- * 1. Phase 1: Mechanism Check - Verify syscalls work.
- * 2. Phase 2: Security Check - Verify privileged instructions trigger a trap.
+ * 1. Verify syscalls work under various SP conditions (normal, malicious).
+ * 2. Verify privileged instructions trap.
  */
 void umode_validation_task(void)
 {
-    /* --- Phase 1: Mechanism Check (Syscalls) --- */
-    umode_printf("[umode] Phase 1: Testing Syscall Mechanism\n");
+    /* --- Phase 1: Kernel Stack Isolation Test --- */
+    umode_printf("Phase 1: Testing Kernel Stack Isolation\n");
+    umode_printf("\n");
 
-    /* Test 1: sys_tid() - Simplest read-only syscall. */
+    /* Test 1-1: Baseline - Syscall with normal SP */
+    umode_printf("Test 1-1: sys_tid() with normal SP\n");
     int my_tid = sys_tid();
     if (my_tid > 0) {
-        umode_printf("[umode] PASS: sys_tid() returned %d\n", my_tid);
+        umode_printf("PASS: sys_tid() returned %d\n", my_tid);
     } else {
-        umode_printf("[umode] FAIL: sys_tid() failed (ret=%d)\n", my_tid);
+        umode_printf("FAIL: sys_tid() failed (ret=%d)\n", my_tid);
     }
+    umode_printf("\n");
+
+    /* Test 1-2: Verify ISR uses mscratch, not malicious user SP */
+    umode_printf("Test 1-2: sys_tid() with malicious SP\n");
 
-    /* Test 2: sys_uptime() - Verify value transmission is correct. */
+    uint32_t saved_sp = __switch_sp(0xDEADBEEF);
+    int my_tid_bad_sp = sys_tid();
+    __switch_sp(saved_sp);
+
+    if (my_tid_bad_sp > 0) {
+        umode_printf(
+            "PASS: sys_tid() succeeded, ISR correctly used kernel "
+            "stack\n");
+    } else {
+        umode_printf("FAIL: Syscall failed with malicious SP (ret=%d)\n",
+                     my_tid_bad_sp);
+    }
+    umode_printf("\n");
+
+    /* Test 1-3: Verify syscall functionality is still intact */
+    umode_printf("Test 1-3: sys_uptime() with normal SP\n");
     int uptime = sys_uptime();
     if (uptime >= 0) {
-        umode_printf("[umode] PASS: sys_uptime() returned %d\n", uptime);
+        umode_printf("PASS: sys_uptime() returned %d\n", uptime);
     } else {
-        umode_printf("[umode] FAIL: sys_uptime() failed (ret=%d)\n", uptime);
+        umode_printf("FAIL: sys_uptime() failed (ret=%d)\n", uptime);
     }
+    umode_printf("\n");
 
-    /* Note: Skipping sys_task_spawn for now, as kernel user pointer checks
-     * might block function pointers in the .text segment, avoiding distraction.
-     */
+    umode_printf("Phase 1 All tests passed.\n");
+    umode_printf("\n");
 
     /* --- Phase 2: Security Check (Privileged Access) --- */
-    umode_printf("[umode] ========================================\n");
-    umode_printf("[umode] Phase 2: Testing Security Isolation\n");
-    umode_printf(
-        "[umode] Action: Attempting to read 'mstatus' CSR from U-mode.\n");
-    umode_printf("[umode] Expect: Kernel Panic with 'Illegal instruction'.\n");
-    umode_printf("[umode] ========================================\n");
-
-    /* CRITICAL: Delay before suicide to ensure logs are flushed from
+    umode_printf("========================================\n");
+    umode_printf("\n");
+    umode_printf("Phase 2: Testing Security Isolation\n");
+    umode_printf("\n");
+    umode_printf("Action: Attempting to read 'mstatus' CSR from U-mode.\n");
+    umode_printf("Expect: Kernel Panic with 'Illegal instruction'.\n");
+    umode_printf("\n");
+    /* Delay before suicide to ensure logs are flushed from
      * buffer to UART.
      */
     sys_tdelay(10);
 
     /* Privileged Instruction Trigger */
+    umode_printf("Result: \n");
     uint32_t mstatus;
     asm volatile("csrr %0, mstatus" : "=r"(mstatus));
 
     /* If execution reaches here, U-mode isolation failed (still has
      * privileges).
      */
-    umode_printf(
-        "[umode] FAIL: Privileged instruction executed! (mstatus=0x%lx)\n",
-        (long) mstatus);
+    umode_printf("FAIL: Privileged instruction executed! (mstatus=0x%lx)\n",
+                 (long) mstatus);
 
     /* Spin loop to prevent further execution. */
     while (1)
@@ -62,7 +87,7 @@ void umode_validation_task(void)
 
 int32_t app_main(void)
 {
-    umode_printf("[Kernel] Spawning U-mode validation task...\n");
+    umode_printf("Spawning U-mode validation task...\n");
 
     /* app_main now runs in U-mode by default.
      * mo_task_spawn routes to sys_task_spawn syscall for U-mode apps,