Implement kernel stack isolation for U-mode tasks #62
+281
−100
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HeatCrab
force-pushed
the
u-mode/basic-support
branch
2 times, most recently
from
9801050 to
afcfd14
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 "used inline" 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
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.
HeatCrab
force-pushed
the
u-mode/basic-support
branch
from
afcfd14 to
fe3574c
Compare
vicLin8712
reviewed
Dec 14, 2025
vicLin8712
reviewed
Dec 14, 2025
vicLin8712
reviewed
Dec 15, 2025
visitorckw
reviewed
Dec 17, 2025
| * 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; |
Collaborator
There was a problem hiding this comment.
Where in the code is _kernel_stack_ptr actually used?
visitorckw
reviewed
Dec 17, 2025
| /* 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); |
Collaborator
There was a problem hiding this comment.
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?
visitorckw
reviewed
Dec 17, 2025
| asm volatile( | ||
| "mv %0, sp \n" | ||
| "li sp, 0xDEADBEEF \n" | ||
| : "=r"(saved_sp)); |
Collaborator
There was a problem hiding this comment.
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
mscratchregister as a discriminator between machine mode and user mode execution contexts. The ISR entry performs a blind swap withmscratch: 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 inmscratch. The interrupt frame structure is extended to 36 words withframe[33]dedicated to stack pointer storage. Task initialization configuresmscratchappropriately during the first dispatch by checking theMPPfield in mstatus.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 frommscratchrather 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