Skip to content

Commit e31f722

Browse files
HeatCrabHeatCrab
committed
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 include dedicated storage for the stack pointer. Task initialization zeroes the entire frame and correctly sets the initial stack pointer to support the new restoration path. Enumeration constants replace magic number usage for improved code clarity and 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 current privilege level 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 and implement task-level synchronization. Direct output ensures strict FIFO ordering for test output clarity, while preventing task preemption during character transmission avoids interleaving when multiple user tasks print concurrently. This ensures each string is output atomically with respect to other tasks. A test helper function was added to support stack pointer manipulation during validation. Following the Linux kernel's __switch_to pattern for context switching, this provides precise control over stack operations without compiler interference. The validation harness uses this to verify syscall stability under corrupted stack pointer conditions. Documentation has been updated to reflect the new 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.
1 parent 8c60804 commit e31f722

File tree

8 files changed

+296
-100
lines changed

8 files changed

+296
-100
lines changed

‎Documentation/hal-calling-convention.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,14 @@ void hal_context_restore(jmp_buf env, int32_t val); /* Restore context + process
109109
The ISR in `boot.c` performs a complete context save of all registers:
110110

111111
```
112-
Stack Frame Layout (144 bytes, 33 words × 4 bytes, offsets from sp):
112+
Stack Frame Layout (144 bytes, 36 words × 4 bytes, offsets from sp):
113113
0: ra, 4: gp, 8: tp, 12: t0, 16: t1, 20: t2
114114
24: s0, 28: s1, 32: a0, 36: a1, 40: a2, 44: a3
115115
48: a4, 52: a5, 56: a6, 60: a7, 64: s2, 68: s3
116116
72: s4, 76: s5, 80: s6, 84: s7, 88: s8, 92: s9
117117
96: s10, 100:s11, 104:t3, 108: t4, 112: t5, 116: t6
118-
120: mcause, 124: mepc, 128: mstatus
119-
132-143: padding (12 bytes for 16-byte alignment)
118+
120: mcause, 124: mepc, 128: mstatus, 132: sp (for restore)
119+
136-143: padding (8 bytes for 16-byte alignment)
120120
```
121121

122122
Why full context save in ISR?
@@ -129,7 +129,7 @@ Why full context save in ISR?
129129

130130
Each task stack must reserve space for the ISR frame:
131131
```c
132-
#define ISR_STACK_FRAME_SIZE 144 /* 33 words × 4 bytes, 16-byte aligned */
132+
#define ISR_STACK_FRAME_SIZE 144 /* 36 words × 4 bytes, 16-byte aligned */
133133
```
134134
135135
This "red zone" is reserved at the top of every task stack to guarantee ISR safety.

‎Documentation/hal-riscv-context-switch.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ void *hal_build_initial_frame(void *stack_top,
130130
ISR_STACK_FRAME_SIZE);
131131
132132
/* Initialize all general purpose registers to zero */
133-
for (int i = 0; i < 32; i++)
133+
for (int i = 0; i < 36; i++)
134134
frame[i] = 0;
135135
136136
/* Compute thread pointer: aligned to 64 bytes from _end */

‎app/umode.c‎

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,72 @@
11
#include <linmo.h>
22

3-
/* U-mode Validation Task
3+
/* Architecture-specific helper for SP manipulation testing.
4+
* Implemented in arch/riscv/entry.c as a naked function.
5+
*/
6+
extern uint32_t __switch_sp(uint32_t new_sp);
7+
8+
/* U-mode validation: syscall stability and privilege isolation.
49
*
5-
* Integrates two tests into a single task flow to ensure sequential execution:
6-
* 1. Phase 1: Mechanism Check - Verify syscalls work.
7-
* 2. Phase 2: Security Check - Verify privileged instructions trigger a trap.
10+
* Phase 1: Verify syscalls work under various SP conditions (normal,
11+
* malicious). Phase 2: Verify privileged instructions trap.
812
*/
913
void umode_validation_task(void)
1014
{
11-
/* --- Phase 1: Mechanism Check (Syscalls) --- */
12-
umode_printf("[umode] Phase 1: Testing Syscall Mechanism\n");
15+
/* --- Phase 1: Kernel Stack Isolation Test --- */
16+
umode_printf("[umode] Phase 1: Testing Kernel Stack Isolation\n");
17+
umode_printf("\n");
1318

14-
/* Test 1: sys_tid() - Simplest read-only syscall. */
19+
/* Test 1a: Baseline - Syscall with normal SP */
20+
umode_printf("[umode] Test 1a: sys_tid() with normal SP\n");
1521
int my_tid = sys_tid();
1622
if (my_tid > 0) {
1723
umode_printf("[umode] PASS: sys_tid() returned %d\n", my_tid);
1824
} else {
1925
umode_printf("[umode] FAIL: sys_tid() failed (ret=%d)\n", my_tid);
2026
}
27+
umode_printf("\n");
28+
29+
/* Test 1b: Verify ISR uses mscratch, not malicious user SP */
30+
umode_printf("[umode] Test 1b: sys_tid() with malicious SP\n");
31+
32+
uint32_t saved_sp = __switch_sp(0xDEADBEEF);
33+
int my_tid_bad_sp = sys_tid();
34+
__switch_sp(saved_sp);
35+
36+
if (my_tid_bad_sp > 0) {
37+
umode_printf(
38+
"[umode] PASS: sys_tid() succeeded, ISR correctly used kernel "
39+
"stack\n");
40+
} else {
41+
umode_printf(
42+
"[umode] FAIL: Syscall failed with malicious SP (ret=%d)\n",
43+
my_tid_bad_sp);
44+
}
45+
umode_printf("\n");
2146

22-
/* Test 2: sys_uptime() - Verify value transmission is correct. */
47+
/* Test 1c: Verify syscall functionality is still intact */
48+
umode_printf("[umode] Test 1c: sys_uptime() with normal SP\n");
2349
int uptime = sys_uptime();
2450
if (uptime >= 0) {
2551
umode_printf("[umode] PASS: sys_uptime() returned %d\n", uptime);
2652
} else {
2753
umode_printf("[umode] FAIL: sys_uptime() failed (ret=%d)\n", uptime);
2854
}
55+
umode_printf("\n");
2956

30-
/* Note: Skipping sys_tadd for now, as kernel user pointer checks might
31-
* block function pointers in the .text segment, avoiding distraction.
32-
*/
57+
umode_printf(
58+
"[umode] Phase 1 Complete: Kernel stack isolation validated\n");
59+
umode_printf("\n");
3360

3461
/* --- Phase 2: Security Check (Privileged Access) --- */
3562
umode_printf("[umode] ========================================\n");
63+
umode_printf("\n");
3664
umode_printf("[umode] Phase 2: Testing Security Isolation\n");
65+
umode_printf("\n");
3766
umode_printf(
3867
"[umode] Action: Attempting to read 'mstatus' CSR from U-mode.\n");
3968
umode_printf("[umode] Expect: Kernel Panic with 'Illegal instruction'.\n");
40-
umode_printf("[umode] ========================================\n");
69+
umode_printf("\n");
4170

4271
/* CRITICAL: Delay before suicide to ensure logs are flushed from
4372
* buffer to UART.

‎arch/riscv/boot.c‎

Lines changed: 150 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -93,35 +93,32 @@ __attribute__((naked, section(".text.prologue"))) void _entry(void)
9393
: "memory");
9494
}
9595

96-
/* Size of the full trap context frame saved on the stack by the ISR.
97-
* 30 GPRs (x1, x3-x31) + mcause + mepc + mstatus = 33 words * 4 bytes = 132
98-
* bytes. Round up to 144 bytes for 16-byte alignment.
96+
/* ISR trap frame layout (144 bytes = 36 words).
97+
* [0-29]: GPRs (ra, gp, tp, t0-t6, s0-s11, a0-a7)
98+
* [30]: mcause
99+
* [31]: mepc
100+
* [32]: mstatus
101+
* [33]: SP (user SP in U-mode, original SP in M-mode)
99102
*/
100103
#define ISR_CONTEXT_SIZE 144
101104

102-
/* Low-level Interrupt Service Routine (ISR) trampoline.
103-
*
104-
* This is the common entry point for all traps. It performs a FULL context
105-
* save, creating a complete trap frame on the stack. This makes the C handler
106-
* robust, as it does not need to preserve any registers itself.
107-
*/
105+
/* Low-level ISR common entry for all traps with full context save */
108106
__attribute__((naked, aligned(4))) void _isr(void)
109107
{
110108
asm volatile(
111-
/* Allocate stack frame for full context save */
109+
/* Blind swap with mscratch for kernel stack isolation.
110+
* Convention: M-mode (mscratch=0, SP=kernel), U-mode (mscratch=kernel,
111+
* SP=user). After swap: if SP != 0 came from U-mode, else M-mode.
112+
*/
113+
"csrrw sp, mscratch, sp\n"
114+
"bnez sp, .Lumode_entry\n"
115+
116+
/* Undo swap and continue for M-mode */
117+
"csrrw sp, mscratch, sp\n"
118+
112119
"addi sp, sp, -%0\n"
113120

114-
/* Save all general-purpose registers except x0 (zero) and x2 (sp).
115-
* This includes caller-saved and callee-saved registers.
116-
*
117-
* Stack Frame Layout (offsets from sp in bytes):
118-
* 0: ra, 4: gp, 8: tp, 12: t0, 16: t1, 20: t2
119-
* 24: s0, 28: s1, 32: a0, 36: a1, 40: a2, 44: a3
120-
* 48: a4, 52: a5, 56: a6, 60: a7, 64: s2, 68: s3
121-
* 72: s4, 76: s5, 80: s6, 84: s7, 88: s8, 92: s9
122-
* 96: s10, 100:s11, 104:t3, 108: t4, 112: t5, 116: t6
123-
* 120: mcause, 124: mepc
124-
*/
121+
/* Save all GPRs and CSRs */
125122
"sw ra, 0*4(sp)\n"
126123
"sw gp, 1*4(sp)\n"
127124
"sw tp, 2*4(sp)\n"
@@ -153,33 +150,149 @@ __attribute__((naked, aligned(4))) void _isr(void)
153150
"sw t5, 28*4(sp)\n"
154151
"sw t6, 29*4(sp)\n"
155152

156-
/* Save trap-related CSRs and prepare arguments for do_trap */
153+
/* Save original SP before frame allocation */
154+
"addi t0, sp, %0\n"
155+
"sw t0, 33*4(sp)\n"
156+
157+
/* Save machine CSRs (mcause, mepc, mstatus) */
157158
"csrr a0, mcause\n"
158159
"csrr a1, mepc\n"
159-
"csrr a2, mstatus\n" /* For context switching in privilege change */
160-
160+
"csrr a2, mstatus\n"
161161
"sw a0, 30*4(sp)\n"
162162
"sw a1, 31*4(sp)\n"
163163
"sw a2, 32*4(sp)\n"
164164

165-
"mv a2, sp\n" /* a2 = isr_sp */
166-
167-
/* Call the high-level C trap handler.
168-
* Returns: a0 = SP to use for restoring context (may be different
169-
* task's stack if context switch occurred).
170-
*/
165+
/* Call trap handler with frame pointer */
166+
"mv a2, sp\n"
171167
"call do_trap\n"
168+
"mv sp, a0\n"
169+
170+
/* Load mstatus and extract MPP to determine M-mode or U-mode return
171+
path */
172+
"lw t0, 32*4(sp)\n"
173+
"csrw mstatus, t0\n"
174+
175+
"srli t1, t0, 11\n"
176+
"andi t1, t1, 0x3\n"
177+
"beqz t1, .Lrestore_umode\n"
178+
179+
/* M-mode restore */
180+
".Lrestore_mmode:\n"
181+
"csrw mscratch, zero\n"
182+
183+
"lw t1, 31*4(sp)\n" /* Restore mepc */
184+
"csrw mepc, t1\n"
185+
186+
/* Restore all GPRs */
187+
"lw ra, 0*4(sp)\n"
188+
"lw gp, 1*4(sp)\n"
189+
"lw tp, 2*4(sp)\n"
190+
"lw t0, 3*4(sp)\n"
191+
"lw t1, 4*4(sp)\n"
192+
"lw t2, 5*4(sp)\n"
193+
"lw s0, 6*4(sp)\n"
194+
"lw s1, 7*4(sp)\n"
195+
"lw a0, 8*4(sp)\n"
196+
"lw a1, 9*4(sp)\n"
197+
"lw a2, 10*4(sp)\n"
198+
"lw a3, 11*4(sp)\n"
199+
"lw a4, 12*4(sp)\n"
200+
"lw a5, 13*4(sp)\n"
201+
"lw a6, 14*4(sp)\n"
202+
"lw a7, 15*4(sp)\n"
203+
"lw s2, 16*4(sp)\n"
204+
"lw s3, 17*4(sp)\n"
205+
"lw s4, 18*4(sp)\n"
206+
"lw s5, 19*4(sp)\n"
207+
"lw s6, 20*4(sp)\n"
208+
"lw s7, 21*4(sp)\n"
209+
"lw s8, 22*4(sp)\n"
210+
"lw s9, 23*4(sp)\n"
211+
"lw s10, 24*4(sp)\n"
212+
"lw s11, 25*4(sp)\n"
213+
"lw t3, 26*4(sp)\n"
214+
"lw t4, 27*4(sp)\n"
215+
"lw t5, 28*4(sp)\n"
216+
"lw t6, 29*4(sp)\n"
172217

173-
/* Use returned SP for context restore (enables context switching) */
218+
/* Restore SP from frame[33] */
219+
"lw sp, 33*4(sp)\n"
220+
221+
/* Return from trap */
222+
"mret\n"
223+
224+
/* U-mode entry receives kernel stack in SP and user SP in mscratch */
225+
".Lumode_entry:\n"
226+
"addi sp, sp, -%0\n"
227+
228+
/* Save t6 first to preserve it before using it as scratch */
229+
"sw t6, 29*4(sp)\n"
230+
231+
/* Retrieve user SP from mscratch into t6 and save it */
232+
"csrr t6, mscratch\n"
233+
"sw t6, 33*4(sp)\n"
234+
235+
/* Save remaining GPRs */
236+
"sw ra, 0*4(sp)\n"
237+
"sw gp, 1*4(sp)\n"
238+
"sw tp, 2*4(sp)\n"
239+
"sw t0, 3*4(sp)\n"
240+
"sw t1, 4*4(sp)\n"
241+
"sw t2, 5*4(sp)\n"
242+
"sw s0, 6*4(sp)\n"
243+
"sw s1, 7*4(sp)\n"
244+
"sw a0, 8*4(sp)\n"
245+
"sw a1, 9*4(sp)\n"
246+
"sw a2, 10*4(sp)\n"
247+
"sw a3, 11*4(sp)\n"
248+
"sw a4, 12*4(sp)\n"
249+
"sw a5, 13*4(sp)\n"
250+
"sw a6, 14*4(sp)\n"
251+
"sw a7, 15*4(sp)\n"
252+
"sw s2, 16*4(sp)\n"
253+
"sw s3, 17*4(sp)\n"
254+
"sw s4, 18*4(sp)\n"
255+
"sw s5, 19*4(sp)\n"
256+
"sw s6, 20*4(sp)\n"
257+
"sw s7, 21*4(sp)\n"
258+
"sw s8, 22*4(sp)\n"
259+
"sw s9, 23*4(sp)\n"
260+
"sw s10, 24*4(sp)\n"
261+
"sw s11, 25*4(sp)\n"
262+
"sw t3, 26*4(sp)\n"
263+
"sw t4, 27*4(sp)\n"
264+
"sw t5, 28*4(sp)\n"
265+
/* t6 already saved */
266+
267+
/* Save CSRs */
268+
"csrr a0, mcause\n"
269+
"csrr a1, mepc\n"
270+
"csrr a2, mstatus\n"
271+
"sw a0, 30*4(sp)\n"
272+
"sw a1, 31*4(sp)\n"
273+
"sw a2, 32*4(sp)\n"
274+
275+
"mv a2, sp\n" /* a2 = ISR frame pointer */
276+
"call do_trap\n"
174277
"mv sp, a0\n"
175278

176-
/* Restore mstatus from frame[32] */
279+
/* Check MPP in mstatus to determine return path */
177280
"lw t0, 32*4(sp)\n"
178281
"csrw mstatus, t0\n"
179282

180-
/* Restore mepc from frame[31] (might have been modified by handler) */
283+
"srli t1, t0, 11\n"
284+
"andi t1, t1, 0x3\n"
285+
"bnez t1, .Lrestore_mmode\n"
286+
287+
/* Setup mscratch for U-mode restore to prepare for next trap */
288+
".Lrestore_umode:\n"
289+
"la t1, _stack\n"
290+
"csrw mscratch, t1\n"
291+
181292
"lw t1, 31*4(sp)\n"
182293
"csrw mepc, t1\n"
294+
295+
/* Restore all GPRs */
183296
"lw ra, 0*4(sp)\n"
184297
"lw gp, 1*4(sp)\n"
185298
"lw tp, 2*4(sp)\n"
@@ -211,12 +324,12 @@ __attribute__((naked, aligned(4))) void _isr(void)
211324
"lw t5, 28*4(sp)\n"
212325
"lw t6, 29*4(sp)\n"
213326

214-
/* Deallocate stack frame */
215-
"addi sp, sp, %0\n"
327+
/* Restore user SP from frame[33] */
328+
"lw sp, 33*4(sp)\n"
216329

217330
/* Return from trap */
218331
"mret\n"
219-
: /* no outputs */
220-
: "i"(ISR_CONTEXT_SIZE) /* +16 for mcause, mepc, mstatus */
332+
: /* no outputs */
333+
: "i"(ISR_CONTEXT_SIZE)
221334
: "memory");
222335
}

‎arch/riscv/entry.c‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
#include <sys/syscall.h>
18+
#include <types.h>
1819

1920
/* Architecture-specific syscall implementation using ecall trap.
2021
* This overrides the weak symbol defined in kernel/syscall.c.
@@ -40,3 +41,26 @@ int syscall(int num, void *arg1, void *arg2, void *arg3)
4041

4142
return a0;
4243
}
44+
45+
/* Stack Pointer Swap for Testing
46+
*
47+
* This naked function provides atomic SP swapping for kernel validation tests.
48+
* Using __attribute__((naked)) ensures the compiler does not generate any
49+
* prologue/epilogue code that would use the stack, and prevents instruction
50+
* reordering that could break the swap semantics.
51+
*
52+
* Inspired by Linux kernel's __switch_to for context switching.
53+
*/
54+
55+
/* Atomically swap the stack pointer with a new value.
56+
* @new_sp: New stack pointer value to install (in a0)
57+
* @return: Previous stack pointer value (in a0)
58+
*/
59+
__attribute__((naked)) uint32_t __switch_sp(uint32_t new_sp)
60+
{
61+
asm volatile(
62+
"mv t0, sp \n" /* Save current SP to temporary */
63+
"mv sp, a0 \n" /* Install new SP from argument */
64+
"mv a0, t0 \n" /* Return old SP in a0 */
65+
"ret \n");
66+
}

0 commit comments

Comments
 (0)