A lot of fixes #48
A lot of fixes #48
Conversation
WalkthroughThis update introduces a new keyboard driver and shell, integrates them into kernel initialization, and refactors interrupt handling to explicitly separate hardware IRQs and CPU exceptions. The kernel and process management subsystems receive enhanced validation, security, and scheduling logic. Several new headers and supporting functions are added, and the build configuration is updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant CPU
participant InterruptHandler
participant KeyboardHandler
participant Scheduler
participant Shell
participant ProcessManager
CPU->>InterruptHandler: Hardware IRQ (e.g., keyboard, timer)
alt Timer IRQ
InterruptHandler->>Scheduler: FastSchedule()
InterruptHandler->>CPU: Send EOI
else Keyboard IRQ
InterruptHandler->>KeyboardHandler: KeyboardHandler()
InterruptHandler->>CPU: Send EOI
else Other IRQ
InterruptHandler->>CPU: Send EOI
end
CPU->>InterruptHandler: CPU Exception (e.g., page fault)
InterruptHandler->>InterruptHandler: Handle exception, print diagnostics
Shell->>KeyboardHandler: GetChar()
KeyboardHandler-->>Shell: Return next char/input
Shell->>ProcessManager: Execute command (e.g., ps, sched)
ProcessManager-->>Shell: Command output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
drivers/Keyboard.h (1)
4-4: Use angle brackets for standard library headers.The include directive should use angle brackets for standard library headers rather than quotes.
Apply this diff:
-#include "stdint.h" +#include <stdint.h>kernel/etc/Shell.c (1)
44-69: Consider adding input validation for control characters.The main shell processing loop is well-implemented with proper buffer bounds checking and input handling. However, consider adding validation to filter or handle control characters other than newline and backspace to prevent potential issues.
Consider adding control character filtering:
if (HasInput()) { char c = GetChar(); if (c == '\n') { command_buffer[cmd_pos] = 0; ExecuteCommand(command_buffer); cmd_pos = 0; PrintKernel("VFS> "); } else if (c == '\b') { if (cmd_pos > 0) { cmd_pos--; PrintKernel("\b \b"); // Visual backspace } + } else if (c < 32 && c != '\t') { + // Skip other control characters except tab + continue; } else if (cmd_pos < 255) { command_buffer[cmd_pos++] = c; char str[2] = {c, 0}; PrintKernel(str); // Echo character }drivers/Keyboard.c (1)
33-33: Use array element count instead of sizeofUsing
sizeof(scancode_to_ascii)works here but is semantically incorrect. It returns bytes, not the number of elements.- if (scancode >= sizeof(scancode_to_ascii)) return; // Invalid scancode + if (scancode >= sizeof(scancode_to_ascii) / sizeof(scancode_to_ascii[0])) return; // Invalid scancodeOr define a constant:
#define SCANCODE_TABLE_SIZE (sizeof(scancode_to_ascii) / sizeof(scancode_to_ascii[0]))arch/x86_64/interrupts/Interrupts.c (2)
25-25: Use of GNU C extension for case rangesThe code uses case ranges (
case 34 ... 47:), which is a GNU C extension and may not be portable to all compilers.If portability is a concern, consider using a default case with range checking:
- case 34 ... 47: + default: + if (regs->interrupt_number >= 34 && regs->interrupt_number <= 47) { PrintKernelWarning("[IRQ] Unhandled hardware interrupt: "); // ... rest of the code + } else if (regs->interrupt_number < 32) { + goto handle_exceptions; + }
39-97: Exception handling always results in panicWhile the detailed error reporting is excellent, all CPU exceptions lead to system panic without any attempt at recovery. Some exceptions like page faults could potentially be handled gracefully (e.g., demand paging, copy-on-write).
For future enhancements, consider:
- Implementing recoverable page fault handling for user-space processes
- Gracefully terminating only the offending process for certain exceptions
- Adding exception statistics/logging before deciding to panic
kernel/core/Kernel.c (1)
57-63: Silent page alignment without warningThe function silently aligns addresses by clearing the lower 12 bits. This could mask programming errors where unaligned addresses are passed.
Consider logging a warning when alignment is needed:
if (vaddr & 0xFFF || paddr & 0xFFF) { + PrintKernelWarning("[WARN] BootstrapMapPage: Aligning addresses\n"); vaddr &= ~0xFFF; // Page-align virtual address paddr &= ~0xFFF; // Page-align physical address }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.cargo/config.toml(0 hunks)arch/x86_64/interrupts/Interrupts.c(1 hunks)arch/x86_64/interrupts/Interrupts.h(1 hunks)arch/x86_64/syscall/Syscall.c(1 hunks)drivers/Keyboard.c(1 hunks)drivers/Keyboard.h(1 hunks)kernel/core/Kernel.c(5 hunks)kernel/etc/Shell.c(1 hunks)kernel/etc/Shell.h(1 hunks)kernel/memory/MemOps.c(1 hunks)kernel/memory/MemOps.h(1 hunks)kernel/memory/Memory.c(2 hunks)kernel/process/Process.c(27 hunks)kernel/process/Process.h(3 hunks)meson.build(1 hunks)
💤 Files with no reviewable changes (1)
- .cargo/config.toml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-08T00:36:50.297Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#2
File: Kernel/Syscall.c:44-44
Timestamp: 2025-07-08T00:36:50.297Z
Learning: In IdtSetGate function calls, the parameters are: (interrupt_vector, handler_address, selector, flags). The interrupt vector (like 0x80) is the first argument, and the selector (like 0x08 for kernel code segment) is the third argument.
Applied to files:
arch/x86_64/syscall/Syscall.carch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-07-16T01:04:23.790Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#47
File: kernel/core/Kernel.c:434-434
Timestamp: 2025-07-16T01:04:23.790Z
Learning: VMemAlloc() in VoidFrame kernel fails with error code -2 (VMEM_ERROR_INVALID_ADDR) due to virtual address space layout conflict where the heap start address (_bss_end + KERNEL_VIRTUAL_OFFSET + 1MB margin) exceeds VIRT_ADDR_SPACE_END (0xFFFFFFFF80000000ULL), causing VMemMap validation to fail.
Applied to files:
arch/x86_64/syscall/Syscall.ckernel/memory/Memory.ckernel/core/Kernel.c
🧬 Code Graph Analysis (7)
kernel/memory/MemOps.h (1)
kernel/memory/MemOps.c (1)
FastStrCmp(90-100)
kernel/etc/Shell.h (1)
kernel/etc/Shell.c (2)
ShellInit(40-42)ShellProcess(44-69)
drivers/Keyboard.h (1)
drivers/Keyboard.c (4)
KeyboardInit(21-23)KeyboardHandler(25-41)GetChar(43-50)HasInput(52-54)
kernel/process/Process.h (1)
kernel/process/Process.c (2)
KillProcess(318-320)DumpPerformanceStats(1325-1352)
kernel/etc/Shell.c (4)
kernel/etc/Console.c (3)
PrintKernelSuccess(102-106)PrintKernel(87-99)ClearScreen(23-39)kernel/memory/MemOps.c (1)
FastStrCmp(90-100)kernel/process/Process.c (4)
ListProcesses(1366-1401)DumpPerformanceStats(1325-1352)DumpSchedulerState(1403-1432)Yield(868-879)drivers/Keyboard.c (2)
HasInput(52-54)GetChar(43-50)
kernel/core/Kernel.c (7)
kernel/memory/Memory.c (1)
AllocPage(175-231)kernel/etc/Console.c (6)
PrintKernel(87-99)PrintKernelSuccess(102-106)PrintKernelError(108-112)PrintKernelHex(121-144)PrintKernelInt(147-171)PrintKernelWarning(114-118)kernel/memory/VMem.c (1)
VMemInit(34-49)kernel/memory/KernelHeap.c (1)
KernelHeapInit(179-184)drivers/Keyboard.c (1)
KeyboardInit(21-23)kernel/etc/Shell.c (1)
ShellInit(40-42)kernel/process/Process.c (1)
ProcessInit(1266-1323)
drivers/Keyboard.c (1)
include/Io.h (1)
inb(10-14)
🔇 Additional comments (25)
kernel/memory/Memory.c (3)
147-148: LGTM! Const qualifiers improve code safety.Adding
consttokernel_start_addrandkernel_end_addris excellent practice since these values are calculated once and never modified. This prevents accidental modification and clearly communicates intent.
150-151: LGTM! Const qualifiers enhance safety.The
constqualifiers onkernel_start_pageandkernel_end_pageare appropriate since these derived values should not change after calculation. This improves code safety and readability.
165-166: LGTM! Consistent const usage.Adding
consttomb_info_start_pageandmb_info_end_pagemaintains consistency with the other memory range calculations and prevents unintended modifications to these computed values.drivers/Keyboard.h (1)
6-9: LGTM! Clean and well-designed keyboard interface.The function declarations provide a clear and complete interface for keyboard operations:
KeyboardInit()for initializationKeyboardHandler()for interrupt processingGetChar()for input retrievalHasInput()for availability checkingThis design follows good separation of concerns and matches the implementation shown in the relevant code snippets.
kernel/process/Process.h (4)
9-84: Excellent MLFQ scheduler parameter restructuring.The reorganization of scheduler parameters with clear sectioning, comprehensive documentation, and derived constants is a significant improvement. The structured approach makes tuning and maintenance much easier.
Key improvements:
- Logical grouping by functionality
- Clear documentation for each parameter
- Derived constants to avoid magic numbers
- Comprehensive coverage of scheduler aspects
19-22: Verify quantum decay configuration.The
QUANTUM_DECAY_SHIFTis set to 0, which means no quantum reduction per priority level. This might not provide the typical MLFQ behavior where lower priority levels get longer time slices.Consider whether this is intentional or if quantum should increase for lower priority levels:
-#define QUANTUM_DECAY_SHIFT 0 // Quantum reduction per level (0 = no decay) +#define QUANTUM_DECAY_SHIFT 1 // Quantum doubles per level down
116-116: LGTM! ProcessContext typedef simplification.Removing the
structkeyword from the typedef is cleaner and more idiomatic C.
200-200: LGTM! Performance monitoring addition.The
DumpPerformanceStats()function declaration aligns with the implementation shown in the relevant code snippets and provides valuable debugging capabilities.arch/x86_64/syscall/Syscall.c (2)
7-7: LGTM! Comment cleanup is appropriate.Removing the "For FastMemcpy" comment is fine since the purpose of including
MemOps.his clear from the actual usage ofFastMemcpyon line 41.
7-7: Removal of Kernel.h from Syscall.c is safeVerified that
Syscall.cdoes not use any declarations fromKernel.h—the only symbol it provides (ParseMultibootInfo) isn’t referenced here. All remaining identifiers (e.g.,Panic,PrintKernel,IdtSetGate,KERNEL_CODE_SELECTOR) come from their respective headers. No further changes needed.kernel/memory/MemOps.h (1)
10-10: LGTM! Clean function declaration.The
FastStrCmpdeclaration follows standard C string comparison conventions and is consistent with the other optimized functions in this header.kernel/etc/Shell.h (1)
1-7: LGTM! Clean and minimal shell interface.The header file follows good practices with proper include guards and provides a clear, minimal interface for shell functionality.
meson.build (2)
74-74: LGTM! Properly integrated shell source file.The Shell.c source file is correctly added to the build configuration following the existing pattern.
80-80: LGTM! Properly integrated keyboard driver.The Keyboard.c source file is correctly added to the build configuration following the existing pattern.
kernel/memory/MemOps.c (1)
90-99: LGTM! Well-implemented string comparison function.The implementation correctly handles null pointers, uses byte-by-byte comparison to avoid alignment issues, and properly casts to unsigned char for consistent results across different char signedness implementations.
kernel/etc/Shell.c (4)
7-8: LGTM! Appropriate buffer management.The static command buffer size and position tracking are well-designed for a kernel shell context.
10-13: LGTM! Simple version display function.Clean implementation for displaying version information.
15-38: LGTM! Well-structured command dispatcher.The command execution function properly uses string comparison and provides good command coverage. The unknown command handling includes helpful guidance.
40-42: LGTM! Simple and effective initialization.Proper initialization of the command position for shell startup.
kernel/core/Kernel.c (1)
245-254: Guard pages are mentioned but not actually implementedThe comments indicate guard pages are being created, but the code doesn't actually leave any unmapped pages. The messages about guard pages are misleading.
To actually implement guard pages:
+ // Skip a page before stack for guard page (leave unmapped) + // No BootstrapMapPage call here - this creates the guard + // Map actual stack for (uint64_t paddr = stack_phys_start; paddr < stack_phys_end; paddr += PAGE_SIZE) { BootstrapMapPage(pml4_addr, paddr + KERNEL_VIRTUAL_OFFSET, paddr, PAGE_WRITABLE); } - // Create guard page AFTER stack (unmapped) - PrintKernel("[GUARD] Stack guard page after stack (unmapped)\n"); + // Skip a page after stack for guard page (leave unmapped) + // No BootstrapMapPage call here either⛔ Skipped due to learnings
Learnt from: assembler-0 PR: assembler-0/VoidFrame#47 File: kernel/core/Kernel.c:434-434 Timestamp: 2025-07-16T01:04:23.790Z Learning: VMemAlloc() in VoidFrame kernel fails with error code -2 (VMEM_ERROR_INVALID_ADDR) due to virtual address space layout conflict where the heap start address (_bss_end + KERNEL_VIRTUAL_OFFSET + 1MB margin) exceeds VIRT_ADDR_SPACE_END (0xFFFFFFFF80000000ULL), causing VMemMap validation to fail.kernel/process/Process.c (5)
1164-1191: Excellent security enhancement for detecting privilege escalationThe addition of frequent checks for unauthorized system-level processes is a critical security improvement. The immediate termination of processes with system privileges but lacking supervisor/critical flags effectively prevents privilege escalation attacks.
The implementation correctly:
- Runs frequently (every 25 ticks) for timely detection
- Limits scan scope to prevent performance impact
- Immediately terminates threats
- Escalates threat level appropriately
1304-1320: Proper security token handling for shell processExcellent implementation of the shell process creation with proper security token management. The code correctly recalculates the checksum after modifying the token flags, maintaining the integrity of the security system.
871-878: Critical fix for Yield functionExcellent fix! The previous implementation setting the state to
PROC_BLOCKEDwas indeed incorrect as there was no mechanism to unblock the process. Setting it toPROC_READYcorrectly allows the process to be rescheduled.The addition of the
hltinstruction is also appropriate to avoid busy-waiting.
410-420: Duplicate condition check in quantum initializationThe condition
i < RT_PRIORITY_THRESHOLDis checked twice in nested if statements, which is redundant.for (int i = 0; i < MAX_PRIORITY_LEVELS; i++) { if (i < RT_PRIORITY_THRESHOLD) { // Real-time queues get larger quantums - if (i < RT_PRIORITY_THRESHOLD) { MLFQscheduler.queues[i].quantum = QUANTUM_BASE << (RT_PRIORITY_THRESHOLD - i); if (MLFQscheduler.queues[i].quantum > QUANTUM_MAX) { MLFQscheduler.queues[i].quantum = QUANTUM_MAX; } - } else { - MLFQscheduler.queues[i].quantum = QUANTUM_BASE >> ((i - RT_PRIORITY_THRESHOLD) * QUANTUM_DECAY_SHIFT); - if (MLFQscheduler.queues[i].quantum < QUANTUM_MIN) { - MLFQscheduler.queues[i].quantum = QUANTUM_MIN; - } - } MLFQscheduler.rt_bitmap |= (1U << i); } else {Likely an incorrect or invalid review comment.
836-853: Verify ProcessBlocked state check and invocationThe
if (proc->state == PROC_READY …)inside ProcessBlocked (kernel/process/Process.c:836–853) will almost never be true, since a process that’s blocking is still inPROC_RUNNINGand there’s no assignment toPROC_BLOCKEDaround this call. As a result, the I/O priority boost will never trigger.Please verify:
- Where and when
ProcessBlocked(slot)is called in relation to settingproc->state- Whether the process state should be updated to
PROC_BLOCKED(or if the condition should checkPROC_RUNNING) before performing the priority boost
| #ifndef INTERRUPTS_H | ||
| #define INTERRUPTS_H | ||
|
|
||
| #endif // INTERRUPTS_H |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding interrupt-related declarations.
This header appears to be a placeholder with only include guards. Given the significant interrupt handling refactoring mentioned in the AI summary, consider adding appropriate function declarations, constants, or type definitions that should be exposed from the interrupt subsystem.
For example, if KeyboardHandler or other interrupt handlers are called from external code, their declarations should be present here:
#ifndef INTERRUPTS_H
#define INTERRUPTS_H
+// Function declarations for interrupt handlers
+void KeyboardHandler(void);
+void TimerHandler(void);
+// Add other interrupt-related declarations as needed
+
#endif // INTERRUPTS_H📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #ifndef INTERRUPTS_H | |
| #define INTERRUPTS_H | |
| #endif // INTERRUPTS_H | |
| #ifndef INTERRUPTS_H | |
| #define INTERRUPTS_H | |
| // Function declarations for interrupt handlers | |
| void KeyboardHandler(void); | |
| void TimerHandler(void); | |
| // Add other interrupt-related declarations as needed | |
| #endif // INTERRUPTS_H |
🤖 Prompt for AI Agents
In arch/x86_64/interrupts/Interrupts.h at lines 1 to 4, the file currently only
contains include guards and lacks any interrupt-related declarations. Add
function declarations, constants, or type definitions relevant to the interrupt
subsystem that need to be accessed externally, such as declarations for
KeyboardHandler or other interrupt handlers used outside this module. This will
ensure proper interface exposure for the interrupt handling functionality.
drivers/Keyboard.c
Outdated
| static char input_buffer[256]; | ||
| static int buffer_head = 0; | ||
| static int buffer_tail = 0; | ||
| static int buffer_count = 0; |
There was a problem hiding this comment.
Race condition in buffer access
The buffer variables (buffer_head, buffer_tail, buffer_count) are accessed from both interrupt context (KeyboardHandler) and normal context (GetChar) without synchronization. This can lead to data corruption or lost keystrokes.
Consider using atomic operations or disabling interrupts during buffer access:
-static int buffer_head = 0;
-static int buffer_tail = 0;
-static int buffer_count = 0;
+static volatile int buffer_head = 0;
+static volatile int buffer_tail = 0;
+static volatile int buffer_count = 0;Additionally, protect critical sections in GetChar:
char GetChar(void) {
irq_flags_t flags = SpinLockIrqSave(&keyboard_lock);
if (buffer_count == 0) {
SpinUnlockIrqRestore(&keyboard_lock, flags);
return 0;
}
char c = input_buffer[buffer_head];
buffer_head = (buffer_head + 1) % 256;
buffer_count--;
SpinUnlockIrqRestore(&keyboard_lock, flags);
return c;
}🤖 Prompt for AI Agents
In drivers/Keyboard.c around lines 8 to 11, the shared buffer variables
buffer_head, buffer_tail, and buffer_count are accessed concurrently from
interrupt and normal contexts without synchronization, causing race conditions.
To fix this, protect all accesses to these variables by disabling interrupts or
using atomic operations. Specifically, wrap the critical sections in both the
interrupt handler and the GetChar function with spinlock calls that save and
restore interrupt flags, ensuring exclusive access during buffer manipulation.
| static char scancode_to_ascii[] = { | ||
| 0, 0, '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '-', '=', '\b', | ||
| '\t', 'q', 'w', 'e', 'r', 't', 'y', 'u', 'i', 'o', 'p', '[', ']', '\n', | ||
| 0, 'a', 's', 'd', 'f', 'g', 'h', 'j', 'k', 'l', ';', '\'', '`', | ||
| 0, '\\', 'z', 'x', 'c', 'v', 'b', 'n', 'm', ',', '.', '/', 0, | ||
| '*', 0, ' ' | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Incomplete keyboard mapping
The scancode mapping table only covers a subset of keys and doesn't handle modifier keys (Shift, Ctrl, Alt) or different keyboard states (Caps Lock, Num Lock). This limits the keyboard functionality significantly.
Consider implementing a more complete keyboard driver with state tracking:
- Add support for shift states to handle uppercase/special characters
- Track Caps Lock, Num Lock, and Scroll Lock states
- Handle extended scancodes (0xE0 prefix) for arrow keys, Page Up/Down, etc.
- Support function keys (F1-F12)
🤖 Prompt for AI Agents
In drivers/Keyboard.c around lines 13 to 19, the scancode_to_ascii array only
maps a limited set of keys and does not account for modifier keys or keyboard
states. To fix this, expand the scancode mapping to include all standard keys
including function keys, arrow keys, and extended scancodes with 0xE0 prefix.
Implement state tracking variables for Shift, Ctrl, Alt, Caps Lock, Num Lock,
and Scroll Lock to modify the output characters accordingly. Update the keyboard
input handling logic to use these states and produce correct ASCII output for
uppercase letters and special characters.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
kernel/core/Kernel.c (1)
114-150: Complete memory protection implementation.The
SetupMemoryProtectionfunction now properly detects SMEP/SMAP support via CPUID, enables the features by setting CR4 bits when supported, and provides accurate logging. This addresses the previous incomplete implementation.This resolves the issue identified in the previous review where memory protection features were not actually being enabled.
🧹 Nitpick comments (2)
kernel/process/Process.c (2)
165-257: Enhanced process termination security.The termination logic now includes comprehensive security checks protecting immune and critical processes, with proper token validation and state management. The implementation follows security best practices.
Consider monitoring the performance impact of the multiple security checks in the termination path, especially under high process creation/destruction workloads.
261-299: SIVA security enhancements approved.The SystemIntegrityVerificationAgent now includes critical privilege escalation detection, scanning for unauthorized system processes every 25 ticks. The direct security manager registration and enhanced threat detection logic significantly improve system security.
Monitor system performance impact of the increased scanning frequency (every 25 ticks) under various workloads to ensure the security benefits don't negatively affect system responsiveness.
Also applies to: 1113-1118, 1156-1191
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
drivers/Keyboard.c(1 hunks)kernel/core/Kernel.c(5 hunks)kernel/process/Process.c(28 hunks)kernel/process/Process.h(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- drivers/Keyboard.c
- kernel/process/Process.h
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-16T01:04:23.790Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#47
File: kernel/core/Kernel.c:434-434
Timestamp: 2025-07-16T01:04:23.790Z
Learning: VMemAlloc() in VoidFrame kernel fails with error code -2 (VMEM_ERROR_INVALID_ADDR) due to virtual address space layout conflict where the heap start address (_bss_end + KERNEL_VIRTUAL_OFFSET + 1MB margin) exceeds VIRT_ADDR_SPACE_END (0xFFFFFFFF80000000ULL), causing VMemMap validation to fail.
Applied to files:
kernel/core/Kernel.c
🧬 Code Graph Analysis (1)
kernel/core/Kernel.c (7)
kernel/memory/Memory.c (1)
AllocPage(175-231)kernel/etc/Console.c (7)
PrintKernel(87-99)PrintKernelSuccess(102-106)ClearScreen(23-39)PrintKernelError(108-112)PrintKernelHex(121-144)PrintKernelInt(147-171)PrintKernelWarning(114-118)kernel/memory/VMem.c (1)
VMemInit(34-49)kernel/memory/KernelHeap.c (1)
KernelHeapInit(179-184)drivers/Keyboard.c (1)
KeyboardInit(21-23)kernel/etc/Shell.c (1)
ShellInit(40-42)kernel/process/Process.c (1)
ProcessInit(1266-1323)
🔇 Additional comments (11)
kernel/process/Process.c (6)
10-10: Shell integration properly implemented.The shell subsystem is correctly integrated with proper header inclusion, secure process creation with system privileges, and correct security token management including checksum recalculation after flag updates.
Also applies to: 1304-1321
30-30: Enhanced security token management.The new
SecureTokenUpdatehelper function provides secure, atomic token updates with proper authorization checks, following security best practices by centralizing token modifications and ensuring checksum integrity.Also applies to: 1089-1107
410-420: Significant scheduler improvements for fairness and stability.The MLFQ scheduler now includes:
- Quantum clamping within defined bounds
- Less aggressive preemption and load balancing to prevent queue thrashing
- Smart aging that boosts starved processes to priority 0 to guarantee execution
- More conservative quantum adjustments and I/O boosting
- Reduced frequency of fairness operations to improve system stability
These changes address common MLFQ issues like starvation and excessive priority changes.
Also applies to: 546-563, 577-641, 653-675, 705-754, 781-812, 835-853
896-896: Process creation function visibility change approved.Making
CreateSecureProcesspublic enables other system components to create secure processes while maintaining all existing security validations and checks.
1015-1015: Cleanup batch size improvement.Using a named constant
CLEANUP_MAX_PER_CALLinstead of a hardcoded value improves maintainability and allows for easier performance tuning of the termination cleanup process.
868-879: Critical Yield function bug fix.The fix correctly sets the yielding process state to
PROC_READYinstead ofPROC_BLOCKED, preventing processes from being permanently stuck. The addition of the HLT instruction properly waits for the next scheduler interrupt.kernel/core/Kernel.c (5)
152-209: Enhanced initialization logging and new subsystem integration.The detailed logging for each initialization step improves system observability and debugging. The integration of keyboard and shell initialization is properly implemented, and calling memory protection setup last follows security best practices.
58-80: Robust input validation for page mapping.The enhanced validation in
BootstrapMapPageincludes alignment checks, null pointer detection, and corruption detection. The automatic page alignment for addresses prevents subtle bugs while maintaining functionality.
288-313: Memory layout validation for system safety.The
ValidateMemoryLayoutfunction provides runtime verification of memory layout assumptions, detecting potential stack/kernel overlaps that could cause security issues. The informational logging helps with system debugging and maintenance.
211-211: Parameter const-qualification improvement.Adding
constqualifiers to themagicandinfoparameters prevents accidental modification and clearly indicates these are read-only inputs, improving code safety and intent clarity.
267-281: Kernel stack guard page security enhancement.The implementation of unmapped guard pages before and after the kernel stack provides immediate detection of stack overflow/underflow conditions through page faults, significantly improving kernel security and debugging capabilities.
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores