From c464bdede8447479911c28ec426f2faf00601657 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sun, 8 Mar 2026 15:51:05 +0100 Subject: [PATCH 1/4] Add and expose sanitizing logic --- lib/Logging/Logging.cpp | 6 ++++++ lib/Logging/Logging.h | 3 +++ lib/hal/HalSystem.cpp | 4 ++++ 3 files changed, 13 insertions(+) diff --git a/lib/Logging/Logging.cpp b/lib/Logging/Logging.cpp index 0de1adf6..a050ffe4 100644 --- a/lib/Logging/Logging.cpp +++ b/lib/Logging/Logging.cpp @@ -80,6 +80,12 @@ std::string getLastLogs() { return output; } +void sanitizeLogHead() { + if (logHead >= MAX_LOG_LINES) { + logHead = 0; + } +} + void clearLastLogs() { for (size_t i = 0; i < MAX_LOG_LINES; i++) { logMessages[i][0] = '\0'; diff --git a/lib/Logging/Logging.h b/lib/Logging/Logging.h index 83cabdc1..fc2d9d37 100644 --- a/lib/Logging/Logging.h +++ b/lib/Logging/Logging.h @@ -57,6 +57,9 @@ void logPrintf(const char* level, const char* origin, const char* format, ...); std::string getLastLogs(); void clearLastLogs(); +// Clamps logHead into range without wiping messages — safe to call on panic +// reboots where the panic occurred before begin() ever had a chance to run. +void sanitizeLogHead(); class MySerialImpl : public Print { public: diff --git a/lib/hal/HalSystem.cpp b/lib/hal/HalSystem.cpp index 606bf80d..dfe8055d 100644 --- a/lib/hal/HalSystem.cpp +++ b/lib/hal/HalSystem.cpp @@ -76,6 +76,10 @@ void begin() { // `clearPanic()` to clear it after dumping if (!isRebootFromPanic()) { clearPanic(); + } else { + // Panic reboot: preserve logs and panic info, but clamp logHead in case the + // panic occurred before begin() ever ran (e.g. in a static constructor). + sanitizeLogHead(); } } From a3a50c8c40d69c5a40012ba199013376dc154f32 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sun, 8 Mar 2026 16:05:08 +0100 Subject: [PATCH 2/4] Clear log if invalid --- lib/Logging/Logging.cpp | 4 +++- lib/Logging/Logging.h | 8 +++++--- lib/hal/HalSystem.cpp | 6 +++++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/Logging/Logging.cpp b/lib/Logging/Logging.cpp index a050ffe4..69a75cb6 100644 --- a/lib/Logging/Logging.cpp +++ b/lib/Logging/Logging.cpp @@ -80,10 +80,12 @@ std::string getLastLogs() { return output; } -void sanitizeLogHead() { +bool sanitizeLogHead() { if (logHead >= MAX_LOG_LINES) { logHead = 0; + return true; } + return false; } void clearLastLogs() { diff --git a/lib/Logging/Logging.h b/lib/Logging/Logging.h index fc2d9d37..2638c86e 100644 --- a/lib/Logging/Logging.h +++ b/lib/Logging/Logging.h @@ -57,9 +57,11 @@ void logPrintf(const char* level, const char* origin, const char* format, ...); std::string getLastLogs(); void clearLastLogs(); -// Clamps logHead into range without wiping messages — safe to call on panic -// reboots where the panic occurred before begin() ever had a chance to run. -void sanitizeLogHead(); +// Clamps logHead into range. Returns true if logHead was out of range (repaired), +// which also means logMessages is untrusted garbage. Callers should call +// clearLastLogs() when this returns true so getLastLogs() does not dump corrupt +// data into crash reports. +bool sanitizeLogHead(); class MySerialImpl : public Print { public: diff --git a/lib/hal/HalSystem.cpp b/lib/hal/HalSystem.cpp index dfe8055d..b92c95a1 100644 --- a/lib/hal/HalSystem.cpp +++ b/lib/hal/HalSystem.cpp @@ -79,7 +79,11 @@ void begin() { } else { // Panic reboot: preserve logs and panic info, but clamp logHead in case the // panic occurred before begin() ever ran (e.g. in a static constructor). - sanitizeLogHead(); + // If logHead was out of range, logMessages is also garbage — clear it so + // getLastLogs() does not dump corrupt data into the crash report. + if (sanitizeLogHead()) { + clearLastLogs(); + } } } From ed1a21fb7ba5dafc7b2bf6aa3a70c9378beaa222 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sun, 8 Mar 2026 16:26:52 +0100 Subject: [PATCH 3/4] Add magic value --- lib/Logging/Logging.cpp | 22 +++++++++++++++++----- lib/Logging/Logging.h | 8 ++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/Logging/Logging.cpp b/lib/Logging/Logging.cpp index 69a75cb6..3a516e0a 100644 --- a/lib/Logging/Logging.cpp +++ b/lib/Logging/Logging.cpp @@ -8,14 +8,25 @@ // Simple ring buffer log, useful for error reporting when we encounter a crash RTC_NOINIT_ATTR char logMessages[MAX_LOG_LINES][MAX_ENTRY_LEN]; RTC_NOINIT_ATTR size_t logHead = 0; +// Magic word written alongside logHead to detect uninitialized RTC memory. +// RTC_NOINIT_ATTR is not zeroed on cold boot, so logHead may appear in-range +// (0..MAX_LOG_LINES-1) by chance even though logMessages is garbage. The magic +// value is only set by clearLastLogs(), so its absence means the buffer was +// never properly initialized. +RTC_NOINIT_ATTR uint32_t rtcLogMagic; +static constexpr uint32_t fnv1a32(const char* s, uint32_t h = 2166136261u) { + return *s ? fnv1a32(s + 1, (h ^ static_cast(*s)) * 16777619u) : h; +} +static constexpr uint32_t LOG_RTC_MAGIC = fnv1a32("crosspoint-reader"); void addToLogRingBuffer(const char* message) { - // Add the message to the ring buffer, overwriting old messages if necessary - // If RTC_NOINIT_ATTR left logHead out of range on cold boot, all slots are - // garbage too — clear the entire buffer so subsequent reads are safe. - if (logHead >= MAX_LOG_LINES) { + // Add the message to the ring buffer, overwriting old messages if necessary. + // If the magic is wrong or logHead is out of range (RTC_NOINIT_ATTR garbage + // on cold boot), clear the entire buffer so subsequent reads are safe. + if (rtcLogMagic != LOG_RTC_MAGIC || logHead >= MAX_LOG_LINES) { memset(logMessages, 0, sizeof(logMessages)); logHead = 0; + rtcLogMagic = LOG_RTC_MAGIC; } strncpy(logMessages[logHead], message, MAX_ENTRY_LEN - 1); logMessages[logHead][MAX_ENTRY_LEN - 1] = '\0'; @@ -81,7 +92,7 @@ std::string getLastLogs() { } bool sanitizeLogHead() { - if (logHead >= MAX_LOG_LINES) { + if (rtcLogMagic != LOG_RTC_MAGIC || logHead >= MAX_LOG_LINES) { logHead = 0; return true; } @@ -93,4 +104,5 @@ void clearLastLogs() { logMessages[i][0] = '\0'; } logHead = 0; + rtcLogMagic = LOG_RTC_MAGIC; } diff --git a/lib/Logging/Logging.h b/lib/Logging/Logging.h index 2638c86e..47e8eb7d 100644 --- a/lib/Logging/Logging.h +++ b/lib/Logging/Logging.h @@ -57,10 +57,10 @@ void logPrintf(const char* level, const char* origin, const char* format, ...); std::string getLastLogs(); void clearLastLogs(); -// Clamps logHead into range. Returns true if logHead was out of range (repaired), -// which also means logMessages is untrusted garbage. Callers should call -// clearLastLogs() when this returns true so getLastLogs() does not dump corrupt -// data into crash reports. +// Validates the RTC log state (magic word + logHead range). Returns true if +// corruption was detected (magic mismatch or logHead out of range), meaning +// logMessages is untrusted garbage. Callers should call clearLastLogs() when +// this returns true so getLastLogs() does not dump corrupt data into crash reports. bool sanitizeLogHead(); class MySerialImpl : public Print { From 996ae89d62a065c62a4a5fdc8e9d16ee0b91ca9b Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sun, 8 Mar 2026 16:33:01 +0100 Subject: [PATCH 4/4] And nitpick, too --- lib/Logging/Logging.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/Logging/Logging.cpp b/lib/Logging/Logging.cpp index 3a516e0a..5b976bc6 100644 --- a/lib/Logging/Logging.cpp +++ b/lib/Logging/Logging.cpp @@ -80,6 +80,9 @@ void logPrintf(const char* level, const char* origin, const char* format, ...) { } std::string getLastLogs() { + if (rtcLogMagic != LOG_RTC_MAGIC) { + return {}; + } std::string output; for (size_t i = 0; i < MAX_LOG_LINES; i++) { size_t idx = (logHead + i) % MAX_LOG_LINES; @@ -91,6 +94,12 @@ std::string getLastLogs() { return output; } +// Checks whether the RTC log state is consistent: rtcLogMagic must equal +// LOG_RTC_MAGIC and logHead must be in 0..MAX_LOG_LINES-1. Returns true if +// corruption is detected, in which case rtcLogMagic is still invalid and +// logMessages may contain garbage. Callers (e.g. HalSystem::begin on the +// panic-reboot path) must call clearLastLogs() after a true result to fully +// reinitialize the ring buffer and stamp the magic before getLastLogs() is used. bool sanitizeLogHead() { if (rtcLogMagic != LOG_RTC_MAGIC || logHead >= MAX_LOG_LINES) { logHead = 0;