fix: guard DC writes in JPEGDEC MCU_SKIP path (#2058)
EIGHT_BIT_GRAYSCALE decode of a 3-component progressive JPEG calls JPEGDecodeMCU_P with MCU_SKIP for Cb and Cr after every Y MCU. The existing safe-pMCU patch redirects the wild pointer to &sMCUs[0] but leaves the DC store unguarded, so each chroma skip overwrites the just-decoded Y DC with the chroma DC predictor. Output reads sMCUs[0], gets the trailing Cr DC (~0), and renders an all-black image. Add `if (iMCU >= 0)` guards to the two pMCU[0] writes (main DC store and successive-approximation update). The pointer redirect stays as the AC wild-pointer defense; the new guards stop the silent corruption at sMCUs[0]. The two fixes are independent and both required. fixes the progressive 8bit grayscale jpeg regression in 1.3.0 Did you use AI tools to help write this code? partial
This commit is contained in:
Jeremy Klein
@@ -0,0 +1,28 @@
|
||||
From 5dff5afab0c68d0d0c4385d72e6f0c030b613960 Mon Sep 17 00:00:00 2001
|
||||
From: patch <patch@local>
|
||||
Date: Mon, 18 May 2026 20:57:54 -0700
|
||||
Subject: [PATCH 1/2] Redirect pMCU to sMCUs[0] when iMCU < 0 (MCU_SKIP)
|
||||
|
||||
---
|
||||
src/jpeg.inl | 5 ++++-
|
||||
1 file changed, 4 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/jpeg.inl b/src/jpeg.inl
|
||||
index a60b548..26bcf6f 100644
|
||||
--- a/src/jpeg.inl
|
||||
+++ b/src/jpeg.inl
|
||||
@@ -1824,7 +1824,10 @@ static int JPEGDecodeMCU_P(JPEGIMAGE *pJPEG, int iMCU, int *iDCPredictor)
|
||||
unsigned short *pFast;
|
||||
uint32_t usHuff; // this prevents an unnecessary & 65535 for shorts
|
||||
signed int iPositive, iNegative, iCoeff;
|
||||
- signed short *pMCU = &pJPEG->sMCUs[iMCU & 0xffffff];
|
||||
+ // CrossPoint patch: redirect pMCU to sMCUs[0] when MCU_SKIP to avoid
|
||||
+ // a wild pointer (~33 MB past sMCUs) that store-faults on AC writes.
|
||||
+ signed short *pMCU = (iMCU < 0) ? pJPEG->sMCUs
|
||||
+ : &pJPEG->sMCUs[iMCU & 0xffffff];
|
||||
uint32_t ulBitOff;
|
||||
my_ulong ulCode, ulBits, ulTemp; // local copies to allow compiler to use register vars
|
||||
uint8_t *pBuf;
|
||||
--
|
||||
2.50.1 (Apple Git-155)
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
From f6238b54c2de34c8e29b0d371a7e902d9cf579bf Mon Sep 17 00:00:00 2001
|
||||
From: patch <patch@local>
|
||||
Date: Mon, 18 May 2026 20:58:22 -0700
|
||||
Subject: [PATCH 2/2] Guard pMCU[0] DC writes against MCU_SKIP
|
||||
|
||||
---
|
||||
src/jpeg.inl | 11 +++++++++--
|
||||
1 file changed, 9 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/src/jpeg.inl b/src/jpeg.inl
|
||||
index 26bcf6f..1bd38b2 100644
|
||||
--- a/src/jpeg.inl
|
||||
+++ b/src/jpeg.inl
|
||||
@@ -1855,7 +1855,11 @@ static int JPEGDecodeMCU_P(JPEGIMAGE *pJPEG, int iMCU, int *iDCPredictor)
|
||||
{
|
||||
// (*iDCPredictor) |= iPositive; // in case the scan is run more than once
|
||||
// pMCU[0] = *iDCPredictor; // store in MCU[0]
|
||||
- pMCU[0] |= iPositive;
|
||||
+ // CrossPoint patch: guard against MCU_SKIP. The pMCU
|
||||
+ // redirect makes &sMCUs[0] safe to dereference, but
|
||||
+ // writing here would clobber the just-decoded Y DC.
|
||||
+ if (iMCU >= 0)
|
||||
+ pMCU[0] |= iPositive;
|
||||
}
|
||||
goto mcu_done; // that's it
|
||||
}
|
||||
@@ -1887,7 +1891,10 @@ static int JPEGDecodeMCU_P(JPEGIMAGE *pJPEG, int iMCU, int *iDCPredictor)
|
||||
ulCode <<= pJPEG->cApproxBitsLow; // successive approximation shift value
|
||||
(*iDCPredictor) += ulCode;
|
||||
}
|
||||
- pMCU[0] = (short)*iDCPredictor; // store in MCU[0]
|
||||
+ // CrossPoint patch: guard against MCU_SKIP. See note on the
|
||||
+ // matching SA write above.
|
||||
+ if (iMCU >= 0)
|
||||
+ pMCU[0] = (short)*iDCPredictor; // store in MCU[0]
|
||||
}
|
||||
// Now get the other 63 AC coefficients
|
||||
pFast = &pJPEG->usHuffAC[pJPEG->ucACTable * HUFF11SIZE];
|
||||
--
|
||||
2.50.1 (Apple Git-155)
|
||||
|
||||
+72
-48
@@ -1,68 +1,92 @@
|
||||
"""
|
||||
PlatformIO pre-build script: patch JPEGDEC for MCU_SKIP wild pointer crash.
|
||||
PlatformIO pre-build script: apply CrossPoint's JPEGDEC patches via `git apply`.
|
||||
|
||||
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.
|
||||
The upstream JPEGDEC pin still has the wild-pointer + DC-write bugs in
|
||||
JPEGDecodeMCU_P that surface when EIGHT_BIT_GRAYSCALE decodes a 3-component
|
||||
progressive JPEG (each Y MCU drags two MCU_SKIP calls behind it for Cb/Cr).
|
||||
The patches in `scripts/jpegdec_patches/` carry the fix; this script applies
|
||||
each one against the libdep working tree.
|
||||
|
||||
Upstream commit 8628297 guarded the DC coefficient write (pMCU[0]) but not the
|
||||
AC coefficient writes at indices 1-63.
|
||||
Each patch's idempotency is decided by git itself:
|
||||
* `git apply --check --reverse` succeeds -> already applied, skip
|
||||
* `git apply --check` succeeds -> apply
|
||||
* neither succeeds -> abort the build
|
||||
|
||||
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.
|
||||
Patches live in `scripts/jpegdec_patches/` as one-commit-per-fix files
|
||||
(see the file headers for context). Applied in lexical order.
|
||||
"""
|
||||
|
||||
Import("env")
|
||||
Import("env") # noqa: F821 (SCons-injected global)
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
|
||||
PATCH_DIR = os.path.join(env["PROJECT_DIR"], "scripts", "jpegdec_patches") # noqa: F821
|
||||
|
||||
|
||||
def patch_jpegdec(env):
|
||||
libdeps_dir = os.path.join(env["PROJECT_DIR"], ".pio", "libdeps")
|
||||
if not os.path.isdir(libdeps_dir):
|
||||
return
|
||||
patches = _patch_files()
|
||||
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)
|
||||
jpeg_dir = os.path.join(libdeps_dir, env_dir, "JPEGDEC")
|
||||
if not os.path.isdir(os.path.join(jpeg_dir, ".git")):
|
||||
continue
|
||||
for patch in patches:
|
||||
_apply_one(jpeg_dir, patch)
|
||||
|
||||
|
||||
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
|
||||
def _patch_files():
|
||||
if not os.path.isdir(PATCH_DIR):
|
||||
raise RuntimeError(
|
||||
"JPEGDEC patches missing -- aborting build (expected directory %s)"
|
||||
% PATCH_DIR
|
||||
)
|
||||
patches = sorted(
|
||||
os.path.join(PATCH_DIR, name)
|
||||
for name in os.listdir(PATCH_DIR)
|
||||
if name.endswith(".patch")
|
||||
)
|
||||
if not patches:
|
||||
raise RuntimeError(
|
||||
"JPEGDEC patches missing -- aborting build (no .patch files in %s)"
|
||||
% PATCH_DIR
|
||||
)
|
||||
return patches
|
||||
|
||||
|
||||
def _apply_one(jpeg_dir, patch_path):
|
||||
name = os.path.basename(patch_path)
|
||||
if _git_apply_succeeds(jpeg_dir, patch_path, reverse=True):
|
||||
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)
|
||||
if not _git_apply_succeeds(jpeg_dir, patch_path, reverse=False):
|
||||
# Not applied, not appliable -- the libdep source has diverged from
|
||||
# what the patch expects. Don't write a half-patched file.
|
||||
result = subprocess.run(
|
||||
["git", "apply", "--check", patch_path],
|
||||
cwd=jpeg_dir,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
sys.stderr.write(
|
||||
"ERROR: JPEGDEC patch %s does not apply cleanly:\n%s%s\n"
|
||||
% (name, result.stdout, result.stderr)
|
||||
)
|
||||
raise SystemExit(1)
|
||||
subprocess.run(["git", "apply", patch_path], cwd=jpeg_dir, check=True)
|
||||
print("Applied JPEGDEC patch: %s" % name)
|
||||
|
||||
|
||||
# Run immediately at script import time (before compilation).
|
||||
patch_jpegdec(env)
|
||||
def _git_apply_succeeds(jpeg_dir, patch_path, *, reverse):
|
||||
cmd = ["git", "apply", "--check"]
|
||||
if reverse:
|
||||
cmd.append("--reverse")
|
||||
cmd.append(patch_path)
|
||||
return subprocess.run(
|
||||
cmd, cwd=jpeg_dir, capture_output=True, text=True
|
||||
).returncode == 0
|
||||
|
||||
|
||||
patch_jpegdec(env) # noqa: F821
|
||||
|
||||
Reference in New Issue
Block a user