From b0df5186e6d10ae72b1109b9d974ad341aa35bca Mon Sep 17 00:00:00 2001 From: Jonas 'Sortie' Termansen Date: Thu, 20 Oct 2011 03:40:37 +0200 Subject: [PATCH] Fixed two very nasty bugs in the x86 memory management code. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1) The PML2 was not initialized to zeroes, thus leaving some bits behind that caused the fork code to go crazy, forking the unforkable, and mapping addresses that never, ever, should have been mapped, leaving behind a trail of page faults and general protection faults on some computers, while other computers worked because the uninitalized memory just wasn't uninitialized enough. Yep, this was a schrödinbug! 2) Fixed a time bomb. The kernel heap was accidentally put such that whenever a few megabytes were allocated, it would begin overwriting the physical page stack causing unthinkable events to unfold and would probably be even more obscure to debug than 1). Oh, and some string errors fixed and removed RunApplication from kernel.cpp, funny thing that even linked in the first place. Guess, the optimizer actually did work for once. :) --- sortix/kernel.cpp | 2 +- sortix/memorymanagement.h | 2 +- sortix/x86-family/memorymanagement.cpp | 15 ++++++++++----- sortix/x86/memorymanagement.cpp | 2 +- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/sortix/kernel.cpp b/sortix/kernel.cpp index 7fa8d90d..8e6258c9 100644 --- a/sortix/kernel.cpp +++ b/sortix/kernel.cpp @@ -226,7 +226,7 @@ namespace Sortix // Initialize the scheduler. Scheduler::Init(); - Thread::Entry initstart = RunApplication; + Thread::Entry initstart = NULL; // Create an address space for the first process. addr_t addrspace = Memory::Fork(); diff --git a/sortix/memorymanagement.h b/sortix/memorymanagement.h index 986e0dd0..460d3ba9 100644 --- a/sortix/memorymanagement.h +++ b/sortix/memorymanagement.h @@ -60,7 +60,7 @@ namespace Sortix #if defined(PLATFORM_X86) const addr_t HEAPLOWER = 0x80000000UL; - const addr_t HEAPUPPER = 0xFF800000UL; + const addr_t HEAPUPPER = 0xFF400000UL; #elif defined(PLATFORM_X64) const addr_t HEAPLOWER = 0xFFFF800000000000UL; const addr_t HEAPUPPER = 0xFFFFFE8000000000UL; diff --git a/sortix/x86-family/memorymanagement.cpp b/sortix/x86-family/memorymanagement.cpp index b72db72f..e0a4b7d9 100644 --- a/sortix/x86-family/memorymanagement.cpp +++ b/sortix/x86-family/memorymanagement.cpp @@ -129,6 +129,11 @@ namespace Sortix if ( !page ) { Panic("out of memory allocating boot PMLs"); } pml->entry[i] = page | flags; + + // Invalidate the new PML and reset it to zeroes. + addr_t pmladdr = (addr_t) (PMLS[TOPPMLLEVEL-1] + i); + InvalidatePage(pmladdr); + Maxsi::Memory::Set((void*) pmladdr, 0, sizeof(PML)); } } } @@ -318,8 +323,8 @@ namespace Sortix else if ( userspace && !(entry & PML_USERSPACE) ) { PanicF("attempted to map physical page %p to virtual page " - "%p with userspace permissions, but the virtual page" - "wasn't in an userspace PML[%zu]. This is a bug in the" + "%p with userspace permissions, but the virtual page " + "wasn't in an userspace PML[%zu]. This is a bug in the " "code calling this function", physical, mapto, i-1); } @@ -362,8 +367,8 @@ namespace Sortix } else if ( userspace && !(entry & PML_USERSPACE) ) { - PanicF("attempted to unmap virtual page %p it wasn't in an" - "userspace PML[%zu]. This is a bug in the code" + PanicF("attempted to unmap virtual page %p it wasn't in an " + "userspace PML[%zu]. This is a bug in the code " "calling this function", mapto, i-1); } @@ -437,7 +442,7 @@ namespace Sortix addr_t entry = (PMLS[level] + pmloffset)->entry[pos]; // If the entry should be forked, fork it! - if ( likely(entry & PML_FORK) ) + if ( (entry & PML_FORK) && (entry & PML_PRESENT) ) { // Pop the physical address of somewhere unused. addr_t phys = Page::Get(); diff --git a/sortix/x86/memorymanagement.cpp b/sortix/x86/memorymanagement.cpp index fc78619d..1ff2f40a 100644 --- a/sortix/x86/memorymanagement.cpp +++ b/sortix/x86/memorymanagement.cpp @@ -49,7 +49,7 @@ namespace Sortix PML* const IDENPML1 = (PML* const) 0x04000UL; // Initialize the memory structures with zeroes. - Maxsi::Memory::Set(BOOTPML1, 0, 0x6000UL); + Maxsi::Memory::Set((PML* const) 0x01000UL, 0, 0x6000UL); // Identity map the first 4 MiB. addr_t flags = PML_PRESENT | PML_WRITABLE;