Re: BUG #19599: RestoreBlockImage: the decode cross-checks never bound hole_offset + hole_length against BLCKSZ

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_len and 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 applies with 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 the duplicate message text.

About back-patching: agreed, thanks for spelling out the XLogRecordAssemble() condition -- no valid WAL can trip the new check.

About the ReadRecPtr vs decoded-record LSN issue: good catch, thank you for noticing while testing. And yes, please send that 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_xlogreader harness that builds hole-image records in memory (valid CRC, compressed + uncompressed) and feeds them to DecodeXLogRecord(), so it checks rejection happens at decode time, before RestoreBlockImage() can touch the 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 crafting script, it sounds more end-to-end than mine.

Thanks again,
Yuriy