diff --git a/lib/Logging/Logging.cpp b/lib/Logging/Logging.cpp index d7f83606..d670f5fb 100644 --- a/lib/Logging/Logging.cpp +++ b/lib/Logging/Logging.cpp @@ -8,9 +8,23 @@ // 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 LOG_RTC_MAGIC = 0xDEADBEEF; void addToLogRingBuffer(const char* message) { - // Add the message to the ring buffer, overwriting old messages if necessary + // 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'; logHead = (logHead + 1) % MAX_LOG_LINES; @@ -63,19 +77,38 @@ 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; if (logMessages[idx][0] != '\0') { - output += logMessages[idx]; + const size_t len = strnlen(logMessages[idx], MAX_ENTRY_LEN); + output.append(logMessages[idx], len); } } 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; + return true; + } + return false; +} + void clearLastLogs() { for (size_t i = 0; i < MAX_LOG_LINES; i++) { logMessages[i][0] = '\0'; } logHead = 0; + rtcLogMagic = LOG_RTC_MAGIC; } diff --git a/lib/Logging/Logging.h b/lib/Logging/Logging.h index 83cabdc1..47e8eb7d 100644 --- a/lib/Logging/Logging.h +++ b/lib/Logging/Logging.h @@ -57,6 +57,11 @@ void logPrintf(const char* level, const char* origin, const char* format, ...); std::string getLastLogs(); void clearLastLogs(); +// 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 { public: diff --git a/lib/hal/HalSystem.cpp b/lib/hal/HalSystem.cpp index 606bf80d..b92c95a1 100644 --- a/lib/hal/HalSystem.cpp +++ b/lib/hal/HalSystem.cpp @@ -76,6 +76,14 @@ 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). + // 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(); + } } }