From de871ac4025776553ab21a1c7598005ba5a3eb91 Mon Sep 17 00:00:00 2001 From: Daniel Hammer Date: Fri, 19 Jun 2026 19:36:21 +0200 Subject: [PATCH] fix: lock crash report ring buffer (SMP race) and guard crashpad crash loop --- kernel/src/Api/BuildNo.hpp | 2 +- kernel/src/Api/CrashReportSyscall.hpp | 52 ++++++++++++++------------- kernel/src/Sched/CrashReport.cpp | 22 ++++++++++++ kernel/src/Sched/CrashReport.hpp | 12 +++++-- kernel/src/Sched/Scheduler.cpp | 12 +++++++ 5 files changed, 72 insertions(+), 28 deletions(-) diff --git a/kernel/src/Api/BuildNo.hpp b/kernel/src/Api/BuildNo.hpp index 28e56c6..65135bd 100644 --- a/kernel/src/Api/BuildNo.hpp +++ b/kernel/src/Api/BuildNo.hpp @@ -12,4 +12,4 @@ #pragma once -#define MONTAUK_BUILD_NUMBER 128 +#define MONTAUK_BUILD_NUMBER 129 diff --git a/kernel/src/Api/CrashReportSyscall.hpp b/kernel/src/Api/CrashReportSyscall.hpp index 4a6263b..2841acb 100644 --- a/kernel/src/Api/CrashReportSyscall.hpp +++ b/kernel/src/Api/CrashReportSyscall.hpp @@ -13,38 +13,40 @@ namespace montauk::abi { static int64_t Sys_CrashReport(CrashReportInfo* out) { if (!out) return 0; - if (CrashReport::g_reportCount == 0) return 0; - int oldestIdx = (CrashReport::g_reportHead - CrashReport::g_reportCount + CrashReport::MaxReports) % CrashReport::MaxReports; - auto* rep = &CrashReport::g_reports[oldestIdx]; + // Pop the oldest report into a kernel-local copy under the ring lock, + // then marshal into user memory with the lock released. + CrashReport::Report rep; + int had = CrashReport::TakeOldest(rep); + if (had == 0) return 0; - out->pid = rep->pid; + out->pid = rep.pid; int pn; - for (pn = 0; pn < 63 && rep->processName[pn]; pn++) out->processName[pn] = rep->processName[pn]; + for (pn = 0; pn < 63 && rep.processName[pn]; pn++) out->processName[pn] = rep.processName[pn]; out->processName[pn] = '\0'; - out->exceptionVector = rep->exceptionVector; + out->exceptionVector = rep.exceptionVector; int en; - for (en = 0; en < 31 && rep->exceptionName[en]; en++) out->exceptionName[en] = rep->exceptionName[en]; + for (en = 0; en < 31 && rep.exceptionName[en]; en++) out->exceptionName[en] = rep.exceptionName[en]; out->exceptionName[en] = '\0'; - out->faultingAddress = rep->faultingAddress; - out->instructionPointer = rep->instructionPointer; - out->stackPointer = rep->stackPointer; - out->codeSegment = rep->codeSegment; - out->flags = rep->flags; - out->stackSegment = rep->stackSegment; - out->pfPresent = rep->pfPresent; - out->pfWrite = rep->pfWrite; - out->pfUser = rep->pfUser; - out->pfReservedWrite = rep->pfReservedWrite; - out->pfInstructionFetch = rep->pfInstructionFetch; - out->pfProtectionKey = rep->pfProtectionKey; - out->pfShadowStack = rep->pfShadowStack; - out->pfSGX = rep->pfSGX; - out->timestampTick = rep->timestampTick; - rep->valid = false; - CrashReport::g_reportCount--; + out->faultingAddress = rep.faultingAddress; + out->instructionPointer = rep.instructionPointer; + out->stackPointer = rep.stackPointer; + out->codeSegment = rep.codeSegment; + out->flags = rep.flags; + out->stackSegment = rep.stackSegment; + out->pfPresent = rep.pfPresent; + out->pfWrite = rep.pfWrite; + out->pfUser = rep.pfUser; + out->pfReservedWrite = rep.pfReservedWrite; + out->pfInstructionFetch = rep.pfInstructionFetch; + out->pfProtectionKey = rep.pfProtectionKey; + out->pfShadowStack = rep.pfShadowStack; + out->pfSGX = rep.pfSGX; + out->timestampTick = rep.timestampTick; - return CrashReport::g_reportCount + 1; + // Preserve the original contract: count of reports present before this + // call (i.e. including the one just returned). + return had; } } diff --git a/kernel/src/Sched/CrashReport.cpp b/kernel/src/Sched/CrashReport.cpp index 6613e92..95af6b2 100644 --- a/kernel/src/Sched/CrashReport.cpp +++ b/kernel/src/Sched/CrashReport.cpp @@ -5,6 +5,7 @@ */ #include "CrashReport.hpp" +#include namespace CrashReport { @@ -12,10 +13,31 @@ Report g_reports[MaxReports]; int g_reportHead = 0; int g_reportCount = 0; +// Serializes ring-buffer access. Interrupt-disabling: the writer runs inside +// the exception handler (IF=0) while readers come from syscall context. +static kcp::Spinlock g_lock; + void AddReport(const Report& report) { + g_lock.Acquire(); g_reports[g_reportHead] = report; g_reportHead = (g_reportHead + 1) % MaxReports; if (g_reportCount < MaxReports) g_reportCount++; + g_lock.Release(); +} + +int TakeOldest(Report& out) { + g_lock.Acquire(); + int had = g_reportCount; + if (had == 0) { + g_lock.Release(); + return 0; + } + int oldestIdx = (g_reportHead - g_reportCount + MaxReports) % MaxReports; + out = g_reports[oldestIdx]; + g_reports[oldestIdx].valid = false; + g_reportCount--; + g_lock.Release(); + return had; } } // namespace CrashReport diff --git a/kernel/src/Sched/CrashReport.hpp b/kernel/src/Sched/CrashReport.hpp index 20afb01..2667215 100644 --- a/kernel/src/Sched/CrashReport.hpp +++ b/kernel/src/Sched/CrashReport.hpp @@ -39,12 +39,20 @@ struct Report { bool valid; }; -// Kernel-side ring buffer (written by exception handlers, read by SYS_CRASH_REPORT) +// Kernel-side ring buffer (written by exception handlers, read by SYS_CRASH_REPORT). +// All access is serialized by an internal interrupt-disabling spinlock: the +// writer (AddReport) runs in exception/interrupt context and the reader +// (TakeOldest) runs in syscall context, possibly on different CPUs. extern Report g_reports[MaxReports]; extern int g_reportHead; extern int g_reportCount; -// Add a report to the kernel ring buffer +// Add a report to the kernel ring buffer (called from exception handlers). void AddReport(const Report& report); +// Pop the oldest report into `out`. Returns the number of reports that were +// present before this call (0 if the buffer was empty, in which case `out` is +// left untouched). Safe against concurrent AddReport / TakeOldest. +int TakeOldest(Report& out); + } // namespace CrashReport diff --git a/kernel/src/Sched/Scheduler.cpp b/kernel/src/Sched/Scheduler.cpp index 1e9b349..df93d6f 100644 --- a/kernel/src/Sched/Scheduler.cpp +++ b/kernel/src/Sched/Scheduler.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -1399,6 +1400,17 @@ namespace Sched { // handle to that same out-stream so it can surface a textual crash // report to the console user when no desktop is running. Process* crasher = GetProcessByPid(crasherPid); + + // Crash-loop guard: never spawn a crashpad to report on the crashpad + // itself. If crashpad.elf faults (startup bug, OOM, etc.) we would + // otherwise spawn another crashpad, which can fault again, ad infinitum + // -- a fork bomb that also exhausts process slots and report buffers. + if (crasher && Lib::strcmp(crasher->name, crashpadPath) == 0) { + Kt::KernelLogStream(Kt::ERROR, "Sched") + << "crashpad faulted; not spawning a nested crashpad"; + return -1; + } + int crasherSlot = Ipc::SlotForPid(crasherPid); bool inheritRedir = crasher && crasher->redirected && crasher->ioOutHandle >= 0 && crasherSlot >= 0;