From bb12aeb9b9d351c66dfb112b7c5f0431a2939162 Mon Sep 17 00:00:00 2001 From: Daniel Hammer Date: Thu, 14 May 2026 11:31:14 +0200 Subject: [PATCH] fix: kernel and desktop bug fixes --- kernel/src/Hal/Apic/Interrupts.hpp | 4 +- kernel/src/Hal/Apic/IsrStubs.asm | 2 +- kernel/src/Ipc/Ipc.cpp | 375 +++++++++++++++++++--- kernel/src/Memory/Heap.cpp | 85 +++-- kernel/src/Memory/PageFrameAllocator.cpp | 11 + kernel/src/Sched/Scheduler.cpp | 20 +- programs/src/desktop/desktop_internal.hpp | 17 +- programs/src/desktop/input.cpp | 16 +- programs/src/desktop/panel.cpp | 12 +- programs/src/desktop/window.cpp | 59 +++- programs/src/dialogs/filedialog.cpp | 12 +- 11 files changed, 507 insertions(+), 106 deletions(-) diff --git a/kernel/src/Hal/Apic/Interrupts.hpp b/kernel/src/Hal/Apic/Interrupts.hpp index 1d39122..95f84e5 100644 --- a/kernel/src/Hal/Apic/Interrupts.hpp +++ b/kernel/src/Hal/Apic/Interrupts.hpp @@ -11,7 +11,8 @@ namespace Hal { // IRQ handler function type. The parameter is the IRQ number (0-47). using IrqHandler = void(*)(uint8_t irq); - // Number of IRQ slots supported (0-23: legacy ISA via IOAPIC, 24-47: MSI) + // Number of IRQ slots supported (0-23: legacy ISA via IOAPIC, + // 24-45: MSI, 46-47: kernel IPIs) constexpr int IRQ_COUNT = 48; // IRQ vector base: hardware IRQs start at IDT vector 32 @@ -28,6 +29,7 @@ namespace Hal { constexpr uint8_t IRQ_MOUSE = 12; constexpr uint8_t IRQ_ATA1 = 14; constexpr uint8_t IRQ_ATA2 = 15; + constexpr uint8_t IRQ_TLB_SHOOTDOWN = 46; constexpr uint8_t IRQ_RESCHEDULE = 47; // Register a handler for the given IRQ number (0-47) diff --git a/kernel/src/Hal/Apic/IsrStubs.asm b/kernel/src/Hal/Apic/IsrStubs.asm index 9978f40..71a8b66 100644 --- a/kernel/src/Hal/Apic/IsrStubs.asm +++ b/kernel/src/Hal/Apic/IsrStubs.asm @@ -81,7 +81,7 @@ IrqCommon: ; ==================================================================== ; Define stubs for IRQs 0..47 (vectors 32..79) -; 0-23: legacy ISA IRQs via IOAPIC, 24-47: MSI vectors +; 0-23: legacy ISA IRQs via IOAPIC, 24-45: MSI, 46-47: kernel IPIs ; ==================================================================== IRQ_STUB 0 IRQ_STUB 1 diff --git a/kernel/src/Ipc/Ipc.cpp b/kernel/src/Ipc/Ipc.cpp index 8d44d42..9c91e87 100644 --- a/kernel/src/Ipc/Ipc.cpp +++ b/kernel/src/Ipc/Ipc.cpp @@ -15,10 +15,14 @@ #include #include #include +#include +#include +#include #include #include #include #include +#include namespace Ipc { @@ -132,7 +136,13 @@ namespace Ipc { uint32_t pageIndex = (uint32_t)(byteOffset / 0x1000ULL); uint32_t pageOffset = (uint32_t)(byteOffset % 0x1000ULL); if (pageIndex >= surface->numPages) return nullptr; - return (uint32_t*)((uint8_t*)Memory::HHDM(surface->physPages[pageIndex]) + pageOffset); + // A 0 physPage means the slot was freed but not yet repopulated + // (e.g. transiently during a failed ResizeSurface). HHDM(0) is a + // valid kernel virtual address, so returning it would silently + // dereference into kernel memory; return nullptr instead. + uint64_t phys = surface->physPages[pageIndex]; + if (phys == 0) return nullptr; + return (uint32_t*)((uint8_t*)Memory::HHDM(phys) + pageOffset); } static HandleEntry g_handleTables[Sched::MaxProcesses][MaxHandlesPerProcess] = {}; @@ -160,6 +170,100 @@ namespace Ipc { static void ReleaseRawObject(Object* object); + static kcp::Mutex g_tlbShootdownLock; + static volatile uint64_t g_tlbShootdownSeq = 0; + static volatile uint64_t g_tlbShootdownPml4 = 0; + static volatile uint64_t g_tlbShootdownStartVa = 0; + static volatile uint32_t g_tlbShootdownPages = 0; + static volatile uint64_t g_tlbShootdownDone[Smp::MaxCPUs] = {}; + + static bool CpuCurrentlyUsesPml4(Smp::CpuData* cpu, uint64_t pml4Phys) { + if (cpu == nullptr || pml4Phys == 0 || cpu->currentSlot < 0) return false; + + Sched::Process* proc = Sched::GetProcessSlot(cpu->currentSlot); + if (proc == nullptr) return false; + if (proc->state == Sched::ProcessState::Free) return false; + return proc->pml4Phys == pml4Phys; + } + + static void InvalidateLocalUserRange(uint64_t startVa, uint32_t pages) { + if (pages == 0) return; + + if (pages > 1024) { + Memory::VMM::FlushTLB(); + return; + } + + for (uint32_t p = 0; p < pages; p++) { + uint64_t va = startVa + (uint64_t)p * 0x1000ULL; + asm volatile("invlpg (%0)" :: "r"(va) : "memory"); + } + } + + static void TlbShootdownIpiHandler(uint8_t) { + Smp::CpuData* cpu = Smp::GetCurrentCpuData(); + uint64_t seq = g_tlbShootdownSeq; + uint64_t pml4 = g_tlbShootdownPml4; + uint64_t startVa = g_tlbShootdownStartVa; + uint32_t pages = g_tlbShootdownPages; + + if (CpuCurrentlyUsesPml4(cpu, pml4)) { + InvalidateLocalUserRange(startVa, pages); + } + + if (cpu != nullptr && cpu->cpuIndex >= 0 && cpu->cpuIndex < Smp::MaxCPUs) { + asm volatile("" ::: "memory"); + g_tlbShootdownDone[cpu->cpuIndex] = seq; + } + } + + static void ShootdownUserRange(uint64_t pml4Phys, uint64_t startVa, uint32_t pages) { + if (pml4Phys == 0 || pages == 0) return; + + bool targets[Smp::MaxCPUs] = {}; + Smp::CpuData* currentCpu = Smp::GetCurrentCpuData(); + int currentCpuIndex = currentCpu ? currentCpu->cpuIndex : -1; + + g_tlbShootdownLock.Acquire(); + uint64_t seq = g_tlbShootdownSeq + 1; + g_tlbShootdownPml4 = pml4Phys; + g_tlbShootdownStartVa = startVa; + g_tlbShootdownPages = pages; + asm volatile("" ::: "memory"); + g_tlbShootdownSeq = seq; + + for (int i = 0; i < Smp::GetCpuCount(); i++) { + Smp::CpuData* cpu = Smp::GetCpuData(i); + if (cpu == nullptr || !cpu->started) continue; + + if (i == currentCpuIndex) { + if (CpuCurrentlyUsesPml4(cpu, pml4Phys)) { + InvalidateLocalUserRange(startVa, pages); + } + g_tlbShootdownDone[i] = seq; + continue; + } + + if (!CpuCurrentlyUsesPml4(cpu, pml4Phys)) { + g_tlbShootdownDone[i] = seq; + continue; + } + + targets[i] = true; + Hal::LocalApic::SendFixedIpi(cpu->lapicId, + Hal::IRQ_VECTOR_BASE + Hal::IRQ_TLB_SHOOTDOWN); + } + + for (int i = 0; i < Smp::GetCpuCount(); i++) { + if (!targets[i]) continue; + while (g_tlbShootdownDone[i] != seq) { + asm volatile("pause"); + } + } + + g_tlbShootdownLock.Release(); + } + static void InitObject(Object& object, HandleType type) { object.type = type; object.active = true; @@ -374,14 +478,15 @@ namespace Ipc { static void* AllocContiguousPages(int numPages) { if (numPages <= 0) return nullptr; - void* first = Memory::g_pfa->AllocateZeroed(); - if (first == nullptr) return nullptr; - if (numPages == 1) return first; - void* span = Memory::g_pfa->ReallocConsecutive(first, numPages); - if (span == nullptr) { - Memory::g_pfa->Free(first); - return nullptr; - } + if (numPages == 1) return Memory::g_pfa->AllocateZeroed(); + + // ReallocConsecutive(nullptr, n) allocates an n-page contiguous span + // directly. The previous implementation always allocated a single + // throwaway page first, then asked ReallocConsecutive to migrate from + // it -- which copies and frees the throwaway page for no benefit. + void* span = Memory::g_pfa->ReallocConsecutive(nullptr, numPages); + if (span == nullptr) return nullptr; + memset(span, 0, (size_t)numPages * 0x1000); return span; } @@ -669,32 +774,36 @@ namespace Ipc { return -1; } - int written = 0; uint32_t head = stream->head; uint32_t cap = stream->capacity; uint32_t space = cap - stream->count; - int first = ((uint32_t)len < space) ? ((head + (uint32_t)len <= cap) ? len : (cap - head)) : (cap - head); + + // Total bytes we can accept is bounded by both requested length and free space. + // The previous implementation could write up to (cap - head) bytes whenever + // len >= space, which exceeded `space` whenever cap-head > space and silently + // corrupted the ring buffer (count > cap, overwritten unread data). + uint32_t toWrite = ((uint32_t)len < space) ? (uint32_t)len : space; + + // First contiguous chunk: from head to end of buffer + uint32_t first = (toWrite < (cap - head)) ? toWrite : (cap - head); if (first > 0) { memcpy(stream->buffer + head, data, first); head = (head + first) % cap; stream->count += first; - written = first; } - if (written < len && stream->count < cap) { - int second = len - written; - if (second > (int)(cap - stream->count)) second = cap - stream->count; - if (second > 0) { - memcpy(stream->buffer + head, data + written, second); - head = (head + second) % cap; - stream->count += second; - written += second; - } + // Remainder wraps to the start of the buffer + uint32_t second = toWrite - first; + if (second > 0) { + memcpy(stream->buffer + head, data + first, second); + head = (head + second) % cap; + stream->count += second; } + stream->head = head; stream->lock.Release(); - if (written > 0) NotifyObjectChanged((Object*)stream); - return written; + if (toWrite > 0) NotifyObjectChanged((Object*)stream); + return (int)toWrite; } int StreamReadHandle(int handle, uint8_t* out, int maxLen) { @@ -1447,29 +1556,179 @@ namespace Ipc { return surface->sizeBytes; } + static void RollbackSurfaceGrowMaps(Surface* surface, uint32_t newPages) { + for (int s = 0; s < Sched::MaxProcesses; s++) { + auto* proc = Sched::GetProcessSlot(s); + if (proc == nullptr) continue; + if (proc->state == Sched::ProcessState::Free) continue; + uint64_t pml4 = proc->pml4Phys; + if (pml4 == 0) continue; + + for (int i = 0; i < MaxSurfaceMapsPerProcess; i++) { + auto& m = g_surfaceMaps[s][i]; + if (!m.used || m.surface != surface) continue; + if (newPages <= m.numPages) continue; + + uint64_t startVa = m.va + (uint64_t)m.numPages * 0x1000ULL; + uint32_t rollbackPages = newPages - m.numPages; + for (uint32_t p = m.numPages; p < newPages; p++) { + Memory::VMM::Paging::UnmapUserIn(pml4, m.va + (uint64_t)p * 0x1000ULL); + } + ShootdownUserRange(pml4, startVa, rollbackPages); + } + } + } + + static bool PreinstallSurfaceGrowMaps(Surface* surface, uint64_t* scratch, uint32_t newPages) { + for (int s = 0; s < Sched::MaxProcesses; s++) { + auto* proc = Sched::GetProcessSlot(s); + if (proc == nullptr) continue; + if (proc->state == Sched::ProcessState::Free) continue; + uint64_t pml4 = proc->pml4Phys; + if (pml4 == 0) continue; + + for (int i = 0; i < MaxSurfaceMapsPerProcess; i++) { + auto& m = g_surfaceMaps[s][i]; + if (!m.used || m.surface != surface) continue; + if (newPages <= m.numPages) continue; + + for (uint32_t p = m.numPages; p < newPages; p++) { + uint64_t va = m.va + (uint64_t)p * 0x1000ULL; + if (!Memory::VMM::Paging::MapUserIn(pml4, scratch[p], va)) { + RollbackSurfaceGrowMaps(surface, newPages); + return false; + } + } + } + } + + return true; + } + int ResizeSurface(Surface* surface, uint64_t newSize) { if (surface == nullptr) return -1; if (newSize == 0) newSize = 0x1000; - uint32_t newPages = (uint32_t)((newSize + 0xFFFu) / 0x1000u); + if (newSize > (uint64_t)MaxSurfacePages * 0x1000ULL) return -1; + uint32_t newPages = (uint32_t)((newSize + 0xFFFULL) / 0x1000ULL); if (newPages == 0 || newPages > MaxSurfacePages) return -1; - surface->lock.Acquire(); - for (uint32_t i = 0; i < surface->numPages; i++) { - if (surface->physPages[i] != 0) { - Memory::g_pfa->Free((void*)Memory::HHDM(surface->physPages[i])); - surface->physPages[i] = 0; - } - } - surface->numPages = newPages; - surface->sizeBytes = (uint64_t)newPages * 0x1000ULL; - surface->lock.Release(); - + // Transactional resize: allocate all new pages BEFORE freeing the old + // ones, then swap them in under the lock. The previous implementation + // freed first and then allocated outside the lock, leaving readers + // with numPages updated but physPages still zero (or transiently + // freed) for the entire allocation window; on partial failure the + // surface was left in a half-shrunken half-allocated state. + // Heap-allocate the scratch arrays: with MaxSurfacePages=8192, two + // uint64_t[8192] arrays are 128 KiB - far past the 16 KiB kernel stack. + uint64_t* scratch = new uint64_t[newPages]; + if (scratch == nullptr) return -1; + for (uint32_t i = 0; i < newPages; i++) scratch[i] = 0; for (uint32_t i = 0; i < newPages; i++) { void* page = Memory::g_pfa->AllocateZeroed(); - if (page == nullptr) return -1; - surface->physPages[i] = Memory::SubHHDM((uint64_t)page); + if (page == nullptr) { + for (uint32_t j = 0; j < i; j++) { + Memory::g_pfa->Free((void*)Memory::HHDM(scratch[j])); + } + delete[] scratch; + return -1; + } + scratch[i] = Memory::SubHHDM((uint64_t)page); } + + surface->lock.Acquire(); + uint32_t oldNumPages = surface->numPages; + uint64_t* oldPages = (oldNumPages > 0) ? new uint64_t[oldNumPages] : nullptr; + if (oldNumPages > 0 && oldPages == nullptr) { + surface->lock.Release(); + for (uint32_t i = 0; i < newPages; i++) { + Memory::g_pfa->Free((void*)Memory::HHDM(scratch[i])); + } + delete[] scratch; + return -1; + } + for (uint32_t i = 0; i < oldNumPages; i++) oldPages[i] = surface->physPages[i]; + + // Grow mappings can require new page-table pages. Preinstall the + // additional leaf PTEs before changing the surface metadata so an OOM + // can roll back cleanly instead of reporting a successful resize with + // only some processes mapped to the larger range. + if (!PreinstallSurfaceGrowMaps(surface, scratch, newPages)) { + surface->lock.Release(); + for (uint32_t i = 0; i < newPages; i++) { + Memory::g_pfa->Free((void*)Memory::HHDM(scratch[i])); + } + delete[] oldPages; + delete[] scratch; + return -1; + } + + for (uint32_t i = 0; i < newPages; i++) surface->physPages[i] = scratch[i]; + for (uint32_t i = newPages; i < oldNumPages; i++) surface->physPages[i] = 0; + surface->numPages = newPages; + surface->sizeBytes = (uint64_t)newPages * 0x1000ULL; + + // Refresh every existing mapping of this surface so each mapper's PTEs + // point at the new physical frames before we hand the old frames back + // to the PFA. Without this, MapSurfaceHandle re-returns the cached VA + // (see MapSurfaceForPid early-return) and any process that mapped the + // surface before the resize keeps writing to freed pages -- corrupting + // whatever the PFA next hands them out for. + // + // We hold surface->lock during the walk so g_surfaceMaps reads stay + // coherent with the physPages snapshot we just committed. MapUserIn + // overwriting an existing leaf PTE doesn't free the old frame -- that + // happens below, after every mapping has been redirected and every CPU + // currently running the affected address space has invalidated the + // touched user range. + for (int s = 0; s < Sched::MaxProcesses; s++) { + auto* proc = Sched::GetProcessSlot(s); + if (proc == nullptr) continue; + if (proc->state == Sched::ProcessState::Free) continue; + uint64_t pml4 = proc->pml4Phys; + if (pml4 == 0) continue; + + for (int i = 0; i < MaxSurfaceMapsPerProcess; i++) { + auto& m = g_surfaceMaps[s][i]; + if (!m.used || m.surface != surface) continue; + + uint64_t baseVa = m.va; + uint32_t mapPages = m.numPages; + uint32_t common = (mapPages < newPages) ? mapPages : newPages; + uint32_t flushPages = (mapPages > newPages) ? mapPages : newPages; + + // Pages that exist in both old and new: redirect PTE to the + // new physical frame. + // MapUserIn here only overwrites the leaf PTE -- all page + // table levels are guaranteed present from the original map, + // so walkLevel never has to allocate and cannot fail. Treat + // failure as a kernel-state-corruption panic rather than a + // recoverable error. + for (uint32_t p = 0; p < common; p++) { + uint64_t va = baseVa + (uint64_t)p * 0x1000ULL; + if (!Memory::VMM::Paging::MapUserIn(pml4, surface->physPages[p], va)) { + Panic("ResizeSurface: MapUserIn failed refreshing existing PTE", nullptr); + } + } + // Pages trimmed off the end on shrink: unmap them entirely. + for (uint32_t p = newPages; p < mapPages; p++) { + uint64_t va = baseVa + (uint64_t)p * 0x1000ULL; + Memory::VMM::Paging::UnmapUserIn(pml4, va); + } + + ShootdownUserRange(pml4, baseVa, flushPages); + m.numPages = newPages; + } + } + surface->lock.Release(); + + for (uint32_t i = 0; i < oldNumPages; i++) { + if (oldPages[i] != 0) { + Memory::g_pfa->Free((void*)Memory::HHDM(oldPages[i])); + } + } + delete[] oldPages; + delete[] scratch; NotifyObjectChanged((Object*)surface); return 0; } @@ -1553,9 +1812,21 @@ namespace Ipc { int slot = SlotForPid(pid); if (slot < 0) return -1; + // surface->lock serialises us against ResizeSurface so the + // numPages/physPages snapshot we install into the caller's page tables + // is internally consistent. Without this, a resize landing mid-loop + // could surface zero/freed physical addresses (see ResizeSurface), + // causing MapUserIn to map PA 0 into the caller's address space. + // + // RetainRawObject also takes surface->lock (Surface inherits its lock + // from Object), so the refcount bump is inlined below to avoid + // self-deadlocking on the non-reentrant mutex. + surface->lock.Acquire(); + for (int i = 0; i < MaxSurfaceMapsPerProcess; i++) { if (g_surfaceMaps[slot][i].used && g_surfaceMaps[slot][i].surface == surface) { outVa = g_surfaceMaps[slot][i].va; + surface->lock.Release(); return 0; } } @@ -1567,15 +1838,20 @@ namespace Ipc { break; } } - if (mapIdx < 0) return -1; + if (mapIdx < 0) { + surface->lock.Release(); + return -1; + } + uint32_t numPages = surface->numPages; uint64_t baseVa = heapNext; - for (uint32_t i = 0; i < surface->numPages; i++) { + for (uint32_t i = 0; i < numPages; i++) { if (!Memory::VMM::Paging::MapUserIn(pml4Phys, surface->physPages[i], baseVa + (uint64_t)i * 0x1000ULL)) { for (uint32_t j = 0; j < i; j++) { Memory::VMM::Paging::UnmapUserIn(pml4Phys, baseVa + (uint64_t)j * 0x1000ULL); } + surface->lock.Release(); return -1; } } @@ -1583,11 +1859,25 @@ namespace Ipc { g_surfaceMaps[slot][mapIdx].used = true; g_surfaceMaps[slot][mapIdx].surface = surface; g_surfaceMaps[slot][mapIdx].va = baseVa; - g_surfaceMaps[slot][mapIdx].numPages = surface->numPages; - RetainRawObject((Object*)surface); + g_surfaceMaps[slot][mapIdx].numPages = numPages; + surface->refs++; // inlined RetainRawObject — we already hold surface->lock - heapNext += (uint64_t)surface->numPages * 0x1000ULL; + // Reserve the full surface address-range cap, not just the currently + // mapped pages. ResizeSurface can grow this surface up to + // MaxSurfacePages, and its grow path extends the existing VA range + // in-place by mapping new pages at baseVa + p*0x1000 for p beyond the + // original size. If we only bumped heapNext by numPages*0x1000, any + // subsequent mapping (another surface, heap allocation, etc.) would + // sit at baseVa + numPages*0x1000 -- and a later grow would silently + // overwrite its PTEs. Reserving MaxSurfacePages*0x1000 (32 MiB) of + // VA per mapping is cheap: 47-bit user VA gives 128 TiB of headroom, + // and at most MaxSurfaceMapsPerProcess (64) mappings per process + // means at most 2 GiB of reserved VA per process. Page tables are + // only populated for pages actually mapped; unused VA in the + // reservation costs nothing beyond the address-space slot. + heapNext += (uint64_t)MaxSurfacePages * 0x1000ULL; outVa = baseVa; + surface->lock.Release(); return 0; } @@ -1982,6 +2272,7 @@ namespace Ipc { for (int i = 0; i < Sched::MaxProcesses; i++) { g_processObjectsBySlot[i] = nullptr; } + Hal::RegisterIrqHandler(Hal::IRQ_TLB_SHOOTDOWN, TlbShootdownIpiHandler); Kt::KernelLogStream(Kt::OK, "IPC") << "Initialized (" << (uint64_t)MaxHandlesPerProcess << " handles/process, " << (uint64_t)MaxStreams << " streams, " diff --git a/kernel/src/Memory/Heap.cpp b/kernel/src/Memory/Heap.cpp index 7ac29e2..d155470 100644 --- a/kernel/src/Memory/Heap.cpp +++ b/kernel/src/Memory/Heap.cpp @@ -29,12 +29,47 @@ namespace Memory return header->size; } + // Insert a free block, keeping the list sorted by address and coalescing with + // any adjacent neighbors. Coalescing keeps the free list compact and prevents + // fragmentation from accumulating across alloc/free cycles, which is otherwise + // pathological because Realloc always allocates a fresh block. void HeapAllocator::InsertToFreelist(void* ptr, std::size_t size) { - auto prev_next = head.next; - - head.next = (Node*)ptr; - head.next->next = prev_next; - head.next->size = size; + if (ptr == nullptr || size == 0) return; + + uintptr_t addr = (uintptr_t)ptr; + + Node* prev = &head; + Node* current = head.next; + while (current != nullptr && (uintptr_t)current < addr) { + prev = current; + current = current->next; + } + + bool mergedPrev = false; + if (prev != &head) { + uintptr_t prevEnd = (uintptr_t)prev + prev->size; + if (prevEnd == addr) { + prev->size += size; + mergedPrev = true; + } + } + + if (current != nullptr && addr + size == (uintptr_t)current) { + if (mergedPrev) { + prev->size += current->size; + prev->next = current->next; + } else { + Node* newNode = (Node*)addr; + newNode->size = size + current->size; + newNode->next = current->next; + prev->next = newNode; + } + } else if (!mergedPrev) { + Node* newNode = (Node*)addr; + newNode->size = size; + newNode->next = current; + prev->next = newNode; + } } void HeapAllocator::InsertPageToFreelist() { @@ -56,7 +91,10 @@ namespace Memory void* HeapAllocator::Request(size_t size) { Lock.Acquire(); - size_t sizeNeeded = size + sizeof(Header); + // Round the user request up so allocated blocks are 8-byte aligned. The + // Header itself is 16 bytes, so the returned pointer is always aligned. + size_t alignedSize = (size + 7u) & ~(size_t)7u; + size_t sizeNeeded = alignedSize + sizeof(Header); retry: Node* current = head.next; @@ -64,21 +102,24 @@ namespace Memory while (current != nullptr) { if (current->size >= sizeNeeded) { - // Unlink the node auto locatedBlockSize = current->size; + // Only split if the leftover region can hold a free-list Node. + // Otherwise hand out the whole block; storing a sub-Node-sized + // remainder would write the Node header past the block. + bool canSplit = (locatedBlockSize >= sizeNeeded + sizeof(Node)); + prev->next = current->next; Header* header = (Header*)current; header->magic = headerMagic; - header->size = size; + header->size = canSplit ? alignedSize : (locatedBlockSize - sizeof(Header)); void* block = (void*)((uintptr_t)header + sizeof(Header)); - if (locatedBlockSize > sizeNeeded) { + if (canSplit) { void* rest = (void*)((uintptr_t)header + sizeNeeded); auto newBlockSize = locatedBlockSize - sizeNeeded; - InsertToFreelist(rest, newBlockSize); } @@ -92,7 +133,9 @@ namespace Memory // No suitable block found -- grow the heap under the lock so // InsertToFreelist is protected from concurrent modification. - size_t pagesNeeded = (sizeNeeded + 0xFFF) / 0x1000; + // Ensure the new block leaves room for the Header *and* a splittable + // free-list Node, so even worst-case requests succeed after growth. + size_t pagesNeeded = (sizeNeeded + sizeof(Node) + 0xFFF) / 0x1000; InsertPagesToFreelist(pagesNeeded); goto retry; } @@ -111,20 +154,24 @@ namespace Memory } void HeapAllocator::Free(void* ptr) { - Lock.Acquire(); - + if (ptr == nullptr) return; + Header* header = GetHeader(ptr); - auto size = header->size; - + + Lock.Acquire(); + + // Validate the magic under the lock so that two concurrent frees of + // the same pointer can't both pass the check and both insert. The + // magic is cleared inside the lock for the same reason. if (header->magic != headerMagic) { + Lock.Release(); Panic("Bad magic in HeapAllocator header", nullptr); return; } - auto actualSize = size + sizeof(Header); - void* actualBlock = (void*)header; - - InsertToFreelist(actualBlock, actualSize); + size_t size = header->size; + header->magic = 0; + InsertToFreelist((void*)header, size + sizeof(Header)); Lock.Release(); } diff --git a/kernel/src/Memory/PageFrameAllocator.cpp b/kernel/src/Memory/PageFrameAllocator.cpp index 8e2d6eb..e57d0dc 100644 --- a/kernel/src/Memory/PageFrameAllocator.cpp +++ b/kernel/src/Memory/PageFrameAllocator.cpp @@ -143,6 +143,17 @@ namespace Memory { return; } + // Overlap check: the new region must end at or before `current` starts. + // The previous double-free guards only matched single-page overlaps; + // freeing a multi-page span that overlaps the start of an existing + // block would silently corrupt the free list. + if (current != nullptr && addr + size > (uint64_t)current) { + Kt::KernelLogStream(Kt::WARNING, "PFA") + << "Overlapping free at " << addr << " size " << size << ", ignoring"; + Lock.Release(); + return; + } + // Try to coalesce with previous block (if prev ends where new block starts) bool merged_prev = false; if (prev != &head) { diff --git a/kernel/src/Sched/Scheduler.cpp b/kernel/src/Sched/Scheduler.cpp index d868960..1601cd1 100644 --- a/kernel/src/Sched/Scheduler.cpp +++ b/kernel/src/Sched/Scheduler.cpp @@ -243,21 +243,14 @@ namespace Sched { return -1; } - // Allocate kernel stack (used during syscalls and interrupts) - void* firstPage = Memory::g_pfa->AllocateZeroed(); - if (firstPage == nullptr) { - Kt::KernelLogStream(Kt::ERROR, "Sched") << "Out of memory for kernel stack"; - Memory::VMM::Paging::FreeUserHalf(pml4Phys); - Memory::g_pfa->Free((void*)Memory::HHDM(pml4Phys)); - schedLock.Acquire(); - processTable[slot].state = ProcessState::Free; - schedLock.Release(); - return -1; - } - void* stackMem = Memory::g_pfa->ReallocConsecutive(firstPage, StackPages); + // Allocate kernel stack (used during syscalls and interrupts). + // ReallocConsecutive(nullptr, n) returns an n-page contiguous span; + // the previous code allocated a throwaway page first and asked + // ReallocConsecutive to migrate from it, which copied + freed the + // throwaway for no benefit and double-counted failure paths. + void* stackMem = Memory::g_pfa->ReallocConsecutive(nullptr, StackPages); if (stackMem == nullptr) { Kt::KernelLogStream(Kt::ERROR, "Sched") << "Failed to allocate contiguous kernel stack"; - Memory::g_pfa->Free(firstPage); Memory::VMM::Paging::FreeUserHalf(pml4Phys); Memory::g_pfa->Free((void*)Memory::HHDM(pml4Phys)); schedLock.Acquire(); @@ -265,6 +258,7 @@ namespace Sched { schedLock.Release(); return -1; } + memset(stackMem, 0, StackPages * 0x1000); uint8_t* kernelStackBase = (uint8_t*)stackMem; uint64_t kernelStackTop = (uint64_t)kernelStackBase + StackSize; diff --git a/programs/src/desktop/desktop_internal.hpp b/programs/src/desktop/desktop_internal.hpp index 471ba2d..16d67a1 100644 --- a/programs/src/desktop/desktop_internal.hpp +++ b/programs/src/desktop/desktop_internal.hpp @@ -58,10 +58,21 @@ inline int menu_row_height(const MenuRow& row) { } inline int menu_total_height() { + // Single-pass walk to avoid O(N^2) from menu_row_visible repeatedly + // counting categories from the start. int h = 10; // top + bottom padding - for (int i = 0; i < menu_row_count; i++) - if (menu_row_visible(i)) - h += menu_row_height(menu_rows[i]); + int cur_cat = -1; + for (int i = 0; i < menu_row_count; i++) { + const MenuRow& row = menu_rows[i]; + if (row.is_category) cur_cat++; + bool visible = true; + if (!row.is_category) { + if (cur_cat >= 0 && cur_cat < MENU_NUM_CATS) { + visible = menu_cat_expanded[cur_cat]; + } + } + if (visible) h += menu_row_height(row); + } return h; } diff --git a/programs/src/desktop/input.cpp b/programs/src/desktop/input.cpp index 0fd36a7..f50ec8c 100644 --- a/programs/src/desktop/input.cpp +++ b/programs/src/desktop/input.cpp @@ -677,8 +677,10 @@ void gui::desktop_handle_mouse(DesktopState* ds) { ds->vol_popup_open = false; } - // Forward continuous mouse events to focused window (hover, drag, release) - if (!left_pressed && ds->focused_window >= 0) { + // Forward continuous mouse events to focused window (hover, drag, release). + // Scroll is sent separately below; clearing ev.scroll here prevents local + // on_mouse handlers from receiving the same scroll twice. + if (!left_pressed && ds->focused_window >= 0 && ds->focused_window < ds->window_count) { Window* win = &ds->windows[ds->focused_window]; if (win->state != WIN_MINIMIZED && win->state != WIN_CLOSED) { Rect cr = desktop_content_rect(win); @@ -695,16 +697,18 @@ void gui::desktop_handle_mouse(DesktopState* ds) { wev.mouse.prev_buttons = prev; montauk::win_sendevent(win->ext_win_id, &wev); } else if (win->on_mouse) { - ev.x = mx; - ev.y = my; - win->on_mouse(win, ev); + MouseEvent hover = ev; + hover.x = mx; + hover.y = my; + hover.scroll = 0; + win->on_mouse(win, hover); } } } } // Handle scroll events on focused window - if (ev.scroll != 0 && ds->focused_window >= 0) { + if (ev.scroll != 0 && ds->focused_window >= 0 && ds->focused_window < ds->window_count) { Window* win = &ds->windows[ds->focused_window]; Rect cr = desktop_content_rect(win); if (cr.contains(mx, my)) { diff --git a/programs/src/desktop/panel.cpp b/programs/src/desktop/panel.cpp index a667bc5..636e0f0 100644 --- a/programs/src/desktop/panel.cpp +++ b/programs/src/desktop/panel.cpp @@ -10,14 +10,16 @@ void gui::desktop_draw_panel(DesktopState* ds) { Framebuffer& fb = ds->fb; int sw = ds->screen_w; - // Panel gradient background (slightly lighter at top) + // Panel gradient background (slightly lighter at top). + // Saturating add so light panel colors don't wrap around to dark via uint8_t overflow. Color pc = ds->settings.panel_color; for (int y = 0; y < PANEL_HEIGHT; y++) { int t = y * 255 / PANEL_HEIGHT; - uint8_t r = pc.r + (10 - t * 10 / 255); - uint8_t g = pc.g + (10 - t * 10 / 255); - uint8_t b = pc.b + (10 - t * 10 / 255); - fb.fill_rect(0, y, sw, 1, Color::from_rgb(r, g, b)); + int delta = 10 - t * 10 / 255; + int r = (int)pc.r + delta; if (r > 255) r = 255; + int g = (int)pc.g + delta; if (g > 255) g = 255; + int b = (int)pc.b + delta; if (b > 255) b = 255; + fb.fill_rect(0, y, sw, 1, Color::from_rgb((uint8_t)r, (uint8_t)g, (uint8_t)b)); } // Bottom highlight line (skip when wallpaper is set — the white bleeds through) diff --git a/programs/src/desktop/window.cpp b/programs/src/desktop/window.cpp index 68eda5a..9937d02 100644 --- a/programs/src/desktop/window.cpp +++ b/programs/src/desktop/window.cpp @@ -72,8 +72,14 @@ void gui::desktop_close_window(DesktopState* ds, int idx) { if (win->on_close) win->on_close(win); - // Free content buffer (skip for external windows — shared memory) - if (win->content && !win->external) { + if (win->external) { + // Drop our mapping of the shared pixel buffer so the page table doesn't + // keep leaking mappings each time the user closes an external window. + if (win->content != nullptr) { + montauk::win_unmap(win->ext_win_id); + win->content = nullptr; + } + } else if (win->content) { montauk::free(win->content); win->content = nullptr; } @@ -182,23 +188,46 @@ void gui::desktop_draw_window(DesktopState* ds, int idx) { Rect cr = desktop_content_rect(win); if (win->content) { if (win->external && (cr.w != win->content_w || cr.h != win->content_h)) { - // Nearest-neighbor scale for external windows (fixed-size shared buffer) + // Nearest-neighbor scale for external windows (fixed-size shared buffer). + // Pre-clip the destination range so the inner loop avoids per-pixel + // framebuffer bounds checks. Avoid stack-allocated lookup tables — + // the desktop user stack is only 32 KiB and shared with TrueType. int src_w = win->content_w; int src_h = win->content_h; int dst_w = cr.w; int dst_h = cr.h; - uint32_t* buf = fb.buffer(); - int pitch = fb.pitch(); - for (int y = 0; y < dst_h; y++) { - int dy = cr.y + y; - if (dy < 0 || dy >= fb.height()) continue; - int sy = y * src_h / dst_h; - uint32_t* dst_row = (uint32_t*)((uint8_t*)buf + dy * pitch); - uint32_t* src_row = win->content + sy * src_w; - for (int x = 0; x < dst_w; x++) { - int dx = cr.x + x; - if (dx < 0 || dx >= fb.width()) continue; - dst_row[dx] = src_row[x * src_w / dst_w]; + if (src_w <= 0 || src_h <= 0 || dst_w <= 0 || dst_h <= 0) { + // nothing to do + } else { + int fbw = fb.width(); + int fbh = fb.height(); + uint32_t* buf = fb.buffer(); + int pitch = fb.pitch(); + + int x_start = 0; + int x_end = dst_w; + if (cr.x < 0) x_start = -cr.x; + if (cr.x + dst_w > fbw) x_end = fbw - cr.x; + if (x_start < 0) x_start = 0; + if (x_end > dst_w) x_end = dst_w; + int y_start = 0; + int y_end = dst_h; + if (cr.y < 0) y_start = -cr.y; + if (cr.y + dst_h > fbh) y_end = fbh - cr.y; + if (y_start < 0) y_start = 0; + if (y_end > dst_h) y_end = dst_h; + + for (int y = y_start; y < y_end; y++) { + int dy = cr.y + y; + int sy = y * src_h / dst_h; + if (sy >= src_h) sy = src_h - 1; + uint32_t* dst_row = (uint32_t*)((uint8_t*)buf + dy * pitch); + uint32_t* src_row = win->content + sy * src_w; + for (int x = x_start; x < x_end; x++) { + int sx = x * src_w / dst_w; + if (sx >= src_w) sx = src_w - 1; + dst_row[cr.x + x] = src_row[sx]; + } } } } else { diff --git a/programs/src/dialogs/filedialog.cpp b/programs/src/dialogs/filedialog.cpp index 297fcfa..f966c39 100644 --- a/programs/src/dialogs/filedialog.cpp +++ b/programs/src/dialogs/filedialog.cpp @@ -384,7 +384,11 @@ void file_push_history(FileDialogState* st) { if (montauk::streq(st->history[st->history_pos], st->current_path)) return; } st->history_pos++; - if (st->history_pos >= MAX_HISTORY) st->history_pos = MAX_HISTORY - 1; + if (st->history_pos >= MAX_HISTORY) { + for (int i = 1; i < MAX_HISTORY; i++) + safe_copy(st->history[i - 1], sizeof(st->history[i - 1]), st->history[i]); + st->history_pos = MAX_HISTORY - 1; + } safe_copy(st->history[st->history_pos], sizeof(st->history[st->history_pos]), st->current_path); st->history_count = st->history_pos + 1; } @@ -664,6 +668,12 @@ void file_activate_selected(FileDialogState* st, bool from_double_click) { } void file_go_home(FileDialogState* st) { + if (st->home_dir[0] && is_dir_path(st->home_dir)) { + safe_copy(st->current_path, sizeof(st->current_path), st->home_dir); + file_read_dir(st); + file_push_history(st); + return; + } file_read_drives(st); file_push_history(st); }