From a965e67c9394bc58dc3431a46e1f9afc20af9806 Mon Sep 17 00:00:00 2001 From: Daniel Hammer Date: Thu, 30 Jul 2026 17:46:38 +0100 Subject: [PATCH] fix: prevent IPC slot reuse during object teardown --- kernel/src/Api/BuildNo.hpp | 2 +- kernel/src/Ipc/Ipc.cpp | 58 ++++++++++++++++++++++++++----- programs/include/gui/truetype.hpp | 7 +++- 3 files changed, 56 insertions(+), 11 deletions(-) diff --git a/kernel/src/Api/BuildNo.hpp b/kernel/src/Api/BuildNo.hpp index 0163127..2a58c51 100644 --- a/kernel/src/Api/BuildNo.hpp +++ b/kernel/src/Api/BuildNo.hpp @@ -12,4 +12,4 @@ #pragma once -#define MONTAUK_BUILD_NUMBER 24 +#define MONTAUK_BUILD_NUMBER 27 diff --git a/kernel/src/Ipc/Ipc.cpp b/kernel/src/Ipc/Ipc.cpp index 4e84bba..f88007c 100644 --- a/kernel/src/Ipc/Ipc.cpp +++ b/kernel/src/Ipc/Ipc.cpp @@ -44,6 +44,7 @@ namespace Ipc { struct Object { HandleType type; bool active; + bool destroying; uint32_t refs; kcp::Mutex lock; }; @@ -267,9 +268,38 @@ namespace Ipc { static void InitObject(Object& object, HandleType type) { object.type = type; object.active = true; + object.destroying = false; object.refs = 0; } + static kcp::Mutex* PoolLockForType(HandleType type) { + switch (type) { + case HandleType::Stream: return &g_streamPoolLock; + case HandleType::Mailbox: return &g_mailboxPoolLock; + case HandleType::File: return &g_filePoolLock; + case HandleType::Socket: return &g_socketPoolLock; + case HandleType::Surface: return &g_surfacePoolLock; + case HandleType::Process: return &g_processPoolLock; + case HandleType::Waitset: return &g_waitsetPoolLock; + default: return nullptr; + } + } + + // An object is hidden from users as soon as its last reference disappears, + // but its pool slot must not become reusable until the type-specific + // destructor has finished. Otherwise a concurrent allocator can install a + // new backend in the slot and the old destructor will tear down the new + // object (observed as open() succeeding immediately before getsize() saw + // an invalid file handle). + static void FinishDestroy(Object* object, HandleType type) { + kcp::Mutex* poolLock = PoolLockForType(type); + if (poolLock) poolLock->Acquire(); + object->lock.Acquire(); + object->destroying = false; + object->lock.Release(); + if (poolLock) poolLock->Release(); + } + static void RetainRawObject(Object* object) { if (object == nullptr) return; object->lock.Acquire(); @@ -376,6 +406,7 @@ namespace Ipc { destroy = true; type = object->type; object->active = false; + object->destroying = true; } object->lock.Release(); @@ -405,6 +436,7 @@ namespace Ipc { default: break; } + FinishDestroy(object, type); } static uint32_t CurrentSignalsForSnapshot(HandleType type, Object* object, uint32_t rights); @@ -540,11 +572,15 @@ namespace Ipc { if (stream->refs == 0) { destroy = true; stream->active = false; + stream->destroying = true; } stream->lock.Release(); if (notify) NotifyObjectChanged((Object*)stream); - if (destroy) DestroyStream(stream); + if (destroy) { + DestroyStream(stream); + FinishDestroy((Object*)stream, HandleType::Stream); + } break; } case HandleType::Mailbox: { @@ -562,11 +598,15 @@ namespace Ipc { if (mailbox->refs == 0) { destroy = true; mailbox->active = false; + mailbox->destroying = true; } mailbox->lock.Release(); if (notify) NotifyObjectChanged((Object*)mailbox); - if (destroy) DestroyMailbox(mailbox); + if (destroy) { + DestroyMailbox(mailbox); + FinishDestroy((Object*)mailbox, HandleType::Mailbox); + } break; } default: @@ -660,7 +700,7 @@ namespace Ipc { g_streamPoolLock.Acquire(); for (int i = 0; i < MaxStreams; i++) { - if (g_streams[i].active) continue; + if (g_streams[i].active || g_streams[i].destroying) continue; InitObject(g_streams[i], HandleType::Stream); g_streams[i].buffer = (uint8_t*)buffer; g_streams[i].capacity = (uint32_t)numPages * 0x1000u; @@ -841,7 +881,7 @@ namespace Ipc { Mailbox* CreateMailbox() { g_mailboxPoolLock.Acquire(); for (int i = 0; i < MaxMailboxes; i++) { - if (g_mailboxes[i].active) continue; + if (g_mailboxes[i].active || g_mailboxes[i].destroying) continue; InitObject(g_mailboxes[i], HandleType::Mailbox); g_mailboxes[i].head = 0; g_mailboxes[i].tail = 0; @@ -1106,7 +1146,7 @@ namespace Ipc { g_filePoolLock.Acquire(); for (int i = 0; i < MaxFiles; i++) { - if (g_files[i].active) continue; + if (g_files[i].active || g_files[i].destroying) continue; InitObject(g_files[i], HandleType::File); g_files[i].backend = backend; g_filePoolLock.Release(); @@ -1173,7 +1213,7 @@ namespace Ipc { static Socket* AllocateSocketObject(int type) { g_socketPoolLock.Acquire(); for (int i = 0; i < MaxSockets; i++) { - if (g_sockets[i].active) continue; + if (g_sockets[i].active || g_sockets[i].destroying) continue; InitObject(g_sockets[i], HandleType::Socket); g_sockets[i].socketType = type; g_sockets[i].tcpConn = nullptr; @@ -1505,7 +1545,7 @@ namespace Ipc { g_surfacePoolLock.Acquire(); int slot = -1; for (int i = 0; i < MaxSurfaces; i++) { - if (!g_surfaces[i].active) { + if (!g_surfaces[i].active && !g_surfaces[i].destroying) { slot = i; break; } @@ -1927,7 +1967,7 @@ namespace Ipc { g_processPoolLock.Acquire(); for (int i = 0; i < MaxProcessObjects; i++) { - if (g_processObjects[i].active) continue; + if (g_processObjects[i].active || g_processObjects[i].destroying) continue; InitObject(g_processObjects[i], HandleType::Process); g_processObjects[i].pid = pid; g_processObjects[i].exited = false; @@ -2127,7 +2167,7 @@ namespace Ipc { g_waitsetPoolLock.Acquire(); for (int i = 0; i < MaxWaitsets; i++) { - if (g_waitsets[i].active) continue; + if (g_waitsets[i].active || g_waitsets[i].destroying) continue; InitObject(g_waitsets[i], HandleType::Waitset); for (int j = 0; j < MaxWaitsetEntries; j++) { g_waitsets[i].entries[j].used = false; diff --git a/programs/include/gui/truetype.hpp b/programs/include/gui/truetype.hpp index 0f450d0..edf3593 100644 --- a/programs/include/gui/truetype.hpp +++ b/programs/include/gui/truetype.hpp @@ -121,8 +121,13 @@ struct TrueTypeFont { return false; } - montauk::read(fd, data, 0, size); + int read_result = montauk::read(fd, data, 0, size); montauk::close(fd); + if (read_result < 0 || (uint64_t)read_result != size) { + montauk::free(data); + data = nullptr; + return false; + } // stbtt_GetFontOffsetForIndex returns -1 for non-font data; passing // that into stbtt_InitFont makes it dereference data + (uint32)-1.