Skip to content

Commit 9801050

Browse files
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 36 words with frame[33] dedicated to stack pointer storage. 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. 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 9801050

File tree

5 files changed

+277
-96
lines changed

5 files changed

+277
-96
lines changed

app/umode.c

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

3-
/* U-mode Validation Task
3+
/* U-mode validation: syscall stability and privilege isolation.
44
*
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.
5+
* Phase 1: Verify syscalls work under various SP conditions (normal,
6+
* malicious). Phase 2: Verify privileged instructions trap.
87
*/
98
void umode_validation_task(void)
109
{
11-
/* --- Phase 1: Mechanism Check (Syscalls) --- */
12-
umode_printf("[umode] Phase 1: Testing Syscall Mechanism\n");
10+
/* --- Phase 1: Kernel Stack Isolation Test --- */
11+
umode_printf("[umode] Phase 1: Testing Kernel Stack Isolation\n");
12+
umode_printf("\n");
1313

14-
/* Test 1: sys_tid() - Simplest read-only syscall. */
14+
/* Test 1a: Baseline - Syscall with normal SP */
15+
umode_printf("[umode] Test 1a: sys_tid() with normal SP\n");
1516
int my_tid = sys_tid();
1617
if (my_tid > 0) {
1718
umode_printf("[umode] PASS: sys_tid() returned %d\n", my_tid);
1819
} else {
1920
umode_printf("[umode] FAIL: sys_tid() failed (ret=%d)\n", my_tid);
2021
}
22+
umode_printf("\n");
2123

22-
/* Test 2: sys_uptime() - Verify value transmission is correct. */
24+
/* Test 1b: Verify ISR uses mscratch, not malicious user SP */
25+
umode_printf("[umode] Test 1b: sys_tid() with malicious SP\n");
26+
27+
uint32_t saved_sp;
28+
asm volatile(
29+
"mv %0, sp \n"
30+
"li sp, 0xDEADBEEF \n"
31+
: "=r"(saved_sp));
32+
33+
int my_tid_bad_sp = sys_tid();
34+
35+
asm volatile("mv sp, %0 \n" : : "r"(saved_sp));
36+
37+
if (my_tid_bad_sp > 0) {
38+
umode_printf(
39+
"[umode] PASS: sys_tid() succeeded, ISR correctly used kernel "
40+
"stack\n");
41+
} else {
42+
umode_printf(
43+
"[umode] FAIL: Syscall failed with malicious SP (ret=%d)\n",
44+
my_tid_bad_sp);
45+
}
46+
umode_printf("\n");
47+
48+
/* Test 1c: Verify syscall functionality is still intact */
49+
umode_printf("[umode] Test 1c: sys_uptime() with normal SP\n");
2350
int uptime = sys_uptime();
2451
if (uptime >= 0) {
2552
umode_printf("[umode] PASS: sys_uptime() returned %d\n", uptime);
2653
} else {
2754
umode_printf("[umode] FAIL: sys_uptime() failed (ret=%d)\n", uptime);
2855
}
56+
umode_printf("\n");
2957

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

3462
/* --- Phase 2: Security Check (Privileged Access) --- */
3563
umode_printf("[umode] ========================================\n");
64+
umode_printf("\n");
3665
umode_printf("[umode] Phase 2: Testing Security Isolation\n");
66+
umode_printf("\n");
3767
umode_printf(
3868
"[umode] Action: Attempting to read 'mstatus' CSR from U-mode.\n");
3969
umode_printf("[umode] Expect: Kernel Panic with 'Illegal instruction'.\n");
40-
umode_printf("[umode] ========================================\n");
70+
umode_printf("\n");
4171

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

arch/riscv/boot.c

Lines changed: 151 additions & 38 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 */
112-
"addi sp, sp, -%0\n"
113-
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
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.
124112
*/
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+
119+
"addi sp, sp, -144\n"
120+
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, 144\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, -144\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+
: /* no inputs, ISR_CONTEXT_SIZE used inline */
221334
: "memory");
222335
}

0 commit comments

Comments
 (0)