Implement kernel stack isolation for U-mode tasks

User mode tasks require kernel stack isolation to prevent malicious or
corrupted user stack pointers from compromising kernel memory during
interrupt handling. Without this protection, a user task could set its
stack pointer to an invalid or controlled address, causing the ISR to
write trap frames to arbitrary memory locations.

This commit implements stack isolation using the mscratch register as a
discriminator between machine mode and user mode execution contexts. The
ISR entry performs a blind swap with mscratch: for machine mode tasks
(mscratch=0), the swap is immediately undone to restore the kernel stack
pointer. For user mode tasks (mscratch=kernel_stack), the swap provides
the kernel stack while preserving the user stack pointer in mscratch.

The interrupt frame structure is extended to 36 words with frame[33]
dedicated to stack pointer storage. Task initialization zeroes the
entire extended frame and correctly sets the initial stack pointer in
frame[FRAME_SP] to support the new restoration path. The FRAME_SP
enumeration constant replaces magic number usage for improved code
clarity. Additionally, FRAME_GP and FRAME_TP are used instead of array
indices for consistency.

The ISR implementation now includes separate entry and restoration paths
for each privilege mode. The M-mode path maintains mscratch=0 throughout
execution. The U-mode path saves the user stack pointer from mscratch
immediately after frame allocation and restores mscratch to the kernel
stack address before returning to user mode.

Task initialization was updated to configure mscratch appropriately
during the first dispatch. The dispatcher checks the MPP field in mstatus
and sets mscratch to zero for machine mode tasks or to the kernel stack
base for user mode tasks.

The user mode output system call was modified to bypass the asynchronous
logger queue, which could cause out-of-order output due to race
conditions between the logger task and user tasks. Direct UART output
ensures strict FIFO ordering for test output clarity.

Documentation has been updated to reflect the new 36-word interrupt frame
layout and initialization logic.

Testing validates that system calls succeed even when invoked with a
malicious stack pointer (0xDEADBEEF), confirming the ISR correctly uses
the kernel stack from mscratch rather than the user-controlled stack
pointer.

Author: HeatCrab

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?
@@ -129,7 +129,7 @@ Why full context save in ISR?
 
 Each task stack must reserve 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.

Documentation/hal-riscv-context-switch.md

@@ -130,7 +130,7 @@ void *hal_build_initial_frame(void *stack_top,
                                     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 */

app/umode.c

@@ -1,43 +1,73 @@
 #include <linmo.h>
 
-/* U-mode Validation Task
+/* 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.
+ * Phase 1: Verify syscalls work under various SP conditions (normal,
+ * malicious). Phase 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("[umode] Phase 1: Testing Kernel Stack Isolation\n");
+    umode_printf("\n");
 
-    /* Test 1: sys_tid() - Simplest read-only syscall. */
+    /* Test 1a: Baseline - Syscall with normal SP */
+    umode_printf("[umode] Test 1a: 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);
     } else {
         umode_printf("[umode] FAIL: sys_tid() failed (ret=%d)\n", my_tid);
     }
+    umode_printf("\n");
 
-    /* Test 2: sys_uptime() - Verify value transmission is correct. */
+    /* Test 1b: Verify ISR uses mscratch, not malicious user SP */
+    umode_printf("[umode] Test 1b: sys_tid() with malicious SP\n");
+
+    uint32_t saved_sp;
+    asm volatile(
+        "mv %0, sp          \n"
+        "li sp, 0xDEADBEEF  \n"
+        : "=r"(saved_sp));
+
+    int my_tid_bad_sp = sys_tid();
+
+    asm volatile("mv sp, %0          \n" : : "r"(saved_sp));
+
+    if (my_tid_bad_sp > 0) {
+        umode_printf(
+            "[umode] PASS: sys_tid() succeeded, ISR correctly used kernel "
+            "stack\n");
+    } else {
+        umode_printf(
+            "[umode] FAIL: Syscall failed with malicious SP (ret=%d)\n",
+            my_tid_bad_sp);
+    }
+    umode_printf("\n");
+
+    /* Test 1c: Verify syscall functionality is still intact */
+    umode_printf("[umode] Test 1c: sys_uptime() with normal SP\n");
     int uptime = sys_uptime();
     if (uptime >= 0) {
         umode_printf("[umode] PASS: sys_uptime() returned %d\n", uptime);
     } else {
         umode_printf("[umode] FAIL: sys_uptime() failed (ret=%d)\n", uptime);
     }
+    umode_printf("\n");
 
-    /* Note: Skipping sys_tadd for now, as kernel user pointer checks might
-     * block function pointers in the .text segment, avoiding distraction.
-     */
+    umode_printf(
+        "[umode] Phase 1 Complete: Kernel stack isolation validated\n");
+    umode_printf("\n");
 
     /* --- Phase 2: Security Check (Privileged Access) --- */
     umode_printf("[umode] ========================================\n");
+    umode_printf("\n");
     umode_printf("[umode] Phase 2: Testing Security Isolation\n");
+    umode_printf("\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");
+    umode_printf("\n");
 
     /* CRITICAL: Delay before suicide to ensure logs are flushed from
      * buffer to UART.

arch/riscv/boot.c

@@ -93,35 +93,32 @@ __attribute__((naked, section(".text.prologue"))) void _entry(void)
         : "memory");
 }
 
-/* Size of the full trap context frame saved on the stack by the ISR.
- * 30 GPRs (x1, x3-x31) + mcause + mepc + mstatus = 33 words * 4 bytes = 132
- * bytes. Round up to 144 bytes for 16-byte alignment.
+/* ISR trap frame layout (144 bytes = 36 words).
+ * [0-29]: GPRs (ra, gp, tp, t0-t6, s0-s11, a0-a7)
+ * [30]: mcause
+ * [31]: mepc
+ * [32]: mstatus
+ * [33]: SP (user SP in U-mode, original SP in M-mode)
  */
 #define ISR_CONTEXT_SIZE 144
 
-/* Low-level Interrupt Service Routine (ISR) trampoline.
- *
- * This is the common entry point for all traps. It performs a FULL context
- * save, creating a complete trap frame on the stack. This makes the C handler
- * robust, as it does not need to preserve any registers itself.
- */
+/* Low-level ISR common entry for all traps with full context save */
 __attribute__((naked, aligned(4))) void _isr(void)
 {
     asm volatile(
-        /* Allocate stack frame for full context save */
-        "addi   sp, sp, -%0\n"
-
-        /* Save all general-purpose registers except x0 (zero) and x2 (sp).
-         * This includes caller-saved and callee-saved registers.
-         *
-         * Stack Frame Layout (offsets from sp in bytes):
-         *   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
+        /* Blind swap with mscratch for kernel stack isolation.
+         * Convention: M-mode (mscratch=0, SP=kernel), U-mode (mscratch=kernel,
+         * SP=user). After swap: if SP != 0 came from U-mode, else M-mode.
          */
+        "csrrw  sp, mscratch, sp\n"
+        "bnez   sp, .Lumode_entry\n"
+
+        /* Undo swap and continue for M-mode */
+        "csrrw  sp, mscratch, sp\n"
+
+        "addi   sp, sp, -144\n"
+
+        /* Save all GPRs and CSRs */
         "sw  ra,   0*4(sp)\n"
         "sw  gp,   1*4(sp)\n"
         "sw  tp,   2*4(sp)\n"
@@ -153,33 +150,149 @@ __attribute__((naked, aligned(4))) void _isr(void)
         "sw  t5,  28*4(sp)\n"
         "sw  t6,  29*4(sp)\n"
 
-        /* Save trap-related CSRs and prepare arguments for do_trap */
+        /* Save original SP before frame allocation */
+        "addi   t0, sp, 144\n"
+        "sw     t0, 33*4(sp)\n"
+
+        /* Save machine CSRs (mcause, mepc, mstatus) */
         "csrr   a0, mcause\n"
         "csrr   a1, mepc\n"
-        "csrr   a2, mstatus\n" /* For context switching in privilege change */
-
+        "csrr   a2, mstatus\n"
         "sw     a0,  30*4(sp)\n"
         "sw     a1,  31*4(sp)\n"
         "sw     a2,  32*4(sp)\n"
 
-        "mv     a2, sp\n" /* a2 = isr_sp */
-
-        /* Call the high-level C trap handler.
-         * Returns: a0 = SP to use for restoring context (may be different
-         * task's stack if context switch occurred).
-         */
+        /* Call trap handler with frame pointer */
+        "mv     a2, sp\n"
         "call   do_trap\n"
+        "mv     sp, a0\n"
+
+        /* Load mstatus and extract MPP to determine M-mode or U-mode return
+           path */
+        "lw     t0, 32*4(sp)\n"
+        "csrw   mstatus, t0\n"
+
+        "srli   t1, t0, 11\n"
+        "andi   t1, t1, 0x3\n"
+        "beqz   t1, .Lrestore_umode\n"
+
+        /* M-mode restore */
+        ".Lrestore_mmode:\n"
+        "csrw   mscratch, zero\n"
+
+        "lw     t1, 31*4(sp)\n" /* Restore mepc */
+        "csrw   mepc, t1\n"
+
+        /* Restore all GPRs */
+        "lw  ra,   0*4(sp)\n"
+        "lw  gp,   1*4(sp)\n"
+        "lw  tp,   2*4(sp)\n"
+        "lw  t0,   3*4(sp)\n"
+        "lw  t1,   4*4(sp)\n"
+        "lw  t2,   5*4(sp)\n"
+        "lw  s0,   6*4(sp)\n"
+        "lw  s1,   7*4(sp)\n"
+        "lw  a0,   8*4(sp)\n"
+        "lw  a1,   9*4(sp)\n"
+        "lw  a2,  10*4(sp)\n"
+        "lw  a3,  11*4(sp)\n"
+        "lw  a4,  12*4(sp)\n"
+        "lw  a5,  13*4(sp)\n"
+        "lw  a6,  14*4(sp)\n"
+        "lw  a7,  15*4(sp)\n"
+        "lw  s2,  16*4(sp)\n"
+        "lw  s3,  17*4(sp)\n"
+        "lw  s4,  18*4(sp)\n"
+        "lw  s5,  19*4(sp)\n"
+        "lw  s6,  20*4(sp)\n"
+        "lw  s7,  21*4(sp)\n"
+        "lw  s8,  22*4(sp)\n"
+        "lw  s9,  23*4(sp)\n"
+        "lw  s10, 24*4(sp)\n"
+        "lw  s11, 25*4(sp)\n"
+        "lw  t3,  26*4(sp)\n"
+        "lw  t4,  27*4(sp)\n"
+        "lw  t5,  28*4(sp)\n"
+        "lw  t6,  29*4(sp)\n"
 
-        /* Use returned SP for context restore (enables context switching) */
+        /* Restore SP from frame[33] */
+        "lw  sp,  33*4(sp)\n"
+
+        /* Return from trap */
+        "mret\n"
+
+        /* U-mode entry receives kernel stack in SP and user SP in mscratch */
+        ".Lumode_entry:\n"
+        "addi   sp, sp, -144\n"
+
+        /* Save t6 first to preserve it before using it as scratch */
+        "sw     t6, 29*4(sp)\n"
+
+        /* Retrieve user SP from mscratch into t6 and save it */
+        "csrr   t6, mscratch\n"
+        "sw     t6, 33*4(sp)\n"
+
+        /* Save remaining GPRs */
+        "sw  ra,   0*4(sp)\n"
+        "sw  gp,   1*4(sp)\n"
+        "sw  tp,   2*4(sp)\n"
+        "sw  t0,   3*4(sp)\n"
+        "sw  t1,   4*4(sp)\n"
+        "sw  t2,   5*4(sp)\n"
+        "sw  s0,   6*4(sp)\n"
+        "sw  s1,   7*4(sp)\n"
+        "sw  a0,   8*4(sp)\n"
+        "sw  a1,   9*4(sp)\n"
+        "sw  a2,  10*4(sp)\n"
+        "sw  a3,  11*4(sp)\n"
+        "sw  a4,  12*4(sp)\n"
+        "sw  a5,  13*4(sp)\n"
+        "sw  a6,  14*4(sp)\n"
+        "sw  a7,  15*4(sp)\n"
+        "sw  s2,  16*4(sp)\n"
+        "sw  s3,  17*4(sp)\n"
+        "sw  s4,  18*4(sp)\n"
+        "sw  s5,  19*4(sp)\n"
+        "sw  s6,  20*4(sp)\n"
+        "sw  s7,  21*4(sp)\n"
+        "sw  s8,  22*4(sp)\n"
+        "sw  s9,  23*4(sp)\n"
+        "sw  s10, 24*4(sp)\n"
+        "sw  s11, 25*4(sp)\n"
+        "sw  t3,  26*4(sp)\n"
+        "sw  t4,  27*4(sp)\n"
+        "sw  t5,  28*4(sp)\n"
+        /* t6 already saved */
+
+        /* Save CSRs */
+        "csrr   a0, mcause\n"
+        "csrr   a1, mepc\n"
+        "csrr   a2, mstatus\n"
+        "sw     a0,  30*4(sp)\n"
+        "sw     a1,  31*4(sp)\n"
+        "sw     a2,  32*4(sp)\n"
+
+        "mv     a2, sp\n" /* a2 = ISR frame pointer */
+        "call   do_trap\n"
         "mv     sp, a0\n"
 
-        /* Restore mstatus from frame[32] */
+        /* Check MPP in mstatus to determine return path */
         "lw     t0, 32*4(sp)\n"
         "csrw   mstatus, t0\n"
 
-        /* Restore mepc from frame[31] (might have been modified by handler) */
+        "srli   t1, t0, 11\n"
+        "andi   t1, t1, 0x3\n"
+        "bnez   t1, .Lrestore_mmode\n"
+
+        /* Setup mscratch for U-mode restore to prepare for next trap */
+        ".Lrestore_umode:\n"
+        "la     t1, _stack\n"
+        "csrw   mscratch, t1\n"
+
         "lw     t1, 31*4(sp)\n"
         "csrw   mepc, t1\n"
+
+        /* Restore all GPRs */
         "lw  ra,   0*4(sp)\n"
         "lw  gp,   1*4(sp)\n"
         "lw  tp,   2*4(sp)\n"
@@ -211,12 +324,12 @@ __attribute__((naked, aligned(4))) void _isr(void)
         "lw  t5,  28*4(sp)\n"
         "lw  t6,  29*4(sp)\n"
 
-        /* Deallocate stack frame */
-        "addi   sp, sp, %0\n"
+        /* Restore user SP from frame[33] */
+        "lw  sp,  33*4(sp)\n"
 
         /* Return from trap */
         "mret\n"
-        :                       /* no outputs */
-        : "i"(ISR_CONTEXT_SIZE) /* +16 for mcause, mepc, mstatus */
+        : /* no outputs */
+        : /* no inputs, ISR_CONTEXT_SIZE used inline */
         : "memory");
 }