Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions Kernel/Core/Ipc.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#include "Ipc.h"
#include "Process.h"
#include "Panic.h"
#include "../Memory/MemOps.h"

void IpcSendMessage(uint32_t target_pid, IpcMessage* msg) {
ASSERT(msg != NULL);

Process* target = GetProcessByPid(target_pid);
if (!target) {
// Handle error: target process not found
return;
}

MessageQueue* queue = &target->ipc_queue;
if (queue->count >= MAX_MESSAGES) {
// Handle error: queue is full
return;
}

FastMemcpy(&queue->messages[queue->tail], msg, sizeof(IpcMessage));
queue->tail = (queue->tail + 1) % MAX_MESSAGES;
queue->count++;

if (target->state == PROC_BLOCKED) {
target->state = PROC_READY;
}
}
Comment on lines +6 to +28
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Improve error handling and add synchronization for IpcSendMessage.

Several issues need to be addressed:

  1. Silent failures on lines 12 and 18 make debugging difficult
  2. No synchronization primitives protecting queue access (race conditions possible)
  3. The sender_pid should be set by the kernel to prevent spoofing

Consider these improvements:

-void IpcSendMessage(uint32_t target_pid, IpcMessage* msg) {
+int IpcSendMessage(uint32_t target_pid, IpcMessage* msg) {
     ASSERT(msg != NULL);
 
     Process* target = GetProcessByPid(target_pid);
     if (!target) {
-        // Handle error: target process not found
-        return;
+        return -ESRCH; // No such process
     }
 
     MessageQueue* queue = &target->ipc_queue;
+    
+    // TODO: Add spinlock or disable interrupts here for queue protection
+    
     if (queue->count >= MAX_MESSAGES) {
-        // Handle error: queue is full
-        return;
+        return -EAGAIN; // Resource temporarily unavailable
     }
+    
+    // Set sender PID to prevent spoofing
+    msg->sender_pid = GetCurrentProcess()->pid;
 
     FastMemcpy(&queue->messages[queue->tail], msg, sizeof(IpcMessage));
     queue->tail = (queue->tail + 1) % MAX_MESSAGES;
     queue->count++;
+    
+    // TODO: Release spinlock or re-enable interrupts here
 
     if (target->state == PROC_BLOCKED) {
         target->state = PROC_READY;
     }
+    
+    return 0; // Success
 }
📝 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.

Suggested change
void IpcSendMessage(uint32_t target_pid, IpcMessage* msg) {
ASSERT(msg != NULL);
Process* target = GetProcessByPid(target_pid);
if (!target) {
// Handle error: target process not found
return;
}
MessageQueue* queue = &target->ipc_queue;
if (queue->count >= MAX_MESSAGES) {
// Handle error: queue is full
return;
}
FastMemcpy(&queue->messages[queue->tail], msg, sizeof(IpcMessage));
queue->tail = (queue->tail + 1) % MAX_MESSAGES;
queue->count++;
if (target->state == PROC_BLOCKED) {
target->state = PROC_READY;
}
}
int IpcSendMessage(uint32_t target_pid, IpcMessage* msg) {
ASSERT(msg != NULL);
Process* target = GetProcessByPid(target_pid);
if (!target) {
return -ESRCH; // No such process
}
MessageQueue* queue = &target->ipc_queue;
// TODO: Add spinlock or disable interrupts here for queue protection
if (queue->count >= MAX_MESSAGES) {
return -EAGAIN; // Resource temporarily unavailable
}
// Set sender PID to prevent spoofing
msg->sender_pid = GetCurrentProcess()->pid;
FastMemcpy(&queue->messages[queue->tail], msg, sizeof(IpcMessage));
queue->tail = (queue->tail + 1) % MAX_MESSAGES;
queue->count++;
// TODO: Release spinlock or re-enable interrupts here
if (target->state == PROC_BLOCKED) {
target->state = PROC_READY;
}
return 0; // Success
}
🤖 Prompt for AI Agents
In Kernel/Core/Ipc.c around lines 6 to 28, improve IpcSendMessage by replacing
silent returns on errors with explicit error logging or handling to aid
debugging, add synchronization (e.g., a mutex or spinlock) around the message
queue access to prevent race conditions, and ensure the sender_pid field in the
IpcMessage is set by the kernel within this function before copying the message
to prevent sender spoofing.


int IpcReceiveMessage(IpcMessage* msg_buffer) {
ASSERT(msg_buffer != NULL);

Process* current = GetCurrentProcess();
MessageQueue* queue = &current->ipc_queue;

while (queue->count == 0) {
current->state = PROC_BLOCKED;
Yield();
}

FastMemcpy(msg_buffer, &queue->messages[queue->head], sizeof(IpcMessage));
queue->head = (queue->head + 1) % MAX_MESSAGES;
queue->count--;

return 0; // Success
}
Comment on lines +30 to +46
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add synchronization and improve blocking mechanism.

The current implementation has potential race conditions and inefficient blocking:

Consider these improvements:

 int IpcReceiveMessage(IpcMessage* msg_buffer) {
     ASSERT(msg_buffer != NULL);
 
     Process* current = GetCurrentProcess();
     MessageQueue* queue = &current->ipc_queue;
 
+    // TODO: Add spinlock or disable interrupts here for queue protection
+    
     while (queue->count == 0) {
+        // TODO: Release spinlock before blocking
         current->state = PROC_BLOCKED;
         Yield();
+        // TODO: Re-acquire spinlock after waking up
     }
 
     FastMemcpy(msg_buffer, &queue->messages[queue->head], sizeof(IpcMessage));
     queue->head = (queue->head + 1) % MAX_MESSAGES;
     queue->count--;
+    
+    // TODO: Release spinlock or re-enable interrupts here
 
     return 0; // Success
 }

Also consider adding a timeout mechanism to prevent indefinite blocking.

📝 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.

Suggested change
int IpcReceiveMessage(IpcMessage* msg_buffer) {
ASSERT(msg_buffer != NULL);
Process* current = GetCurrentProcess();
MessageQueue* queue = &current->ipc_queue;
while (queue->count == 0) {
current->state = PROC_BLOCKED;
Yield();
}
FastMemcpy(msg_buffer, &queue->messages[queue->head], sizeof(IpcMessage));
queue->head = (queue->head + 1) % MAX_MESSAGES;
queue->count--;
return 0; // Success
}
int IpcReceiveMessage(IpcMessage* msg_buffer) {
ASSERT(msg_buffer != NULL);
Process* current = GetCurrentProcess();
MessageQueue* queue = &current->ipc_queue;
// TODO: Add spinlock or disable interrupts here for queue protection
while (queue->count == 0) {
// TODO: Release spinlock before blocking
current->state = PROC_BLOCKED;
Yield();
// TODO: Re-acquire spinlock after waking up
}
FastMemcpy(msg_buffer, &queue->messages[queue->head], sizeof(IpcMessage));
queue->head = (queue->head + 1) % MAX_MESSAGES;
queue->count--;
// TODO: Release spinlock or re-enable interrupts here
return 0; // Success
}
🤖 Prompt for AI Agents
In Kernel/Core/Ipc.c around lines 30 to 46, the IpcReceiveMessage function lacks
synchronization, leading to potential race conditions when accessing the message
queue, and uses a busy-wait loop for blocking. To fix this, introduce proper
synchronization primitives like mutexes or spinlocks to protect access to the
queue, replace the busy-wait with a blocking wait mechanism such as condition
variables or semaphores to efficiently block the process until a message
arrives, and optionally implement a timeout feature to avoid indefinite blocking
by returning an error or timeout status if no message is received within the
specified time.

35 changes: 35 additions & 0 deletions Kernel/Core/Ipc.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#ifndef IPC_H
#define IPC_H

#include "stdint.h"

#define MAX_MESSAGES 16 // Max messages per process queue

// Extensible message types
typedef enum {
IPC_TYPE_DATA,
IPC_TYPE_NOTIFICATION,
// Future types can be added here
} IpcMessageType;

typedef struct {
uint32_t sender_pid;
IpcMessageType type;
uint64_t size;
union {
char data[256]; // For general data
uint64_t value; // For notifications or simple values
} payload;
} IpcMessage;

typedef struct {
IpcMessage messages[MAX_MESSAGES];
uint32_t head;
uint32_t tail;
uint32_t count;
} MessageQueue;

void IpcSendMessage(uint32_t target_pid, IpcMessage* msg);
int IpcReceiveMessage(IpcMessage* msg_buffer);

#endif
1 change: 1 addition & 0 deletions Kernel/Core/Kernel.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#ifndef KERNEL_H
#define KERNEL_H
#include "stdint.h"
extern int CurrentLine;
extern int CurrentColumn;
void ClearScreen();
Expand Down
5 changes: 3 additions & 2 deletions Kernel/Core/Panic.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
void __attribute__((noreturn)) Panic(const char* message);
void __attribute__((noreturn)) PanicWithCode(const char* message, uint64_t error_code);

#define STRINGIFY(x) #x
#define STRINGIFY_HELPER(x) #x
#define STRINGIFY(x) STRINGIFY_HELPER(x)
#define ASSERT(condition) \
do { \
if (!(condition)) { \
Panic("Assertion failed: " #condition " at " __FILE__ ":" STRINGIFY(__LINE__); \
Panic("Assertion failed: " #condition " at " __FILE__ ":" STRINGIFY(__LINE__)); \
} \
} while(0)

Expand Down
2 changes: 2 additions & 0 deletions Kernel/Core/stdint.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef VOIDFRAME_STDINT_H
#define VOIDFRAME_STDINT_H

#define NULL ((void*)0)

typedef unsigned char uint8_t;
_Static_assert(sizeof(uint8_t) == 1, "sizeof(uint8_t) != 1");

Expand Down
3 changes: 2 additions & 1 deletion Kernel/Drivers/Cpu.c
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#include "Cpu.h"

#include "../Core/Panic.h"
static CpuFeatures cpu_features = {0};

static void cpuid(uint32_t leaf, uint32_t* eax, uint32_t* ebx, uint32_t* ecx, uint32_t* edx) {
ASSERT(eax != NULL && ebx != NULL && ecx != NULL && edx != NULL);
asm volatile("cpuid" : "=a"(*eax), "=b"(*ebx), "=c"(*ecx), "=d"(*edx) : "a"(leaf));
}

Expand Down
1 change: 0 additions & 1 deletion Kernel/Drivers/Cpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#define CPU_H

#include "stdint.h"

typedef struct {
uint8_t sse:1;
uint8_t sse2:1;
Expand Down
1 change: 1 addition & 0 deletions Kernel/Drivers/Interrupts.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ static void FastDisplayTicks(uint64_t ticks) {

// The C-level interrupt handler
void InterruptHandler(struct Registers* regs) {
ASSERT(regs != NULL);
if (regs->interrupt_number == 32) {
tick_count++;
// FastDisplayTicks(tick_count); // Re-enabled
Expand Down
4 changes: 4 additions & 0 deletions Kernel/Memory/MemOps.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#include "MemOps.h"
#include "Cpu.h"
#include "../Core/Panic.h"

void* FastMemset(void* dest, int value, uint64_t size) {
ASSERT(dest != NULL);
CpuFeatures* features = GetCpuFeatures();
uint8_t* d = (uint8_t*)dest;

Expand Down Expand Up @@ -34,6 +36,7 @@ void* FastMemset(void* dest, int value, uint64_t size) {
}

void* FastMemcpy(void* dest, const void* src, uint64_t size) {
ASSERT(dest != NULL && src != NULL);
CpuFeatures* features = GetCpuFeatures();
uint8_t* d = (uint8_t*)dest;
const uint8_t* s = (const uint8_t*)src;
Expand All @@ -58,6 +61,7 @@ void* FastMemcpy(void* dest, const void* src, uint64_t size) {
}

void FastZeroPage(void* page) {
ASSERT(page != NULL);
CpuFeatures* features = GetCpuFeatures();

if (features->sse2) {
Expand Down
11 changes: 8 additions & 3 deletions Kernel/Process/Process.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
#include "Panic.h"
#include "Io.h"
#include "../Memory/MemOps.h"

#define NULL ((void*)0)
#include "../Core/Ipc.h"
#define offsetof(type, member) ((uint64_t)&(((type*)0)->member))

static Process processes[MAX_PROCESSES];
Expand Down Expand Up @@ -121,7 +120,7 @@ uint32_t CreateProcess(void (*entry_point)(void)) {
}

void ProcessExitStub() {
PrintKernel("[KERNEL] Process returned from its main function. This is an error!\n");
PrintKernel("[KERNEL] Process returned from its main function. -FATAL EXECPTION-\n");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix typo in error message.

"EXECPTION" should be "EXCEPTION".

-    PrintKernel("[KERNEL] Process returned from its main function. -FATAL EXECPTION-\n");
+    PrintKernel("[KERNEL] Process returned from its main function. -FATAL EXCEPTION-\n");
📝 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.

Suggested change
PrintKernel("[KERNEL] Process returned from its main function. -FATAL EXECPTION-\n");
PrintKernel("[KERNEL] Process returned from its main function. -FATAL EXCEPTION-\n");
🤖 Prompt for AI Agents
In Kernel/Process/Process.c at line 123, correct the typo in the error message
by changing "EXECPTION" to "EXCEPTION" in the PrintKernel call to ensure the
message is spelled correctly.

PrintKernel("Terminating process PID: ");
PrintKernelInt(GetCurrentProcess()->pid);
PrintKernel("\n");
Expand Down Expand Up @@ -177,6 +176,11 @@ uint32_t CreateSecureProcess(void (*entry_point)(void), uint8_t privilege) {
processes[slot].priority = (privilege == PROC_PRIV_SYSTEM) ? 10 : 100;
processes[slot].is_user_mode = (privilege != PROC_PRIV_SYSTEM);

// Initialize IPC queue
processes[slot].ipc_queue.head = 0;
processes[slot].ipc_queue.tail = 0;
processes[slot].ipc_queue.count = 0;

// Create the token
init_token(&processes[slot].token, creator->pid, privilege, new_pid);

Expand All @@ -198,6 +202,7 @@ uint32_t CreateSecureProcess(void (*entry_point)(void), uint8_t privilege) {
}

void ScheduleFromInterrupt(struct Registers* regs) {
ASSERT(regs != NULL);
if (process_count <= 1) return;

// Save current context
Expand Down
2 changes: 2 additions & 0 deletions Kernel/Process/Process.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define PROCESS_H

#include "stdint.h"
#include "../Core/Ipc.h"

#define MAX_PROCESSES 64
#define STACK_SIZE 4096
Expand Down Expand Up @@ -48,6 +49,7 @@ typedef struct {
uint8_t privilege_level;
uint8_t _padding;
SecurityToken token;
MessageQueue ipc_queue;
ProcessContext context;
} Process;

Expand Down
2 changes: 2 additions & 0 deletions Kernel/Process/UserMode.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "Panic.h"

void JumpToUserMode(void (*user_function)(void)) {
ASSERT(user_function != NULL);
// Allocate user stack
void* user_stack = AllocPage();
if (!user_stack) {
Expand Down Expand Up @@ -32,6 +33,7 @@ void JumpToUserMode(void (*user_function)(void)) {
}

void CreateUserProcess(void (*user_function)(void)) {
ASSERT(user_function != NULL);
// Create process but mark it as user mode
uint32_t pid = CreateProcess(user_function);
if (pid > 0) {
Expand Down
12 changes: 11 additions & 1 deletion Kernel/System/Syscall.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
#include "Idt.h"
#include "Panic.h"
#include "../Memory/MemOps.h" // For FastMemcpy
#include "../Core/Ipc.h"

#define likely(x) __builtin_expect(!!(x), 1)
#define unlikely(x) __builtin_expect(!!(x), 0)
extern void SyscallEntry(void);
uint64_t SyscallHandler(uint64_t syscall_num, uint64_t arg1, uint64_t arg2, uint64_t arg3) {
uint64_t Syscall(uint64_t syscall_num, uint64_t arg1, uint64_t arg2, uint64_t arg3) {
Process* current = GetCurrentProcess();
if (unlikely(!current)) {
Panic("Syscall from invalid process");
Expand Down Expand Up @@ -49,6 +50,15 @@ uint64_t SyscallHandler(uint64_t syscall_num, uint64_t arg1, uint64_t arg2, uint

case SYS_GETPID:
return current ? current->pid : -1;

case SYS_IPC_SEND:
// arg1 = target_pid, arg2 = message
IpcSendMessage((uint32_t)arg1, (IpcMessage*)arg2);
return 0;
Comment on lines +54 to +57
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Validate user pointers and add security checks for IPC send.

The current implementation has several security vulnerabilities:

  1. No validation of the user-provided message pointer
  2. No bounds checking on message content
  3. No authorization checks for sending messages to other processes

Consider implementing proper validation:

 case SYS_IPC_SEND:
     // arg1 = target_pid, arg2 = message
-    IpcSendMessage((uint32_t)arg1, (IpcMessage*)arg2);
-    return 0;
+    if (!arg2 || arg1 == 0) {
+        return -1; // Invalid arguments
+    }
+    
+    // Copy message from user space to kernel space
+    IpcMessage kernel_msg;
+    if (CopyFromUser(&kernel_msg, (void*)arg2, sizeof(IpcMessage)) != 0) {
+        return -1; // Failed to copy from user space
+    }
+    
+    // Validate message fields
+    if (kernel_msg.size > sizeof(kernel_msg.payload.data)) {
+        return -1; // Invalid message size
+    }
+    
+    // Set sender PID to prevent spoofing
+    kernel_msg.sender_pid = current->pid;
+    
+    IpcSendMessage((uint32_t)arg1, &kernel_msg);
+    return 0;

Note: You'll need to implement CopyFromUser function for safe user-to-kernel memory copying.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In Kernel/System/Syscall.c around lines 54 to 57, the code lacks validation of
the user-provided message pointer, bounds checking on the message content, and
authorization checks for sending IPC messages. To fix this, implement a
CopyFromUser function to safely copy the message from user space to kernel
space, validate that the message pointer is not null and points to accessible
memory, check the message size against allowed limits, and verify that the
sending process is authorized to send messages to the target process before
calling IpcSendMessage.


case SYS_IPC_RECV:
// arg1 = message_buffer
return IpcReceiveMessage((IpcMessage*)arg1);
Comment on lines +59 to +61
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Validate user buffer for IPC receive.

The implementation lacks validation of the user-provided buffer pointer, which could lead to arbitrary memory writes.

Implement proper validation and safe copying:

 case SYS_IPC_RECV:
     // arg1 = message_buffer
-    return IpcReceiveMessage((IpcMessage*)arg1);
+    if (!arg1) {
+        return -1; // Invalid buffer
+    }
+    
+    // Receive message into kernel buffer first
+    IpcMessage kernel_msg;
+    int result = IpcReceiveMessage(&kernel_msg);
+    if (result != 0) {
+        return result;
+    }
+    
+    // Copy message to user space
+    if (CopyToUser((void*)arg1, &kernel_msg, sizeof(IpcMessage)) != 0) {
+        return -1; // Failed to copy to user space
+    }
+    
+    return 0;

Note: You'll need to implement CopyToUser function for safe kernel-to-user memory copying.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In Kernel/System/Syscall.c around lines 59 to 61, the code directly uses the
user-provided buffer pointer for IPC receive without validating it, risking
arbitrary memory writes. To fix this, add validation to ensure the user buffer
pointer is valid and accessible before use. Implement and use a CopyToUser
function to safely copy data from kernel space to the user buffer, preventing
unsafe memory operations.


default:
return -1;
Expand Down
4 changes: 3 additions & 1 deletion Kernel/System/Syscall.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@
#define SYS_WRITE 2
#define SYS_READ 3
#define SYS_GETPID 4
#define SYS_IPC_SEND 5
#define SYS_IPC_RECV 6
#define SYSCALL_INTERRUPT_VECTOR 0x80
#define IDT_INTERRUPT_GATE_KERNEL 0x8E
#define SYSCALL_SEGMENT_SELECTOR 0x08
#define MAX_SYSCALL_BUFFER_SIZE 4096
// System call handler
void SyscallInit(void);
uint64_t SyscallHandler(uint64_t syscall_num, uint64_t arg1, uint64_t arg2, uint64_t arg3);
uint64_t Syscall(uint64_t syscall_num, uint64_t arg1, uint64_t arg2, uint64_t arg3);

#endif
4 changes: 2 additions & 2 deletions Kernel/System/SyscallEntry.asm
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
bits 64

extern SyscallHandler
extern Syscall

global SyscallEntry

Expand Down Expand Up @@ -28,7 +28,7 @@ SyscallEntry:
mov rdx, rcx ; arg2
mov rcx, r8 ; arg3

call SyscallHandler
call Syscall

; Return value in rax is already set

Expand Down
1 change: 1 addition & 0 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ c_sources = [
'Kernel/Drivers/Interrupts.c',
'Kernel/Drivers/Pic.c',
'Kernel/Drivers/Cpu.c',
'Kernel/Core/Ipc.c'
]

# Build include flags
Expand Down