Hi Rahul,
Thank you very much for the thorough review and testing -- the crash-recovery matrix (pglz/lz4/zstd/off + consistency
checking)and especially the crafted-record repro with pg_waldump --save-fullpage crashing without the patch are super
convincing.And thanks for confirming the back-patch safety argument.
v2 attached, addressing all your points:
Commit message now says uncompressed images are affected too. You are right -- there hole_length is derived as BLCKSZ -
bimg_lenand hole_offset was unchecked against bimg_len; the new bound covers it (it amounts to hole_offset <= bimg_len
there).
Rebased onto current master (%d / %X/%08X). Sorry for the stale context in v1 -- it was made against an older tree. v2
applieswith git am.
3-4. Dropped the runtime re-check in RestoreBlockImage() in favor of Assert()s, as you suggested. Since
DecodeXLogRecord()is the only filler of DecodedBkpBlock, it cannot fail, and this also resolves the wrong format and
theduplicate message text.
About back-patching: agreed, thanks for spelling out the XLogRecordAssemble() condition -- no valid WAL can trip the
newcheck.
About the ReadRecPtr vs decoded-record LSN issue: good catch, thank you for noticing while testing. And yes, please
sendthat as a separate patch.
About the frontend test: attached as 0002, since you said you'd like to see it. It is a small
src/test/modules/test_xlogreaderharness that builds hole-image records in memory (valid CRC, compressed + uncompressed)
andfeeds them to DecodeXLogRecord(), so it checks rejection happens at decode time, before RestoreBlockImage() can
touchthe page:
- valid compressed hole, hole ending exactly at BLCKSZ,
- the BUG #19599 shape (hole_offset + hole_length > BLCKSZ), one byte past the page, hole_offset past the page,
- valid uncompressed hole and uncompressed hole_offset past the image.
With v2 all valid cases are accepted and all malformed ones are rejected with the existing BKPIMAGE_HAS_HOLE set
message.I kept it as a separate patch to keep v2 small -- happy to squash if you prefer. And I'd still love to see your
craftingscript, it sounds more end-to-end than mine.
Thanks again,
Yuriy
