Implement kernel stack isolation for U-mode tasks #62

HeatCrab · 2025-12-13T13:24:04Z

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.

Stack isolation is implemented by 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, 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 configures mscratch appropriately during the first dispatch by checking the MPP field in mstatus.

[umode] Phase 1: Testing Kernel Stack Isolation

[umode] Test 1a: sys_tid() with normal SP
[umode] PASS: sys_tid() returned 2

[umode] Test 1b: sys_tid() with malicious SP
[umode] PASS: sys_tid() succeeded, ISR correctly used kernel stack

[umode] Test 1c: sys_uptime() with normal SP
[umode] PASS: sys_uptime() returned 4

[umode] Phase 1 Complete: Kernel stack isolation validated

[umode] ========================================

[umode] Phase 2: Testing Security Isolation

[umode] Action: Attempting to read 'mstatus' CSR from U-mode.
[umode] Expect: Kernel Panic with 'Illegal instruction'.

[EXCEPTION] Illegal instruction epc=0x800002F0

Test code and the outputs 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. All existing tests continue to pass, demonstrating that the isolation mechanism does not affect machine mode task execution.

Related to #53

cubic-dev-ai

No issues found across 5 files

cubic-dev-ai

1 issue found across 7 files

Prompt for AI agents (all 1 issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="arch/riscv/boot.c">

<violation number="1" location="arch/riscv/boot.c:333">
P3: Comment claims `ISR_CONTEXT_SIZE` is &quot;used inline&quot; but the macro is not actually used - the value `144` is hardcoded. Consider either using the macro via extended asm operands (as the original code did) or updating the comment to reflect the actual implementation.</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}

arch/riscv/boot.c

arch/riscv/hal.c

visitorckw · 2025-12-17T20:16:16Z

arch/riscv/hal.c

+     * This value is used by the ISR epilogue to restore mscratch
+     * when returning to U-mode tasks. _stack is the kernel stack base.
+     */
+    _kernel_stack_ptr = (uint32_t) &_stack;


Where in the code is _kernel_stack_ptr actually used?

After checking the code, the _kernel_stack_ptr variable was intended to mirror this value but is not actually read by any consumer. I've removed it to eliminate the dead code. Thanks !

visitorckw · 2025-12-17T20:19:13Z

kernel/syscall.c

+    /* Output directly character by character. U-mode tasks need strict
+     * ordering, which the async logger queue cannot guarantee. */
+    for (const char *p = str; *p; p++)
+        _putchar(*p);


However, if an interrupt occurs in the middle of the loop, would this cause output interleaving between kernel space and user space, or between two different user-space tasks?

You're right. To address this, I have wrapped the output loop within a critical section.

However, while tracing the relevant I/O code, I also noticed similar unprotected output loops in other parts of the codebase. To keep this PR focused on its primary objective, I decided to only fix the specific instance you pointed out. I‘ll open a separate issue to address the similar problems in the rest of the code.

visitorckw · 2025-12-17T20:27:12Z

app/umode.c

+    asm volatile(
+        "mv %0, sp          \n"
+        "li sp, 0xDEADBEEF  \n"
+        : "=r"(saved_sp));


We should consider encapsulating this logic into a user library helper in the future. This would prevent code duplication and eliminate the risk of compiler reordering breaking the inline assembly.

Agreed. Inspired by the Linux kernel's __switch_to mechanism, I've refactored the code by introducing a helper function __switch_sp(uint32_t new_sp) in arch/riscv/entry.c using __attribute__((naked)).As the function signature suggests, it is designed to accept the new stack pointer as an argument rather than relying on hardcoded values. This makes it a reusable utility for any future tests that need to validate different stack scenarios.

The test case in app/umode.c now cleanly calls this helper instead of using raw inline assembly, ensuring the compiler does not generate any prologue/epilogue code or reorder instructions that could interfere with the swap.

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.

cubic-dev-ai bot reviewed Dec 13, 2025

View reviewed changes

HeatCrab marked this pull request as draft December 13, 2025 13:28

HeatCrab force-pushed the u-mode/basic-support branch 2 times, most recently from 9801050 to afcfd14 Compare December 13, 2025 14:15

HeatCrab marked this pull request as ready for review December 13, 2025 14:21

cubic-dev-ai bot reviewed Dec 13, 2025

View reviewed changes

arch/riscv/boot.c Outdated Show resolved Hide resolved

HeatCrab force-pushed the u-mode/basic-support branch from afcfd14 to fe3574c Compare December 13, 2025 14:42

vicLin8712 reviewed Dec 14, 2025

View reviewed changes

arch/riscv/boot.c Show resolved Hide resolved

vicLin8712 reviewed Dec 14, 2025

View reviewed changes

arch/riscv/boot.c Show resolved Hide resolved

vicLin8712 reviewed Dec 15, 2025

View reviewed changes

arch/riscv/hal.c Show resolved Hide resolved

visitorckw reviewed Dec 17, 2025

View reviewed changes

HeatCrab force-pushed the u-mode/basic-support branch from fe3574c to e31f722 Compare December 18, 2025 09:11

HeatCrab mentioned this pull request Dec 19, 2025

Output Interleaving due to Unprotected I/O Loops #64

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement kernel stack isolation for U-mode tasks #62

Implement kernel stack isolation for U-mode tasks #62

Uh oh!

HeatCrab commented Dec 13, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

visitorckw Dec 17, 2025

Uh oh!

HeatCrab Dec 18, 2025

Uh oh!

visitorckw Dec 17, 2025

Uh oh!

HeatCrab Dec 18, 2025 •

edited

Loading

Uh oh!

visitorckw Dec 17, 2025

Uh oh!

HeatCrab Dec 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Implement kernel stack isolation for U-mode tasks #62

Are you sure you want to change the base?

Implement kernel stack isolation for U-mode tasks #62

Uh oh!

Conversation

HeatCrab commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

visitorckw Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

HeatCrab Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

visitorckw Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

HeatCrab Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

visitorckw Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

HeatCrab Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HeatCrab commented Dec 13, 2025 •

edited

Loading

HeatCrab Dec 18, 2025 •

edited

Loading