diff --git a/lib/GfxRenderer/BitmapHelpers.cpp b/lib/GfxRenderer/BitmapHelpers.cpp index 07e985f3..f564c9bb 100644 --- a/lib/GfxRenderer/BitmapHelpers.cpp +++ b/lib/GfxRenderer/BitmapHelpers.cpp @@ -107,7 +107,7 @@ uint8_t quantize1bit(int gray, int x, int y) { return (gray >= adjustedThreshold) ? 1 : 0; } -void createBmpHeader(BmpHeader* bmpHeader, int width, int height) { +void createBmpHeader(BmpHeader* bmpHeader, int width, int height, BmpRowOrder rowOrder) { if (!bmpHeader) return; // Zero out the memory to ensure no garbage data if called on uninitialized stack memory @@ -125,15 +125,15 @@ void createBmpHeader(BmpHeader* bmpHeader, int width, int height) { bmpHeader->infoHeader.biSize = sizeof(bmpHeader->infoHeader); bmpHeader->infoHeader.biWidth = width; - bmpHeader->infoHeader.biHeight = height; + bmpHeader->infoHeader.biHeight = (rowOrder == BmpRowOrder::TopDown) ? -height : height; bmpHeader->infoHeader.biPlanes = 1; bmpHeader->infoHeader.biBitCount = 1; bmpHeader->infoHeader.biCompression = 0; bmpHeader->infoHeader.biSizeImage = imageSize; - bmpHeader->infoHeader.biXPelsPerMeter = 0; - bmpHeader->infoHeader.biYPelsPerMeter = 0; - bmpHeader->infoHeader.biClrUsed = 0; - bmpHeader->infoHeader.biClrImportant = 0; + bmpHeader->infoHeader.biXPelsPerMeter = 2835; // 72 DPI + bmpHeader->infoHeader.biYPelsPerMeter = 2835; // 72 DPI + bmpHeader->infoHeader.biClrUsed = 2; + bmpHeader->infoHeader.biClrImportant = 2; // Color 0 (black) bmpHeader->colors[0].rgbBlue = 0; diff --git a/lib/GfxRenderer/BitmapHelpers.h b/lib/GfxRenderer/BitmapHelpers.h index 8220fc22..bd76cefd 100644 --- a/lib/GfxRenderer/BitmapHelpers.h +++ b/lib/GfxRenderer/BitmapHelpers.h @@ -11,8 +11,10 @@ uint8_t quantizeSimple(int gray); uint8_t quantize1bit(int gray, int x, int y); int adjustPixel(int gray); +enum class BmpRowOrder { BottomUp, TopDown }; + // Populates a 1-bit BMP header in the provided memory. -void createBmpHeader(BmpHeader* bmpHeader, int width, int height); +void createBmpHeader(BmpHeader* bmpHeader, int width, int height, BmpRowOrder rowOrder); // 1-bit Atkinson dithering - better quality than noise dithering for thumbnails // Error distribution pattern (same as 2-bit but quantizes to 2 levels): diff --git a/lib/Xtc/Xtc.cpp b/lib/Xtc/Xtc.cpp index b18a69ac..6dd3886b 100644 --- a/lib/Xtc/Xtc.cpp +++ b/lib/Xtc/Xtc.cpp @@ -7,6 +7,7 @@ #include "Xtc.h" +#include #include #include @@ -175,52 +176,12 @@ bool Xtc::generateCoverBmp() const { return false; } - // Write BMP header - // BMP file header (14 bytes) - const uint32_t rowSize = ((pageInfo.width + 31) / 32) * 4; // Row size aligned to 4 bytes - const uint32_t imageSize = rowSize * pageInfo.height; - const uint32_t fileSize = 14 + 40 + 8 + imageSize; // Header + DIB + palette + data + // Write 1-bit BMP header (top-down row order) + BmpHeader bmpHeader; + createBmpHeader(&bmpHeader, pageInfo.width, pageInfo.height, BmpRowOrder::TopDown); + coverBmp.write(reinterpret_cast(&bmpHeader), sizeof(bmpHeader)); - // File header - coverBmp.write('B'); - coverBmp.write('M'); - coverBmp.write(reinterpret_cast(&fileSize), 4); - uint32_t reserved = 0; - coverBmp.write(reinterpret_cast(&reserved), 4); - uint32_t dataOffset = 14 + 40 + 8; // 1-bit palette has 2 colors (8 bytes) - coverBmp.write(reinterpret_cast(&dataOffset), 4); - - // DIB header (BITMAPINFOHEADER - 40 bytes) - uint32_t dibHeaderSize = 40; - coverBmp.write(reinterpret_cast(&dibHeaderSize), 4); - int32_t width = pageInfo.width; - coverBmp.write(reinterpret_cast(&width), 4); - int32_t height = -static_cast(pageInfo.height); // Negative for top-down - coverBmp.write(reinterpret_cast(&height), 4); - uint16_t planes = 1; - coverBmp.write(reinterpret_cast(&planes), 2); - uint16_t bitsPerPixel = 1; // 1-bit monochrome - coverBmp.write(reinterpret_cast(&bitsPerPixel), 2); - uint32_t compression = 0; // BI_RGB (no compression) - coverBmp.write(reinterpret_cast(&compression), 4); - coverBmp.write(reinterpret_cast(&imageSize), 4); - int32_t ppmX = 2835; // 72 DPI - coverBmp.write(reinterpret_cast(&ppmX), 4); - int32_t ppmY = 2835; - coverBmp.write(reinterpret_cast(&ppmY), 4); - uint32_t colorsUsed = 2; - coverBmp.write(reinterpret_cast(&colorsUsed), 4); - uint32_t colorsImportant = 2; - coverBmp.write(reinterpret_cast(&colorsImportant), 4); - - // Color palette (2 colors for 1-bit) - // XTC 1-bit polarity: 0 = black, 1 = white (standard BMP palette order) - // Color 0: Black (text/foreground in XTC) - uint8_t black[4] = {0x00, 0x00, 0x00, 0x00}; - coverBmp.write(black, 4); - // Color 1: White (background in XTC) - uint8_t white[4] = {0xFF, 0xFF, 0xFF, 0x00}; - coverBmp.write(white, 4); + const uint32_t rowSize = ((pageInfo.width + 31) / 32) * 4; // Write bitmap data // BMP requires 4-byte row alignment @@ -403,49 +364,12 @@ bool Xtc::generateThumbBmp(int height) const { return false; } - // Write 1-bit BMP header for fast home screen rendering - const uint32_t rowSize = (thumbWidth + 31) / 32 * 4; // 1 bit per pixel, aligned to 4 bytes - const uint32_t imageSize = rowSize * thumbHeight; - const uint32_t fileSize = 14 + 40 + 8 + imageSize; // 8 bytes for 2-color palette + // Write 1-bit BMP header (top-down row order) + BmpHeader bmpHeader; + createBmpHeader(&bmpHeader, thumbWidth, thumbHeight, BmpRowOrder::TopDown); + thumbBmp.write(reinterpret_cast(&bmpHeader), sizeof(bmpHeader)); - // File header - thumbBmp.write('B'); - thumbBmp.write('M'); - thumbBmp.write(reinterpret_cast(&fileSize), 4); - uint32_t reserved = 0; - thumbBmp.write(reinterpret_cast(&reserved), 4); - uint32_t dataOffset = 14 + 40 + 8; // 1-bit palette has 2 colors (8 bytes) - thumbBmp.write(reinterpret_cast(&dataOffset), 4); - - // DIB header - uint32_t dibHeaderSize = 40; - thumbBmp.write(reinterpret_cast(&dibHeaderSize), 4); - int32_t widthVal = thumbWidth; - thumbBmp.write(reinterpret_cast(&widthVal), 4); - int32_t heightVal = -static_cast(thumbHeight); // Negative for top-down - thumbBmp.write(reinterpret_cast(&heightVal), 4); - uint16_t planes = 1; - thumbBmp.write(reinterpret_cast(&planes), 2); - uint16_t bitsPerPixel = 1; // 1-bit for black and white - thumbBmp.write(reinterpret_cast(&bitsPerPixel), 2); - uint32_t compression = 0; - thumbBmp.write(reinterpret_cast(&compression), 4); - thumbBmp.write(reinterpret_cast(&imageSize), 4); - int32_t ppmX = 2835; - thumbBmp.write(reinterpret_cast(&ppmX), 4); - int32_t ppmY = 2835; - thumbBmp.write(reinterpret_cast(&ppmY), 4); - uint32_t colorsUsed = 2; - thumbBmp.write(reinterpret_cast(&colorsUsed), 4); - uint32_t colorsImportant = 2; - thumbBmp.write(reinterpret_cast(&colorsImportant), 4); - - // Color palette (2 colors for 1-bit: black and white) - uint8_t palette[8] = { - 0x00, 0x00, 0x00, 0x00, // Color 0: Black - 0xFF, 0xFF, 0xFF, 0x00 // Color 1: White - }; - thumbBmp.write(palette, 8); + const uint32_t rowSize = (thumbWidth + 31) / 32 * 4; // Allocate row buffer for 1-bit output uint8_t* rowBuffer = static_cast(malloc(rowSize)); @@ -628,4 +552,4 @@ void Xtc::prefetchPages(uint32_t pageIndex) const { return; } parser->prefetchWindow(pageIndex); -} +} \ No newline at end of file diff --git a/lib/ZipFile/ZipFile.cpp b/lib/ZipFile/ZipFile.cpp index 58fa64d5..2bb19147 100644 --- a/lib/ZipFile/ZipFile.cpp +++ b/lib/ZipFile/ZipFile.cpp @@ -18,6 +18,28 @@ namespace { constexpr uint16_t ZIP_METHOD_STORED = 0; constexpr uint16_t ZIP_METHOD_DEFLATED = 8; +// RAII zip: opens the zip if not already open, closes on destruction only if +// it performed the open. Removes the wasOpen/close boilerplate from every method. +class ScopedOpenClose final { + public: + [[nodiscard]] explicit ScopedOpenClose(ZipFile& zf) : zf(zf), needsClose(!zf.isOpen()) { + if (needsClose) ok = zf.open(); + } + ~ScopedOpenClose() { + if (needsClose && ok) zf.close(); + } + ScopedOpenClose(const ScopedOpenClose&) = delete; + ScopedOpenClose& operator=(const ScopedOpenClose&) = delete; + ScopedOpenClose(ScopedOpenClose&&) = delete; + ScopedOpenClose& operator=(ScopedOpenClose&&) = delete; + explicit operator bool() const { return ok || !needsClose; } + + private: + ZipFile& zf; + bool needsClose = false; + bool ok = true; // true when zip was already open (no open() call needed) +}; + int zipReadCallback(uzlib_uncomp* uncomp) { auto* ctx = reinterpret_cast(uncomp); if (ctx->fileRemaining == 0) return -1; @@ -35,17 +57,10 @@ int zipReadCallback(uzlib_uncomp* uncomp) { } // namespace bool ZipFile::loadAllFileStatSlims() { - const bool wasOpen = isOpen(); - if (!wasOpen && !open()) { - return false; - } + const ScopedOpenClose zip{*this}; + if (!zip) return false; - if (!loadZipDetails()) { - if (!wasOpen) { - close(); - } - return false; - } + if (!loadZipDetails()) return false; file.seek(zipDetails.centralDirOffset); @@ -89,9 +104,6 @@ bool ZipFile::loadAllFileStatSlims() { lastCentralDirPos = zipDetails.centralDirOffset; lastCentralDirPosValid = true; - if (!wasOpen) { - close(); - } return true; } @@ -105,17 +117,10 @@ bool ZipFile::loadFileStatSlim(const char* filename, FileStatSlim* fileStat) { return false; } - const bool wasOpen = isOpen(); - if (!wasOpen && !open()) { - return false; - } + const ScopedOpenClose zip{*this}; + if (!zip) return false; - if (!loadZipDetails()) { - if (!wasOpen) { - close(); - } - return false; - } + if (!loadZipDetails()) return false; // Phase 1: Try scanning from cursor position first uint32_t startPos = lastCentralDirPosValid ? lastCentralDirPos : zipDetails.centralDirOffset; @@ -179,17 +184,12 @@ bool ZipFile::loadFileStatSlim(const char* filename, FileStatSlim* fileStat) { file.seekCur(m + k); } - if (!wasOpen) { - close(); - } return found; } long ZipFile::getDataOffset(const FileStatSlim& fileStat) { - const bool wasOpen = isOpen(); - if (!wasOpen && !open()) { - return -1; - } + const ScopedOpenClose zip{*this}; + if (!zip) return -1; constexpr auto localHeaderSize = 30; @@ -198,9 +198,6 @@ long ZipFile::getDataOffset(const FileStatSlim& fileStat) { file.seek(fileOffset); const size_t read = file.read(pLocalHeader, localHeaderSize); - if (!wasOpen) { - close(); - } if (read != localHeaderSize) { LOG_ERR("ZIP", "Something went wrong reading the local header"); @@ -223,17 +220,12 @@ bool ZipFile::loadZipDetails() { return true; } - const bool wasOpen = isOpen(); - if (!wasOpen && !open()) { - return false; - } + const ScopedOpenClose zip{*this}; + if (!zip) return false; const size_t fileSize = file.size(); if (fileSize < 22) { LOG_ERR("ZIP", "File too small to be a valid zip"); - if (!wasOpen) { - close(); - } return false; // Minimum EOCD size is 22 bytes } @@ -243,9 +235,6 @@ bool ZipFile::loadZipDetails() { const auto buffer = static_cast(malloc(scanRange)); if (!buffer) { LOG_ERR("ZIP", "Failed to allocate memory for EOCD scan buffer"); - if (!wasOpen) { - close(); - } return false; } @@ -265,9 +254,6 @@ bool ZipFile::loadZipDetails() { if (foundOffset == -1) { LOG_ERR("ZIP", "EOCD signature not found in zip file"); free(buffer); - if (!wasOpen) { - close(); - } return false; } @@ -280,9 +266,6 @@ bool ZipFile::loadZipDetails() { zipDetails.isSet = true; free(buffer); - if (!wasOpen) { - close(); - } return true; } @@ -317,17 +300,10 @@ int ZipFile::fillUncompressedSizes(std::vector& targets, std::vector return 0; } - const bool wasOpen = isOpen(); - if (!wasOpen && !open()) { - return 0; - } + const ScopedOpenClose zip{*this}; + if (!zip) return 0; - if (!loadZipDetails()) { - if (!wasOpen) { - close(); - } - return 0; - } + if (!loadZipDetails()) return 0; file.seek(zipDetails.centralDirOffset); @@ -384,34 +360,18 @@ int ZipFile::fillUncompressedSizes(std::vector& targets, std::vector file.seekCur(m + k); } - if (!wasOpen) { - close(); - } - return matched; } uint8_t* ZipFile::readFileToMemory(const char* filename, size_t* size, const bool trailingNullByte) { - const bool wasOpen = isOpen(); - if (!wasOpen && !open()) { - return nullptr; - } + const ScopedOpenClose zip{*this}; + if (!zip) return nullptr; FileStatSlim fileStat = {}; - if (!loadFileStatSlim(filename, &fileStat)) { - if (!wasOpen) { - close(); - } - return nullptr; - } + if (!loadFileStatSlim(filename, &fileStat)) return nullptr; const long fileOffset = getDataOffset(fileStat); - if (fileOffset < 0) { - if (!wasOpen) { - close(); - } - return nullptr; - } + if (fileOffset < 0) return nullptr; file.seek(fileOffset); @@ -421,18 +381,12 @@ uint8_t* ZipFile::readFileToMemory(const char* filename, size_t* size, const boo const auto data = static_cast(malloc(dataSize)); if (data == nullptr) { LOG_ERR("ZIP", "Failed to allocate memory for output buffer (%zu bytes)", dataSize); - if (!wasOpen) { - close(); - } return nullptr; } if (fileStat.method == ZIP_METHOD_STORED) { // no deflation, just read content const size_t dataRead = file.read(data, inflatedDataSize); - if (!wasOpen) { - close(); - } if (dataRead != inflatedDataSize) { LOG_ERR("ZIP", "Failed to read data"); @@ -446,16 +400,11 @@ uint8_t* ZipFile::readFileToMemory(const char* filename, size_t* size, const boo const auto deflatedData = static_cast(malloc(deflatedDataSize)); if (deflatedData == nullptr) { LOG_ERR("ZIP", "Failed to allocate memory for decompression buffer"); - if (!wasOpen) { - close(); - } + free(data); return nullptr; } const size_t dataRead = file.read(deflatedData, deflatedDataSize); - if (!wasOpen) { - close(); - } if (dataRead != deflatedDataSize) { LOG_ERR("ZIP", "Failed to read data, expected %d got %d", deflatedDataSize, dataRead); @@ -482,9 +431,7 @@ uint8_t* ZipFile::readFileToMemory(const char* filename, size_t* size, const boo // Continue out of block with data set } else { LOG_ERR("ZIP", "Unsupported compression method"); - if (!wasOpen) { - close(); - } + free(data); return nullptr; } @@ -494,20 +441,14 @@ uint8_t* ZipFile::readFileToMemory(const char* filename, size_t* size, const boo } bool ZipFile::readFileToStream(const char* filename, Print& out, const size_t chunkSize) { - const bool wasOpen = isOpen(); - if (!wasOpen && !open()) { - return false; - } + const ScopedOpenClose zip{*this}; + if (!zip) return false; FileStatSlim fileStat = {}; - if (!loadFileStatSlim(filename, &fileStat)) { - return false; - } + if (!loadFileStatSlim(filename, &fileStat)) return false; const long fileOffset = getDataOffset(fileStat); - if (fileOffset < 0) { - return false; - } + if (fileOffset < 0) return false; file.seek(fileOffset); const auto deflatedDataSize = fileStat.compressedSize; @@ -518,9 +459,6 @@ bool ZipFile::readFileToStream(const char* filename, Print& out, const size_t ch const auto buffer = static_cast(malloc(chunkSize)); if (!buffer) { LOG_ERR("ZIP", "Failed to allocate memory for buffer"); - if (!wasOpen) { - close(); - } return false; } @@ -530,19 +468,17 @@ bool ZipFile::readFileToStream(const char* filename, Print& out, const size_t ch if (dataRead == 0) { LOG_ERR("ZIP", "Could not read more bytes"); free(buffer); - if (!wasOpen) { - close(); - } return false; } - out.write(buffer, dataRead); + if (out.write(buffer, dataRead) != dataRead) { + LOG_ERR("ZIP", "Failed to write all output bytes to stream"); + free(buffer); + return false; + } remaining -= dataRead; } - if (!wasOpen) { - close(); - } free(buffer); return true; } @@ -551,9 +487,6 @@ bool ZipFile::readFileToStream(const char* filename, Print& out, const size_t ch auto* fileReadBuffer = static_cast(malloc(chunkSize)); if (!fileReadBuffer) { LOG_ERR("ZIP", "Failed to allocate memory for zip file read buffer"); - if (!wasOpen) { - close(); - } return false; } @@ -561,9 +494,6 @@ bool ZipFile::readFileToStream(const char* filename, Print& out, const size_t ch if (!outputBuffer) { LOG_ERR("ZIP", "Failed to allocate memory for output buffer"); free(fileReadBuffer); - if (!wasOpen) { - close(); - } return false; } @@ -577,9 +507,6 @@ bool ZipFile::readFileToStream(const char* filename, Print& out, const size_t ch LOG_ERR("ZIP", "Failed to init inflate reader"); free(outputBuffer); free(fileReadBuffer); - if (!wasOpen) { - close(); - } return false; } ctx.reader.setReadCallback(zipReadCallback); @@ -623,18 +550,11 @@ bool ZipFile::readFileToStream(const char* filename, Print& out, const size_t ch // InflateStatus::Ok: output buffer full, continue } - if (!wasOpen) { - close(); - } free(outputBuffer); free(fileReadBuffer); return success; // ctx.reader destructor frees the ring buffer } - if (!wasOpen) { - close(); - } - LOG_ERR("ZIP", "Unsupported compression method"); return false; } diff --git a/platformio.ini b/platformio.ini index 28715914..b66b8725 100644 --- a/platformio.ini +++ b/platformio.ini @@ -53,6 +53,7 @@ extra_scripts = pre:scripts/build_html.py pre:scripts/gen_i18n.py pre:scripts/git_branch.py + pre:scripts/patch_jpegdec.py ; Libraries lib_deps = diff --git a/scripts/patch_jpegdec.py b/scripts/patch_jpegdec.py new file mode 100644 index 00000000..015b1761 --- /dev/null +++ b/scripts/patch_jpegdec.py @@ -0,0 +1,68 @@ +""" +PlatformIO pre-build script: patch JPEGDEC for MCU_SKIP wild pointer crash. + +Problem: + JPEGDecodeMCU_P computes pMCU = &sMCUs[iMCU & 0xffffff]. When iMCU is + MCU_SKIP (-8), the bitmask produces index 0xFFFFF8 (16 777 208), creating a + pointer ~33 MB past the 392-entry sMCUs array. If the progressive JPEG's + first scan includes AC coefficients (iScanEnd > 0), the AC decode loop writes + through this wild pointer and crashes with a store-access fault. + + Upstream commit 8628297 guarded the DC coefficient write (pMCU[0]) but not the + AC coefficient writes at indices 1-63. + +Fix: + Redirect pMCU to sMCUs[0] when MCU_SKIP is active. Writes to sMCUs[1..63] + are harmless: for JPEG_SCALE_EIGHTH only sMCUs[0] is read for output, and + the DC write at sMCUs[0] is already guarded by the existing `if (iMCU >= 0)` + check. + +Applied idempotently — safe to run on every build. +""" + +Import("env") +import os + + +def patch_jpegdec(env): + libdeps_dir = os.path.join(env["PROJECT_DIR"], ".pio", "libdeps") + if not os.path.isdir(libdeps_dir): + return + for env_dir in os.listdir(libdeps_dir): + jpeg_inl = os.path.join(libdeps_dir, env_dir, "JPEGDEC", "src", "jpeg.inl") + if os.path.isfile(jpeg_inl): + _apply_mcu_skip_pointer_fix(jpeg_inl) + + +def _apply_mcu_skip_pointer_fix(filepath): + MARKER = "// CrossPoint patch: safe pMCU for MCU_SKIP" + with open(filepath, "r") as f: + content = f.read() + + if MARKER in content: + return # already patched + + # The wild-pointer line in JPEGDecodeMCU_P: + OLD = " signed short *pMCU = &pJPEG->sMCUs[iMCU & 0xffffff];" + + NEW = ( + " " + MARKER + "\n" + " signed short *pMCU = (iMCU < 0) ? pJPEG->sMCUs\n" + " : &pJPEG->sMCUs[iMCU & 0xffffff];" + ) + + if OLD not in content: + print( + "WARNING: JPEGDEC MCU_SKIP pointer patch target not found in %s " + "— library may have been updated" % filepath + ) + return + + content = content.replace(OLD, NEW, 1) + with open(filepath, "w") as f: + f.write(content) + print("Patched JPEGDEC: safe pMCU for MCU_SKIP in JPEGDecodeMCU_P: %s" % filepath) + + +# Run immediately at script import time (before compilation). +patch_jpegdec(env) diff --git a/src/util/ScreenshotUtil.cpp b/src/util/ScreenshotUtil.cpp index 67d16f00..bee2b8c6 100644 --- a/src/util/ScreenshotUtil.cpp +++ b/src/util/ScreenshotUtil.cpp @@ -62,7 +62,7 @@ bool ScreenshotUtil::saveFramebufferAsBmp(const char* filename, const uint8_t* f BmpHeader header; - createBmpHeader(&header, phyWidth, phyHeight); + createBmpHeader(&header, phyWidth, phyHeight, BmpRowOrder::BottomUp); bool write_error = false; if (file.write(reinterpret_cast(&header), sizeof(header)) != sizeof(header)) {