From 6862d802a882d989e36fee2b0aa200391d948f16 Mon Sep 17 00:00:00 2001 From: Amlal El Mahrouss Date: Tue, 10 Jun 2025 16:24:48 +0200 Subject: fix: UserProcessScheduler security patches. These patches are regarding: - Thread safety (exit code global has been removed.) - Code quality, in `SpawnDylib` - In memory allocation checks too. Signed-off-by: Amlal El Mahrouss --- dev/kernel/src/UserProcessScheduler.cc | 40 ++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 19 deletions(-) (limited to 'dev/kernel/src/UserProcessScheduler.cc') diff --git a/dev/kernel/src/UserProcessScheduler.cc b/dev/kernel/src/UserProcessScheduler.cc index e417cc8d..6753b238 100644 --- a/dev/kernel/src/UserProcessScheduler.cc +++ b/dev/kernel/src/UserProcessScheduler.cc @@ -25,12 +25,6 @@ ///! BUGS: 0 namespace Kernel { -/***********************************************************************************/ -/// @brief Exit Code global variable. -/***********************************************************************************/ - -STATIC UInt32 kLastExitCode = 0U; - USER_PROCESS::USER_PROCESS() = default; USER_PROCESS::~USER_PROCESS() = default; @@ -38,10 +32,6 @@ USER_PROCESS::~USER_PROCESS() = default; /// @note Not thread-safe. /// @return Int32 the last exit code. -const UInt32& sched_get_exit_code(void) noexcept { - return kLastExitCode; -} - /***********************************************************************************/ /// @brief Crashes the current process. /***********************************************************************************/ @@ -135,6 +125,11 @@ ErrorOr USER_PROCESS::New(SizeT sz, SizeT pad_amount) { if (!this->HeapTree) { this->HeapTree = new PROCESS_HEAP_TREE(); + if (!this->HeapTree) { + this->Crash(); + return ErrorOr(-kErrorHeapOutOfMemory); + } + this->HeapTree->EntryPad = pad_amount; this->HeapTree->EntrySize = sz; @@ -173,6 +168,11 @@ ErrorOr USER_PROCESS::New(SizeT sz, SizeT pad_amount) { auto new_entry = new PROCESS_HEAP_TREE(); + if (!new_entry) { + this->Crash(); + return ErrorOr(-kErrorHeapOutOfMemory); + } + new_entry->Entry = ptr; new_entry->EntrySize = sz; new_entry->EntryPad = pad_amount; @@ -262,8 +262,6 @@ Void USER_PROCESS::Exit(const Int32& exit_code) { this->LastExitCode = exit_code; this->UTime = 0; - kLastExitCode = exit_code; - --this->ParentTeam->mProcessCur; auto memory_ptr_list = this->HeapTree; @@ -322,7 +320,7 @@ Void USER_PROCESS::Exit(const Int32& exit_code) { /***********************************************************************************/ Bool USER_PROCESS::SpawnDylib() { - // React according to process kind. + // React according to the process's kind. switch (this->Kind) { case USER_PROCESS::kExecutableDylibKind: { this->DylibDelegate = rtl_init_dylib_pef(*this); @@ -338,11 +336,15 @@ Bool USER_PROCESS::SpawnDylib() { return NO; } default: { - (Void)(kout << "Unknown process kind: " << hex_number(this->Kind) << kendl); - this->Crash(); - return NO; + break; } } + + (Void)(kout << "Unknown process kind: " << hex_number(this->Kind) << kendl); + this->Crash(); + return NO; + + return NO; } /***********************************************************************************/ @@ -507,11 +509,11 @@ SizeT UserProcessScheduler::Run() noexcept { // We add a bigger cooldown according to the RTime and affinity here. if (process.PTime < process.RTime && AffinityKind::kRealTime != process.Affinity) { if (process.RTime < (Int32) AffinityKind::kVeryHigh) - process.RTime = (Int32) AffinityKind::kLowUsage / 2; + process.RTime += (Int32) AffinityKind::kLowUsage; else if (process.RTime < (Int32) AffinityKind::kHigh) - process.RTime = (Int32) AffinityKind::kStandard / 3; + process.RTime += (Int32) AffinityKind::kStandard; else if (process.RTime < (Int32) AffinityKind::kStandard) - process.RTime = (Int32) AffinityKind::kHigh / 4; + process.RTime += (Int32) AffinityKind::kHigh; process.PTime -= process.RTime; process.RTime = 0UL; -- cgit v1.2.3 From 14cd1abcbd08240faf30953a9b371ad4a964a475 Mon Sep 17 00:00:00 2001 From: Amlal El Mahrouss Date: Tue, 10 Jun 2025 18:18:46 +0200 Subject: fix: security: UAF on the `ups-allocation-tree` credits: - @0xf00sec who reported the issue. - @amlel-el-mahrouss who implemented the patch. Signed-off-by: Amlal El Mahrouss --- dev/kernel/src/UserProcessScheduler.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'dev/kernel/src/UserProcessScheduler.cc') diff --git a/dev/kernel/src/UserProcessScheduler.cc b/dev/kernel/src/UserProcessScheduler.cc index 6753b238..47a65202 100644 --- a/dev/kernel/src/UserProcessScheduler.cc +++ b/dev/kernel/src/UserProcessScheduler.cc @@ -242,10 +242,13 @@ STATIC Void sched_free_ptr_tree(PROCESS_HEAP_TREE* memory_ptr_list) { auto next = memory_ptr_list->Next; - mm_free_ptr(memory_ptr_list); + if (next->Child) sched_free_ptr_tree(next->Child); + + memory_ptr_list->Child = nullptr; - if (memory_ptr_list->Child) sched_free_ptr_tree(memory_ptr_list->Child); + mm_free_ptr(memory_ptr_list); + memory_ptr_list = nullptr; memory_ptr_list = next; } } @@ -262,16 +265,13 @@ Void USER_PROCESS::Exit(const Int32& exit_code) { this->LastExitCode = exit_code; this->UTime = 0; - --this->ParentTeam->mProcessCur; - - auto memory_ptr_list = this->HeapTree; - #ifdef __NE_VIRTUAL_MEMORY_SUPPORT__ auto pd = kKernelVM; hal_write_cr3(this->VMRegister); #endif - sched_free_ptr_tree(memory_ptr_list); + sched_free_ptr_tree(this->HeapTree); + this->HeapTree = nullptr; #ifdef __NE_VIRTUAL_MEMORY_SUPPORT__ hal_write_cr3(pd); -- cgit v1.2.3 From 3ce7e7ba9251b7fd5642c11dff2ca87032bb5ea4 Mon Sep 17 00:00:00 2001 From: Amlal El Mahrouss Date: Tue, 10 Jun 2025 18:23:56 +0200 Subject: security: fix: memory-leak on `ups-alloc-tree`, and mismatch in traversal. Signed-off-by: Amlal El Mahrouss --- dev/kernel/src/UserProcessScheduler.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'dev/kernel/src/UserProcessScheduler.cc') diff --git a/dev/kernel/src/UserProcessScheduler.cc b/dev/kernel/src/UserProcessScheduler.cc index 47a65202..eff53745 100644 --- a/dev/kernel/src/UserProcessScheduler.cc +++ b/dev/kernel/src/UserProcessScheduler.cc @@ -152,12 +152,10 @@ ErrorOr USER_PROCESS::New(SizeT sz, SizeT pad_amount) { prev_entry = entry; - if (entry->Color == kBlackTreeKind) break; - if (entry->Child && entry->Child->EntrySize > 0 && entry->Child->EntrySize == sz) { entry = entry->Child; is_parent = YES; - } else if (entry->Next && entry->Child->EntrySize > 0 && entry->Next->EntrySize == sz) { + } else if (entry->Next && entry->Next->EntrySize > 0 && entry->Next->EntrySize == sz) { is_parent = NO; entry = entry->Next; } else { -- cgit v1.2.3