Shell changed #94
Shell changed #94
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds VBE-aware dual-path console I/O, refactors the shell into a command registry with path resolution and modular handlers, restructures scheduler/process APIs and hardens process lifecycle/security, and simplifies some Kernel.c messages plus an unreachable marker after the idle loop. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Boot
participant ConsoleInit as ConsoleInit()
participant VBECheck as VBEIsInitialized()
participant VBEPath as VBEConsole*
participant VGAPath as VGA Buffer
Boot->>ConsoleInit: ConsoleInit()
ConsoleInit->>VBECheck: VBEIsInitialized()
alt VBE available
ConsoleInit->>VBEPath: VBEConsoleInit()
Note right of VBEPath #DDEEFF: use_vbe = true
else VGA fallback
ConsoleInit->>VGAPath: map VGA buffer, init state
Note right of VGAPath #F6F1E0: use_vbe = false
end
participant Print as PrintKernel(...)
Print->>ConsoleInit: PrintKernel(text)
alt use_vbe
ConsoleInit->>VBEPath: VBEConsolePrint(text)
else
ConsoleInit->>VGAPath: write VGA buffer chars
end
sequenceDiagram
autonumber
actor User
participant Shell as ShellProcess
participant Registry as commands[]
participant Handler as CommandHandler
User->>Shell: input line
Shell->>Registry: lookup command by name
alt found
Registry->>Handler: Handler(args)
Handler-->>Shell: return
else not found
Shell-->>User: "Unknown command"
end
Shell-->>User: print prompt (current_dir)
sequenceDiagram
autonumber
actor IRQ as TimerIRQ
participant Fast as FastSchedule(Registers*)
participant Scheduler as SchedulerCore
participant Proc as ProcessTable
IRQ->>Fast: timer interrupt (regs)
Fast->>Scheduler: FastSchedule(regs)
Scheduler->>Proc: pick next slot/process
alt context switch needed
Scheduler->>Proc: context switch
else
Note right of Scheduler #F0F8FF: keep current
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
✨ 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/Issue comments)Type 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 (7)
kernel/core/Kernel.c (1)
693-694: Unreachable sentinel after an infinite loop is fine (minor nit).The
__builtin_unreachable();after thewhile (1) { Yield(); }loop is a harmless hint to the optimizer. If you want to make the intent even clearer to toolchains and sanitizers, consider markingKernelMainHigherHalfas non-returning or using afor (;;)loop with an explicithlt:- while (1) { // redundant but added for worst case scenario, should not reach here - Yield(); - } - __builtin_unreachable(); + for (;;) { + Yield(); + asm volatile("hlt"); + }kernel/etc/Shell.c (6)
88-118: ResolvePath doesn’t normalize “.”/“..” segments or collapse duplicate slashes (optional).Current logic only concatenates. Consider normalizing to avoid surprises like
/a/b/../c//d. This also helps VFS backends that expect canonical paths.If you want, I can draft a compact in-place normalizer that handles
./.., repeated slashes, and keeps withinmax_len.
123-127: Consistency: add newline to error message.The error path prints without a trailing newline.
- PrintKernelError("RTL8139 not ready"); + PrintKernelError("RTL8139 not ready\n");
621-655: Minor: const correctness for string literal in Fstest.
test_textcan beconst char*.- char* test_text = "Hello VoidFrame!\n"; + const char* test_text = "Hello VoidFrame!\n";
716-717: Show prompt immediately on shell start (QoL).Currently the prompt appears only after the first Enter. Print it once before the loop.
- PrintKernelSuccess("System: VoidFrame Shell v0.0.1-beta ('help' for list of commands)\n"); + PrintKernelSuccess("System: VoidFrame Shell v0.0.1-beta ('help' for list of commands)\n"); + PrintKernel(current_dir); + PrintKernel("> ");
735-737: Micro-alloc avoidance: use PrintKernelChar for echoing keystrokes.Avoid stack temp and string function call per keystroke.
- const char str[2] = {c, 0}; - PrintKernel(str); // Echo character + PrintKernelChar(c); // Echo character
48-86: Argument parser is simple by design; call out current limits (FYI).
- Max token length 63 chars (truncates silently).
- No quoting/escaping;
echo "hello world"will split on spaces.- Static buffer makes it non-reentrant (OK for single-threaded shell).
If you plan to accept spaces in args (e.g., echo), we can extend
GetArgto honor quotes.Would you like me to draft a minimal quoted-argument parser that keeps kernel allocations bounded?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
kernel/core/Kernel.c(1 hunks)kernel/etc/Console.c(0 hunks)kernel/etc/Console.h(1 hunks)kernel/etc/Shell.c(8 hunks)
💤 Files with no reviewable changes (1)
- kernel/etc/Console.c
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-08-10T06:03:22.076Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#62
File: kernel/core/Panic.c:69-79
Timestamp: 2025-08-10T06:03:22.076Z
Learning: In VoidFrame kernel, the console color state (current_color) needs to be explicitly preserved and restored when switching between display modes or clearing the screen. ConsoleSetColor() should update both text mode and VBE mode color states when vbe_enabled is true.
Applied to files:
kernel/etc/Console.h
📚 Learning: 2025-08-10T06:03:22.076Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#62
File: kernel/core/Panic.c:69-79
Timestamp: 2025-08-10T06:03:22.076Z
Learning: In VoidFrame kernel's VBEConsoleClear() function, the color is hardcoded to 0x08 (gray on black) instead of using the current console color state, causing color washing issues. The function should use console.color or the current color state to maintain color consistency across screen clears.
Applied to files:
kernel/etc/Console.h
📚 Learning: 2025-08-12T02:13:46.266Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#72
File: arch/x86_64/syscall/SyscallEntry.asm:24-30
Timestamp: 2025-08-12T02:13:46.266Z
Learning: User assembler-0 has existing ELF infrastructure in VoidFrame kernel with Elf.h header definitions and CreateProcess() function that properly sets up process context including stack allocation, ProcessExitStub return address, and register initialization. The issue was missing ELF loader implementation, not compilation problems.
Applied to files:
kernel/etc/Shell.c
📚 Learning: 2025-08-12T03:19:52.873Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#73
File: kernel/elf/ELFloader.c:0-0
Timestamp: 2025-08-12T03:19:52.873Z
Learning: VoidFrame kernel now has complete UEFI boot support with hybrid BIOS/UEFI ISO creation capability. The existing VBE framebuffer implementation requires no changes as it uses Multiboot2 framebuffer tags and direct memory-mapped access, making it inherently UEFI-compatible. Build process: ninja + ./scripts/build_hybrid_iso.sh creates dual-platform bootable ISO.
Applied to files:
kernel/etc/Shell.c
📚 Learning: 2025-08-12T03:19:52.873Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#73
File: kernel/elf/ELFloader.c:0-0
Timestamp: 2025-08-12T03:19:52.873Z
Learning: User assembler-0 has successfully implemented UEFI boot support for VoidFrame kernel. The VBE framebuffer implementation is fully UEFI-compatible using Multiboot2 framebuffer tags and direct memory-mapped access, requiring no kernel code changes - only bootloader configuration updates.
Applied to files:
kernel/etc/Shell.c
📚 Learning: 2025-08-09T09:29:00.630Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#57
File: kernel/memory/VMem.h:64-66
Timestamp: 2025-08-09T09:29:00.630Z
Learning: In the VoidFrame kernel, removing locks from KernelMemoryAlloc/KernelMemoryFree causes heap corruption where shell command strings (like "clear" = 0x61656C63) overwrite heap block magic values due to race conditions between allocation and free operations.
Applied to files:
kernel/etc/Shell.c
🧬 Code graph analysis (2)
kernel/etc/Console.h (1)
kernel/etc/Console.c (1)
PrintKernel(146-162)
kernel/etc/Shell.c (11)
drivers/ethernet/RTL8139.c (2)
GetRtl8139Device(117-119)Rtl8139_SendPacket(89-115)kernel/etc/Console.c (5)
PrintKernelError(183-188)PrintKernelSuccess(176-181)PrintKernel(146-162)PrintKernelInt(259-283)PrintKernelWarning(190-195)kernel/memory/MemOps.c (2)
FastMemset(95-158)FastMemcpy(160-247)kernel/process/Process.c (5)
CreateProcess(1095-1097)ListProcesses(1688-1723)DumpPerformanceStats(1647-1674)DumpSchedulerState(1725-1757)KillProcess(335-337)kernel/memory/Memory.c (1)
GetDetailedMemoryStats(319-369)kernel/memory/KernelHeap.c (2)
KernelFree(278-308)KernelMemoryAlloc(185-227)fs/VFS.c (8)
VfsIsDir(330-349)VfsListDir(167-201)VfsReadFile(126-165)VfsCreateDir(227-245)VfsGetFileSize(267-328)VfsCreateFile(203-225)VfsDelete(247-265)VfsWriteFile(351-374)kernel/elf/ELFloader.c (1)
CreateProcessFromElf(104-287)kernel/etc/Editor.c (1)
EditorOpen(229-286)kernel/etc/StringOps.c (1)
FastStrCmp(25-35)drivers/PS2.c (2)
HasInput(238-240)GetChar(229-236)
🔇 Additional comments (3)
kernel/etc/Console.h (1)
40-48: No stale references toINIT_ERROR_SYSCALLfound; removal is safe.The ripgrep search for any occurrences of
INIT_ERROR_SYSCALLreturned no matches across the repository (excluding build artifacts), confirming that no code still references the removed enum value.kernel/etc/Shell.c (2)
2-2: Include path update looks good.Switching to
"ELFloader.h"aligns with the new loader path and matches usage inElfloadHandler.
400-442: PrintKernelF is implemented and lock-protected—safe to use in TimeHandler
- PrintKernelF is defined in kernel/etc/Console.c (lines 223–228) and invokes the shared Format(fmt, args) helper.
- Format uses only local stack buffers (no static state), making it reentrant.
- PrintKernelF then calls PrintKernel(formatted), which acquires SpinLock(&lock) around all console output before unlocking, ensuring mutual exclusion.
You can confidently replace the manual itoa/strcat sequence in TimeHandler with:
static void TimeHandler(const char *args) { (void)args; rtc_time_t t; RtcReadTime(&t); PrintKernelF("%04d-%02d-%02d %02d:%02d:%02d\n", t.year, t.month, t.day, t.hour, t.minute, t.second); }
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
kernel/process/Process.h (1)
243-243: Remove staleBoostProcessPrioritydeclarationThe prototype for
BoostProcessPriorityinkernel/process/Process.hhas no corresponding definition in the codebase and isn’t referenced by any translation unit. Keeping it will lead to unresolved-symbol errors. Please remove the declaration at line 243.• File:
kernel/process/Process.h(line 243)Proposed diff:
--- a/kernel/process/Process.h +++ b/kernel/process/Process.h @@ -241,7 +241,6 @@ void GetProcessStats(uint32_t pid, uint32_t* cpu_time, uint32_t* io_ops, uint32_t* preemptions); -void BoostProcessPriority(uint32_t pid); void KillProcess(uint32_t pid);kernel/process/Process.c (5)
252-277: Double-decrement of MLFQscheduler.total_processes on termination.TerminateProcess() calls RemoveFromScheduler(slot), which already decrements total_processes. The extra decrement here causes drift and can underflow.
Apply this diff:
// Update scheduler statistics - if (MLFQscheduler.total_processes > 0) { - MLFQscheduler.total_processes--; - }
312-314: Fix ordering when enqueuing terminated processes (AS path).ASTerminate() enqueues the slot before marking PROC_ZOMBIE. CleanupTerminatedProcesses() explicitly checks for PROC_ZOMBIE and may skip the entry if it races, leaving stale queue entries.
Apply this diff:
- AddToTerminationQueueAtomic(slot); - proc->state = PROC_ZOMBIE; + proc->state = PROC_ZOMBIE; + __atomic_thread_fence(__ATOMIC_SEQ_CST); + AddToTerminationQueueAtomic(slot);
424-447: Unreachable branch in InitScheduler() due to duplicated condition.The nested if (i < RT_PRIORITY_THRESHOLD) inside the outer if duplicates the condition, making the inner else unreachable. Also ensure QUANTUM_MIN is enforced for non-RT levels.
Apply this diff:
- 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 { - // Regular queues use exponential decay - MLFQscheduler.queues[i].quantum = QUANTUM_BASE >> ((i - RT_PRIORITY_THRESHOLD) * QUANTUM_DECAY_SHIFT); - } - MLFQscheduler.queues[i].head = NULL; + for (int i = 0; i < MAX_PRIORITY_LEVELS; i++) { + if (i < RT_PRIORITY_THRESHOLD) { + // Real-time queues get larger quantums + MLFQscheduler.queues[i].quantum = QUANTUM_BASE << (RT_PRIORITY_THRESHOLD - i); + if (MLFQscheduler.queues[i].quantum > QUANTUM_MAX) { + MLFQscheduler.queues[i].quantum = QUANTUM_MAX; + } + MLFQscheduler.rt_bitmap |= (1U << i); + } else { + // Regular queues use exponential decay + 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.queues[i].head = NULL;
881-920: ProcessBlocked mutates scheduler state without holding scheduler_lock.This routine calls RemoveFromScheduler/AddToScheduler, updates quantum_remaining, and reads current_running without synchronization. Races here can corrupt queue links and counters.
Apply this diff to guard the entire routine:
void ProcessBlocked(uint32_t slot) { Process* proc = &processes[slot]; // Track I/O operations for classification proc->io_operations++; + irq_flags_t flags = SpinLockIrqSave(&scheduler_lock); if (slot == MLFQscheduler.current_running) { // Calculate partial CPU burst uint32_t partial_burst = MLFQscheduler.queues[proc->priority].quantum - MLFQscheduler.quantum_remaining; @@ MLFQscheduler.quantum_remaining = 0; RequestSchedule(); } @@ if (proc->state == PROC_READY && proc->privilege_level != PROC_PRIV_SYSTEM) { @@ // Boost priority directly proc->priority = highest_user_priority; @@ AddToScheduler(slot); } } + SpinUnlockIrqRestore(&scheduler_lock, flags); }
1151-1160: Read-lock leak in GetProcessByPid causes deadlocks.The early return path does not release the read lock, leaking it and risking deadlocks.
Apply this diff:
-Process* GetProcessByPid(uint32_t pid) { - ReadLock(&process_table_rwlock); - for (int i = 0; i < MAX_PROCESSES; i++) { - if (processes[i].pid == pid && processes[i].state != PROC_TERMINATED) { - return &processes[i]; - } - } - ReadUnlock(&process_table_rwlock); - return NULL; -} +Process* GetProcessByPid(uint32_t pid) { + ReadLock(&process_table_rwlock); + Process* found = NULL; + for (int i = 0; i < MAX_PROCESSES; i++) { + if (processes[i].pid == pid && processes[i].state != PROC_TERMINATED) { + found = &processes[i]; + break; + } + } + ReadUnlock(&process_table_rwlock); + return found; +}
♻️ Duplicate comments (6)
kernel/etc/Console.h (1)
69-72: Nice: header-defined newline helper with internal linkage.Defining PrintNewline as static inline and keeping always_inline addresses prior linkage concerns and improves call sites’ readability.
kernel/etc/Shell.c (5)
41-46: Clean command-dispatch abstraction.The ShellCommandFunc typedef and structured registry enable easy extension and fix the prior function-pointer mismatches.
322-337: Good: setfreq now validates signed input before casting to uint16_t.This addresses the negative wrap issue previously flagged.
458-468: Double-free fix is correct.Freeing path only once after the if/else resolves the earlier heap double-free.
470-498: OOM path in cat handler is now clean.On allocation failure, ‘file’ is freed and the function returns without touching the NULL buffer.
517-531: filesize resolves relative paths first.Using ResolvePath before VfsGetFileSize brings it in line with other path-based commands.
🧹 Nitpick comments (7)
kernel/process/Process.h (1)
231-238: Document locking contract for the scheduler API.Add a short contract comment stating that AddToScheduler/RemoveFromScheduler/FastSchedule/ProcessBlocked must be called with scheduler_lock held (or that they acquire it internally). This prevents accidental unsynchronized queue mutations by future callers.
Apply this diff to clarify:
void InitScheduler(void); void AddToScheduler(uint32_t slot); void RemoveFromScheduler(uint32_t slot); -void FastSchedule(Registers* regs); +// Concurrency: All scheduler queue mutations must be performed with scheduler_lock held. +// FastSchedule is invoked with interrupts disabled and scheduler_lock held by the caller. +void FastSchedule(Registers* regs); void ProcessBlocked(uint32_t slot); void DumpSchedulerState(void);kernel/process/Process.c (2)
492-513: Avoid inflating total_processes on routine re-queues.AddToScheduler() increments total_processes even when re-queuing a previously running process. Since DeQueue() does not decrement it, this metric will drift upward over time.
Apply this diff to let total_processes represent “non-terminated processes in system” and manage it only at process create/destroy:
- MLFQscheduler.total_processes++;Follow-up: Remove the symmetric decrement in RemoveFromScheduler() (see next comment) and keep adjustments in CreateSecureProcess/CleanupTerminatedProcesses only.
515-566: Symmetric decrement skews total_processes semantics.As above, RemoveFromScheduler() decrements total_processes, but DeQueue() doesn’t increment, so the value no longer reflects system load consistently.
Apply this diff:
- MLFQscheduler.total_processes--;Alternatively, if you want total_processes to reflect “ready + running” processes, recompute it per tick from process table or track separately from per-queue counts to avoid drift.
kernel/etc/Console.h (1)
57-57: Parameter cv-qualifier mismatch: unify PrintKernelChar’s signature across header and source.Header uses
char cwhile Console.c definesconst char c. In C this compiles, but unifying avoids confusion.Apply this change in Console.c (not this file):
-void PrintKernelChar(const char c) { +void PrintKernelChar(char c) {kernel/etc/Shell.c (3)
88-118: ResolvePath lacks canonicalization (.., ., redundant slashes).Current logic concatenates paths and may produce invalid/uncanonical paths (e.g., “/a/../b”, “//foo”). Consider normalizing segments and bounding writes robustly.
Here’s a minimal in-place normalization pass:
static void ResolvePath(const char* input, char* output, int max_len) { if (!input || !output) return; @@ - if (input[0] == '/') { + if (input[0] == '/') { // Absolute path int len = 0; while (input[len] && len < max_len - 1) { output[len] = input[len]; len++; } output[len] = '\0'; } else { @@ output[curr_len + input_len] = '\0'; } + // Simple canonicalization: collapse // and handle "." segments. + // (Keeps ".." as-is for now to avoid complex traversal.) + int r = 0, w = 0; + while (output[r] && w < max_len - 1) { + if (output[r] == '/' && output[r+1] == '/') { r++; continue; } + if (output[r] == '/' && output[r+1] == '.' && (output[r+2] == '/' || output[r+2] == '\0')) { r += 2; continue; } + output[w++] = output[r++]; + } + output[w] = '\0'; }
125-137: Consistency: Add newline to error message.The error path prints “RTL8139 not ready” without a trailing newline, unlike other messages.
Apply this diff:
- PrintKernelError("RTL8139 not ready"); + PrintKernelError("RTL8139 not ready\n");
724-737: Prompt UX: consider printing the initial prompt before first input.After the banner, print “ > ” so the user sees the prompt immediately.
Apply this diff:
void ShellProcess(void) { PrintKernelSuccess("System: VoidFrame Shell v0.0.1-beta ('help' for list of commands)\n"); + PrintKernel(current_dir); + PrintKernel("> "); while (1) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
kernel/etc/Console.h(2 hunks)kernel/etc/Shell.c(7 hunks)kernel/process/Process.c(6 hunks)kernel/process/Process.h(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-08-10T06:03:22.076Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#62
File: kernel/core/Panic.c:69-79
Timestamp: 2025-08-10T06:03:22.076Z
Learning: In VoidFrame kernel, the console color state (current_color) needs to be explicitly preserved and restored when switching between display modes or clearing the screen. ConsoleSetColor() should update both text mode and VBE mode color states when vbe_enabled is true.
Applied to files:
kernel/etc/Console.h
📚 Learning: 2025-08-10T06:03:22.076Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#62
File: kernel/core/Panic.c:69-79
Timestamp: 2025-08-10T06:03:22.076Z
Learning: In VoidFrame kernel's VBEConsoleClear() function, the color is hardcoded to 0x08 (gray on black) instead of using the current console color state, causing color washing issues. The function should use console.color or the current color state to maintain color consistency across screen clears.
Applied to files:
kernel/etc/Console.h
📚 Learning: 2025-08-09T09:29:00.630Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#57
File: kernel/memory/VMem.h:64-66
Timestamp: 2025-08-09T09:29:00.630Z
Learning: In the VoidFrame kernel, removing locks from KernelMemoryAlloc/KernelMemoryFree causes heap corruption where shell command strings (like "clear" = 0x61656C63) overwrite heap block magic values due to race conditions between allocation and free operations.
Applied to files:
kernel/etc/Shell.c
📚 Learning: 2025-08-12T02:13:46.266Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#72
File: arch/x86_64/syscall/SyscallEntry.asm:24-30
Timestamp: 2025-08-12T02:13:46.266Z
Learning: User assembler-0 has existing ELF infrastructure in VoidFrame kernel with Elf.h header definitions and CreateProcess() function that properly sets up process context including stack allocation, ProcessExitStub return address, and register initialization. The issue was missing ELF loader implementation, not compilation problems.
Applied to files:
kernel/etc/Shell.c
📚 Learning: 2025-08-12T03:19:52.873Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#73
File: kernel/elf/ELFloader.c:0-0
Timestamp: 2025-08-12T03:19:52.873Z
Learning: VoidFrame kernel now has complete UEFI boot support with hybrid BIOS/UEFI ISO creation capability. The existing VBE framebuffer implementation requires no changes as it uses Multiboot2 framebuffer tags and direct memory-mapped access, making it inherently UEFI-compatible. Build process: ninja + ./scripts/build_hybrid_iso.sh creates dual-platform bootable ISO.
Applied to files:
kernel/etc/Shell.c
📚 Learning: 2025-08-12T03:19:52.873Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#73
File: kernel/elf/ELFloader.c:0-0
Timestamp: 2025-08-12T03:19:52.873Z
Learning: User assembler-0 has successfully implemented UEFI boot support for VoidFrame kernel. The VBE framebuffer implementation is fully UEFI-compatible using Multiboot2 framebuffer tags and direct memory-mapped access, requiring no kernel code changes - only bootloader configuration updates.
Applied to files:
kernel/etc/Shell.c
📚 Learning: 2025-08-10T08:52:13.313Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#63
File: kernel/etc/Shell.c:24-28
Timestamp: 2025-08-10T08:52:13.313Z
Learning: VoidFrame's TerminateProcess function in Process.c only checks if the caller has PROC_PRIV_SYSTEM privilege but doesn't compare caller vs target privilege levels, allowing supervisor processes like the shell to incorrectly kill system-level processes.
Applied to files:
kernel/process/Process.c
📚 Learning: 2025-08-10T08:52:13.313Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#63
File: kernel/etc/Shell.c:24-28
Timestamp: 2025-08-10T08:52:13.313Z
Learning: The VoidFrame kernel's KillProcess function lacks proper privilege enforcement - supervisor-level processes can incorrectly kill system-level processes and vice versa, indicating missing privilege validation checks.
Applied to files:
kernel/process/Process.c
🧬 Code graph analysis (4)
kernel/process/Process.h (1)
kernel/process/Process.c (1)
FastSchedule(708-879)
kernel/etc/Console.h (1)
kernel/etc/Console.c (2)
PrintKernelChar(164-170)PrintKernel(146-162)
kernel/etc/Shell.c (12)
drivers/ethernet/RTL8139.c (2)
GetRtl8139Device(117-119)Rtl8139_SendPacket(89-115)kernel/etc/Console.c (6)
PrintKernelError(183-188)ClearScreen(58-81)PrintKernelSuccess(176-181)PrintKernel(146-162)PrintKernelInt(259-283)PrintKernelWarning(190-195)kernel/process/Process.c (5)
CreateProcess(1087-1089)ListProcesses(1675-1710)DumpPerformanceStats(1634-1661)DumpSchedulerState(1712-1744)KillProcess(339-341)kernel/memory/KernelHeap.c (2)
KernelFree(278-308)KernelMemoryAlloc(185-227)include/stdlib.h (1)
atoi(18-44)drivers/sound/SB16.c (1)
SB16_Beep(35-58)drivers/Serial.c (1)
SerialWrite(129-138)drivers/LPT/LPT.c (1)
LPT_WriteString(70-76)drivers/Pic.c (3)
PitSetFrequency(26-42)PIC_disable_irq(56-60)PIC_enable_irq(50-54)fs/VFS.c (8)
VfsIsDir(330-349)VfsListDir(167-201)VfsReadFile(126-165)VfsCreateDir(227-245)VfsGetFileSize(267-328)VfsCreateFile(203-225)VfsDelete(247-265)VfsWriteFile(351-374)kernel/elf/ELFloader.c (1)
CreateProcessFromElf(104-287)drivers/PS2.c (2)
HasInput(238-240)GetChar(229-236)
kernel/process/Process.c (1)
kernel/etc/Console.c (1)
PrintKernelError(183-188)
🔇 Additional comments (4)
kernel/process/Process.h (1)
235-235: Signature normalization looks good.Switching to FastSchedule(Registers* regs) (typedef-based) aligns the header with the implementation and avoids repeating struct keywords in call sites.
kernel/process/Process.c (3)
933-946: Good addition: __builtin_unreachable after the HLT loop.Adding the unreachable marker helps the optimizer and silences “fallthrough” warnings in freestanding builds.
676-702: Security preflight messaging and enforcement look solid.The tightened privilege-escalation check with explicit SYSTEM privilege gating and clear error messaging is a welcome hardening step.
1366-1371: ABSi is already defined and included
Process.cincludes"stdlib.h", which definesstatic inline int ABSi(const int x)(see include/stdlib.h:4–6). No additional helper is needed.
|
dont know why but i think my kernel fails this test |
1 participant
Summary by CodeRabbit
New Features
Improvements
Refactor
Behavior Changes